test: Reduce test sleep durations in proxy, retry, and cache tests

[Vad1mo]

Summary

• Reduce test execution time by ~528 seconds (8.8 minutes) across the three slowest pure-unit-test packages
• Introduce benbjohnson/clock (already an indirect dependency) for deterministic time control in lib/retry and lib/cache/memory tests
• Replace fixed-interval sleep-poll loops in controller/proxy with context-aware exponential backoff

Related Issues

Fixes #100

Type of Change

• Tests (test:)
• Refactoring (refactor:)

Changes

controller/proxy — 421s → <1s

Replace fixed time.Sleep(20s) poll loops in manifest cache with context-aware exponential backoff.

Aspect	Before	After
First action	Sleep 20s	Check immediately
Poll interval	Fixed 20s	Exponential 1s → 2s → 4s → ... → 30s max
Max wait (manifest list)	20 × 20s = 400s	10 min (context timeout)
Max wait (manifest)	10 × 20s = 200s	5 min (context timeout)
Cancellation	None (ignores context)	Respects ctx.Done()

Tests use short context deadlines (200ms) instead of overriding constants.

lib/retry — 75s → <1s

Add WithClock(clock.Clock) option. Tests inject clock.NewMock() and advance time instantly. Production callers are unchanged — nil clock defaults to real time.

lib/cache/memory — 19s → <1s

Add WithClock(clock.Clock) option to cache.Options, threaded into the memory backend. time.Now() replaced with clock.Now(). Tests inject mock clock and advance past TTL instantly — fully deterministic.

lib/cache/redis — 19s → 6s

Reduce TTL from 5s to 1s, sleep from 8s to 2s. Redis manages its own TTL so clock injection isn't applicable.

Production Code Side Effects

Five production files are modified. Here's why each is safe:

lib/retry/retry.go — No behavior change

• Adds optional Clock field to Options struct
• When nil (all 25 existing callers), bclock.New() returns a real clock
• bclock.New().After() delegates to time.After(), .Sleep() to time.Sleep()
• Existing callers get identical behavior

lib/cache/options.go + lib/cache/memory/memory.go — No behavior change

• Adds optional Clock field to Options struct
• When nil (all existing callers), clock.New() returns a real clock
• clock.New().Now() returns time.Now() — identical behavior
• Only the memory backend reads the clock; Redis ignores it
• Existing callers get identical behavior

controller/proxy/controller.go + manifestcache.go — Improved behavior

The manifest cache retry loops run in a background goroutine detached from the HTTP request:

ProxyManifest()
  └─ go func() {
       bCtx := orm.Copy(ctx)          ← strips request deadline/cancellation
       waitAndPushManifest(bCtx, ...)  ← runs independently of request
     }()

orm.Copy() returns a valueOnlyContext where Done() returns nil and Deadline() returns zero. The goroutine is completely independent of the request lifecycle.

Side effect analysis:

1. Faster initial response — old code slept 20s before first check; new code checks immediately. If dependencies are ready, push happens ~20s sooner. Positive.
2. More frequent early polling — first checks at 1s, 2s, 4s instead of 20s. These are local DB queries, not remote calls. Negligible overhead.
3. Less frequent late polling — after ~2min, interval is 30s vs old 20s. Negligible.
4. Longer manifest list timeout — 400s → 600s in worst case (no deps ever arrive). Goroutine lives 3.3min longer before giving up. Runs in background, doesn't block anything. Minor.
5. Context cancellation — orm.Copy() context has no parent cancellation, so ctx.Done() only fires from the WithTimeout added in new code. Equivalent to old iteration-count limit. No change in practice.

Testing

• controller/proxy — all tests pass (<1s)
• lib/retry — all tests pass with mock clock (<1s)
• lib/cache/memory — all tests pass with mock clock (<1s)
• All tests pass with -race

Checklist

• PR title follows Conventional Commits format
• Commits are signed off (git commit -s)
• No new warnings introduced

[coderabbitai]

📝 Walkthrough

Walkthrough

Replaced fixed retry constants with timeout- and backoff-based constants; updated manifest push logic to use context-scoped timeouts and exponential backoff; adjusted test contexts and mock expectations to accept any context; shortened TTLs/sleeps in cache tests; added explicit Timeout option in retry test.

Changes

Cohort / File(s)	Summary
Manifest constants src/controller/proxy/controller.go	Replaced fixed-wait constants with manifestListPushTimeout (10m), manifestPushTimeout (5m), and backoff params initialRetryInterval (1s) / maxRetryInterval (30s); preserved manifestListCacheInterval.
Manifest cache logic src/controller/proxy/manifestcache.go	Switched push loops to use context.WithTimeout and exponential backoff (time.After(wait) with doubling to maxRetryInterval); on context timeout the code logs and proceeds to push partial results instead of relying on fixed iteration counts.
Manifest cache tests src/controller/proxy/manifestcache_test.go	Tests updated to use context timeouts and relax mock argument matching to mock.Anything for context parameters.
In-memory cache tests src/lib/cache/memory/memory_test.go	Reduced test cache expiration/TTL from 5s → 1s and shortened sleep assertions from 8s → 2s.
Redis cache tests src/lib/cache/redis/redis_test.go	Reduced test cache expiration/TTL from 5s → 1s and shortened sleep assertions from 8s → 2s.
Retry test src/lib/retry/retry_test.go	TestRetry now calls Retry(..., Timeout(3*time.Second)) for the always-fail case to bound the test duration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check	❓ Inconclusive	The PR addresses issue #100 by identifying and reducing test execution time from sleep operations, though it takes a different approach (optimizing sleep durations) rather than resolving the underlying testify/mock synchronization overhead that is the focus of #100.	Clarify whether reducing sleep durations is a preliminary optimization or a complete alternative to addressing the testify/mock race-detector overhead documented in #100. Consider if the PR should also include mock framework migration planning.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: reducing test sleep durations across proxy, retry, and cache tests to improve test execution time.
Out of Scope Changes check	✅ Passed	All changes are scoped to test code only (constants in manifestcache.go, test files, TTL/sleep adjustments) with no unintended production behavior modifications beyond the intended test timing optimizations.
Description check	✅ Passed	The pull request provides a comprehensive, well-structured description that covers summary, related issues, type of change, testing details, and a thorough analysis of side effects.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/reduce-test-sleep-durations

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 555ffda.

Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

[Copilot]

Pull request overview

Reduces overall Go test runtime by shortening long time.Sleep()-based waits in the slowest test suites (proxy manifest caching, retry timeouts, and cache TTL expiry), while keeping production behavior the same by default.

Changes:

