Skip to content

Speed up fallback tests#1724

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

Speed up fallback tests#1724
dgageot merged 1 commit intodocker:mainfrom
dgageot:fix-slow-tests-2

Conversation

@dgageot
Copy link
Member

@dgageot dgageot commented Feb 13, 2026

No description provided.

Signed-off-by: David Gageot <david.gageot@docker.com>
@dgageot dgageot requested a review from a team as a code owner February 13, 2026 18:09
@dgageot
Copy link
Member Author

dgageot commented Feb 13, 2026

@rumpl synctest.Test just for you

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.

Code Review Summary

Reviewed the changes to speed up fallback tests by using testing/synctest.Test() with fake time instead of real time delays.

Changes Analyzed

  • Added testing/synctest import
  • Wrapped 7 test functions with synctest.Test(t, func(t *testing.T) {...})
  • Removed t.Parallel() calls (synctest runs serially with deterministic scheduling)
  • Removed elapsed time assertions (no longer needed with fake time)
  • Replaced require.Eventually() with time.Sleep() + direct assertion in TestFallbackCooldownState

Verification

✅ All CI tests pass (test: SUCCESS)
✅ The use of synctest.Test() with streaming operations works correctly
✅ The fake clock properly synchronizes time.Sleep() and time.Now() calls
✅ The cooldown expiration logic works correctly with fake time

Findings

No issues found. The changes correctly implement test speedup using Go's testing/synctest package. The fake time scheduling is properly synchronized, and all tests pass.

Verdict: APPROVE ✓

Copy link
Member

@rumpl rumpl left a comment

Choose a reason for hiding this comment

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

<3

@dgageot dgageot merged commit c28b4f5 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.

2 participants