remove jonboulle/clockwork for testing/synctest

[thaJeztah]

The fake clock is only used by tests to control the debounce timer,
but requires carrying a clock abstraction through production code.

Use the standard time package for the debounce ticker and use
testing/synctest to control time in tests instead.

This removes the clock from composeService and BatchDebounceEvents,
and eliminates the clockwork dependency from the watch implementation.

What I did

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did

[thaJeztah]

Not sure if this function is used outside of compose itself, because it's a signature change; if that's a problem, we can perhaps replace it with _ any or something

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

This PR cleanly removes the jonboulle/clockwork dependency by replacing it with Go 1.26's testing/synctest — a good improvement that eliminates the clock abstraction from production code. The debounce.go implementation correctly uses t.Reset(QuietPeriod) after each event, preserving the sliding-window debounce semantics.

One test logic issue was found in watch_test.go: the second event batch sends both a rebuild event and a sync event, but handleWatchBatch processes both independently — meaning syncer.Sync() IS called. The subsequent select { default: } assertion (asserting no sync occurred) misses this, leaving a goroutine permanently blocked in the synctest bubble, which will cause a panic rather than a clean test pass.

[docker-agent]

[medium] Second test batch asserts no sync occurred, but handleWatchBatch WILL call syncer.Sync() — leaving a goroutine permanently blocked

The second event batch sends both /rebuild and /sync/changed:

• /rebuild matches the WatchActionRebuild rule → triggers s.rebuild(...)
• /sync/changed matches the WatchActionSync rule → triggers syncer.Sync(...) which blocks on the unbuffered syncer.synced <- paths

handleWatchBatch processes both actions (rebuild first, then sync via syncfiles iteration — they are independent maps). So syncer.Sync() is called, and the goroutine durably blocks on f.synced <- paths (a bubble-internal channel send).

After synctest.Wait() returns (because all other goroutines are durably blocked), the select { default: } branch is taken — the pending send on syncer.synced is never read. The blocked goroutine leaks inside the bubble. When synctest.Test exits and waits for all goroutines to finish, the test panics with a deadlock/goroutine-leak message rather than a clear failure.

The original clockwork-based test used time.After(100ms) as the "no sync" timeout, which would also time out incorrectly in this scenario — but the old test's assertion was already wrong before this PR (the test comment says // TODO: there's not a great way to assert that the rebuild attempt happened). The migration to synctest surfaces this latent bug as a hard panic.

Fix options:

1. Read (and drain) syncer.synced after the rebuild batch, then assert the contents were not pure-sync (or just drain it):

   // Drain any pending sync caused by the mixed rebuild+sync batch
   select {
   case <-syncer.synced:
   default:
   }

2. Adjust the watch rules so the second batch contains only a rebuild event (no /sync/changed), making the "no sync" assertion valid.
3. Explicitly assert that a sync DID happen for the /sync/changed event (since it matches a sync rule even in a rebuild batch), updating the test intention.

Confidence	Score
🟡 moderate	60/100

[thaJeztah]

@docker-agent I think this is a false positive. handleWatchBatch only reaches the sync actions if the rebuild succeeds:

if len(rebuild) > 0 {
	if err := s.rebuild(...); err != nil {
		return err
	}
}

In this test the rebuild intentionally fails (invalid service "test". Must specify either image or build), so syncer.Sync() is never called for the /sync/changed event in that batch.

That also explains the existing assertion and TODO: the mixed batch is useful to verify that sync processing does not continue after the rebuild failure, while the test doesn't have a clean way to directly assert the rebuild attempt itself.

With synctest.Wait() followed by the non-blocking select, the original assertion remains valid and deterministic.

[docker-agent]

You're absolutely right — I misread the control flow. The early return on rebuild failure means syncer.Sync() is never reached for the /sync/changed event in the mixed batch, so the "no sync" assertion is correct.

I've stored this pattern (early-return-on-rebuild-failure preventing downstream sync processing) to avoid flagging it incorrectly in future reviews. Thanks for the clarification and for explaining the test's intention — the mixed batch now makes much more sense as a deliberate verification that sync doesn't proceed after rebuild failures.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[docker-agent]

Assessment: 🟢 APPROVE

This PR cleanly removes the jonboulle/clockwork abstraction and replaces fake-clock test control with Go's standard testing/synctest package. The implementation is correct.

Key correctness checks performed:

• synctest.Test bubble scoping: All goroutines started within the synctest.Test callback — including the go func() running watchEvents and the inner goroutine spawned by BatchDebounceEvents — participate in the fake-time bubble. time.NewTicker, time.Sleep, and Ticker.Reset all interact correctly with the bubble's fake clock.

• time.Sleep(QuietPeriod) → synctest.Wait() → <-syncer.synced ordering: Fake time only advances when all goroutines in the bubble are durably blocked. By the time time.Sleep(QuietPeriod) returns, the full pipeline (ticker fire → flushEvents → out <- events → watchEvents receive → handleWatchBatch → syncer.Sync → f.synced <- paths) has already completed to the point where watchEvents is blocked on the unbuffered f.synced <- paths send. synctest.Wait() then confirms this stable state, and <-syncer.synced safely receives. No race.

• Rebuild scenario default: branch: After rebuild events are processed, s.rebuild fails (no build context in the inline composeService struct literal), so handleWatchBatch returns early without calling syncer.Sync. After synctest.Wait() all goroutines are durably blocked in their select loops and syncer.synced is empty — the default: case is correctly non-racy in the synctest context.

• Ticker.Reset within synctest: time.Ticker.Reset works correctly inside a synctest bubble; Go 1.26.3's synctest implementation handles all standard time operations.

• clockwork fully removed: Confirmed absent from go.mod, go.sum, pkg/compose/compose.go (clock field and clockwork.NewRealClock() initialisation), pkg/watch/debounce.go (signature and t.Chan() → t.C), and both test files.

• Debounce semantics preserved: The sliding-window behaviour (t.Reset(QuietPeriod) on each incoming event) is unchanged; only the clock source changes from clockwork to the standard library.

[thaJeztah]

@glours @ndeloof - quite some whitespace-only changes because the synctest increases indentation, so easiest to review with whitespace hidden; add ?w=1 to the diff URL;

• https://github.com/docker/compose/pull/14185/changes?w=1