• Added an explicit short timeout to the “always fails” retry test to avoid the default 60s wait.
• Reduced cache TTLs and corresponding sleeps in memory and redis cache tests.
• Made proxy manifest wait parameters test-overridable and updated proxy manifest cache sleeps accordingly.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/lib/retry/retry_test.go	Reduces worst-case test duration by setting a 3s retry timeout for an always-failing case.
src/lib/cache/redis/redis_test.go	Shortens Redis cache expiration and sleep durations to speed up TTL-related tests.
src/lib/cache/memory/memory_test.go	Shortens in-memory cache expiration and sleep durations to speed up TTL-related tests.
src/controller/proxy/manifestcache.go	Updates sleep calls to work with new test-overridable interval variables.
src/controller/proxy/manifestcache_test.go	Overrides proxy manifest wait parameters to avoid minutes-long sleeps in the suite.
src/controller/proxy/controller.go	Converts manifest retry/sleep parameters from consts to vars so tests can override them.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/controller/proxy/manifestcache_test.go`:
- Around line 64-67: The test suite overrides package globals sleepIntervalSec,
maxManifestListWait, and maxManifestWait for faster tests but never restores
them; update the suite to save the original values before changing them (e.g.,
in SetupSuite or where they are set) and restore those originals in
TearDownSuite so other tests don't inherit the mutated timing configuration,
referencing the globals sleepIntervalSec, maxManifestListWait, maxManifestWait
and the test lifecycle method TearDownSuite.

In `@src/lib/cache/memory/memory_test.go`:
- Line 35: The suite-wide cache is created with a 1s TTL which causes flaky
failures under the race detector; instead, change the default in the TestSuite
setup (where suite.cache is assigned via cache.New and cache.Expiration) to a
much longer safe TTL (e.g., minutes) and reserve the 1s TTL only for tests that
specifically assert expiration by constructing a dedicated cache instance inside
those tests (call cache.New(..., cache.Expiration(time.Second)) locally). Update
any tests that rely on the global short TTL to create their own short-lived
cache so suite-wide timing is stable.

In `@src/lib/cache/redis/redis_test.go`:
- Line 35: The suite-wide use of a 1s TTL (suite.cache = cache.New("redis",
cache.Expiration(time.Second))) can cause race-mode flakiness; change the
suite-level initialization to use a much longer default TTL (e.g., time.Minute
or no expiration) and restrict cache.Expiration(time.Second) to only the tests
that assert expiration behavior (create per-test caches inside those
expiry-specific tests using cache.New with cache.Expiration(time.Second));
update references to suite.cache initialization in the setup helper (where
suite.cache is created) and ensure expiry tests construct their own short-TTL
caches instead of relying on suite.cache.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fed13a64-9d97-443c-981e-5c5446103597

📥 Commits

Reviewing files that changed from the base of the PR and between 49ce0f3 and e08ea83.

📒 Files selected for processing (6)

• src/controller/proxy/controller.go
• src/controller/proxy/manifestcache.go
• src/controller/proxy/manifestcache_test.go
• src/lib/cache/memory/memory_test.go
• src/lib/cache/redis/redis_test.go
• src/lib/retry/retry_test.go

[cubic-dev-ai]

1 issue found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/controller/proxy/manifestcache_test.go">

<violation number="1" location="src/controller/proxy/manifestcache_test.go:65">
P2: Package-level globals (`sleepIntervalSec`, `maxManifestListWait`, `maxManifestWait`) are overridden in `SetupSuite` but never restored. This leaks modified timing configuration to any other tests in the `controller/proxy` package that run after this suite. Save the original values before overriding and restore them in `TearDownSuite`.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[github-actions]

Preview images for this PR are available in registry.goharbor.io/harbor-next with tag pr-103.

• registry.goharbor.io/harbor-next/harbor-core:pr-103
• registry.goharbor.io/harbor-next/harbor-jobservice:pr-103
• registry.goharbor.io/harbor-next/harbor-registryctl:pr-103
• registry.goharbor.io/harbor-next/harbor-exporter:pr-103
• registry.goharbor.io/harbor-next/harbor-portal:pr-103
• registry.goharbor.io/harbor-next/harbor-registry:pr-103
• registry.goharbor.io/harbor-next/harbor-trivy-adapter:pr-103

Verify a preview image:

cosign verify \
  --certificate-identity-regexp="https://github.com/container-registry/harbor-next/.github/workflows/pr-ci.yml@.*" \
  --certificate-oidc-issuer="https://token.actions.githubusercontent.com" \
  registry.goharbor.io/harbor-next/harbor-core:pr-103

Verify SBOM attestation:

cosign verify-attestation \
  --certificate-identity-regexp="https://github.com/container-registry/harbor-next/.github/workflows/pr-ci.yml@.*" \
  --certificate-oidc-issuer="https://token.actions.githubusercontent.com" \
  --type spdxjson \
  registry.goharbor.io/harbor-next/harbor-core:pr-103