Skip to content

Replace time.Sleep in tests with deterministic synchronization#1722

Merged
dgageot merged 1 commit intodocker:mainfrom
dgageot:fix-tests-sleep
Feb 13, 2026
Merged

Replace time.Sleep in tests with deterministic synchronization#1722
dgageot merged 1 commit intodocker:mainfrom
dgageot:fix-tests-sleep

Conversation

@dgageot
Copy link
Member

@dgageot dgageot commented Feb 13, 2026

Use require.Eventually, time.AfterFunc, and channel synchronization instead of fixed time.Sleep calls to make tests faster and less flaky.

  • telemetry: poll mockHTTP.GetRequestCount() with require.Eventually
  • fake/proxy: use channel write and require.Eventually for sync points
  • runtime/fallback: use time.AfterFunc for context cancel, require.Eventually for cooldown expiry
  • tui/styles: poll atomic callback counter with require.Eventually

Assisted-By: cagent

Use require.Eventually, time.AfterFunc, and channel synchronization
instead of fixed time.Sleep calls to make tests faster and less flaky.

- telemetry: poll mockHTTP.GetRequestCount() with require.Eventually
- fake/proxy: use channel write and require.Eventually for sync points
- runtime/fallback: use time.AfterFunc for context cancel, require.Eventually for cooldown expiry
- tui/styles: poll atomic callback counter with require.Eventually

Assisted-By: cagent
@dgageot dgageot requested a review from a team as a code owner February 13, 2026 17:12
Copy link

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

No issues found!

The changes look good. The PR successfully replaces fixed time.Sleep calls with deterministic synchronization patterns:

  • pkg/fake/proxy_test.go: Channel write + require.Eventually for proper sync
  • pkg/runtime/fallback_test.go: time.AfterFunc + require.Eventually for clean timing control
  • pkg/runtime/runtime_test.go: Improved comment explaining timing-based negative test
  • pkg/telemetry/telemetry_test.go: All time.Sleep replaced with require.Eventually polling
  • pkg/tui/styles/theme_watcher_test.go: Proper atomic polling with require.Eventually

All require.Eventually calls use correct syntax, appropriate timeouts, and proper thread-safe operations. No race conditions or synchronization issues detected.

@rumpl
Copy link
Member

rumpl commented Feb 13, 2026

Can't we use "testing/synctest"

@dgageot
Copy link
Member Author

dgageot commented Feb 13, 2026

@rumpl It's coming in the next batch (where it's really useful)

@dgageot dgageot merged commit 14ea695 into docker:main Feb 13, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants