Skip to content

Comments

test: migrate timer-based tests to testing/synctest#4453

Draft
kakkoyun wants to merge 3 commits intoDataDog:mainfrom
kakkoyun:kakkoyun/synctest
Draft

test: migrate timer-based tests to testing/synctest#4453
kakkoyun wants to merge 3 commits intoDataDog:mainfrom
kakkoyun:kakkoyun/synctest

Conversation

@kakkoyun
Copy link
Member

What does this PR do?

Migrates timer-dependent and polling-loop tests to use Go 1.25's
testing/synctest fake-clock
bubbles, replacing real wall-clock sleeps and assert.Eventually
polling with deterministic, instant-in-fake-time equivalents.

The migration is structured in four phases:

Phase 0 — extend internal/synctest wrapper with a Wait()
re-export.

Phase 1 — pure timer tests (1 sleep replaced per test):

  • internal/log/log_test.go — rate-limiter expiry
  • ddtrace/tracer/span_test.go — duration measurement + polling
  • ddtrace/tracer/stats_test.go — concentrator flush
  • ddtrace/tracer/writer_test.go — concurrent flush

Phase 2assert.Eventually + injectable-time patterns:

  • ddtrace/tracer/abandonedspans_test.go — replace 10
    assert.Eventually calls and defer setTestTime()() overrides; span
    start times are now relative to time.Now() inside each bubble.
  • internal/telemetry/client_test.go — wrap TestClientFlush
    table-driven loop in synctest.Test; fix heartbeat case with a 1 ns
    sleep; remove runtime.Gosched() (no-op inside bubbles); use defer c.Close() instead of t.Cleanup so the ticker goroutine stops before
    the bubble exits.

Phase 3 — polling loops:

  • ddtrace/tracer/tracer_test.go: migrate TestTracerAtomicFlush and
    TestWorker; replace a time.After(2s*timeMultiplicator) +
    time.Sleep(10ms) polling loop with synctest.Wait() + direct
    assertion.

Phase 4 — cleanup: remove setTestTime(), which overrode the
package-level now var for deterministic span-age strings. The
synctest fake clock makes this override redundant.

Motivation

Timer-based tests that use time.Sleep or assert.Eventually are
inherently racy and slow. They either rely on wall-clock timing (fragile
in CI, slow on loaded machines) or use inflated timeouts that make the
suite unnecessarily slow. testing/synctest provides a fake clock where
time.Sleep(1 * time.Second) is instant and deterministic, eliminating
the root cause rather than working around it with time multipliers.

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.
  • New code is free of linting errors. You can check this by running make lint locally.
  • New code doesn't break existing tests. You can check this by running make test locally.
  • Add an appropriate team label so this PR gets put in the right place for the release notes.
  • All generated files are up to date. You can check this by running make generate locally.
  • Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild. Make sure all nested modules are up to date by running make fix-modules locally.

@kakkoyun kakkoyun force-pushed the kakkoyun/synctest branch 2 times, most recently from 2d61b7b to cd2d72b Compare February 19, 2026 16:30
Extend the internal/synctest wrapper with a Wait() re-export and
migrate tests that used raw time.Sleep as a proxy for timer expiry.

- internal/synctest: add Wait() re-export
- internal/log: wrap rate-limiter expiry test in synctest bubble
- ddtrace/tracer/span_test: replace duration-measurement sleeps with
  synctest bubbles; simplify polling in TestSpanFinishWithError
- ddtrace/tracer/stats_test: replace concentrator flush sleep with
  synctest.Wait()
- ddtrace/tracer/writer_test: replace concurrent flush sleep with
  synctest.Wait()

Inside a synctest bubble time.Sleep is instant (fake clock), so these
tests go from ~seconds of wall time to effectively zero.

Signed-off-by: Kemal Akkoyun <kemal.akkoyun@datadoghq.com>
Replace assert.Eventually polling loops, time-variable overrides, and
runtime.Gosched() workarounds with deterministic synctest bubbles.

- ddtrace/tracer/abandonedspans_test: wrap all subtests in
  synctest.Test, compute span start times relative to the fake clock's
  time.Now(), and remove the setTestTime() helper that overrode the
  package-level now var.
- internal/telemetry/client_test: wrap TestClientFlush in
  synctest.Test; add a 1 ns sleep in the heartbeat case to advance the
  fake clock past the rate-limiter interval; remove runtime.Gosched()
  calls (no-op inside bubbles); switch from t.Cleanup to defer for
  c.Close() so the ticker goroutine stops before the bubble exits.
- ddtrace/tracer/tracer_test: migrate TestTracerAtomicFlush and
  TestWorker; replace a time.After+time.Sleep polling loop with
  synctest.Wait() followed by a direct assertion.

Signed-off-by: Kemal Akkoyun <kemal.akkoyun@datadoghq.com>
On Windows, var now points to GetSystemTimePreciseAsFileTime() via a
direct Windows syscall. This syscall bypasses testing/synctest's fake
clock, which only intercepts time.Now(). As a result, synctest-wrapped
tests fail on Windows: span durations read the real wall clock (a few
µs) instead of the fake 2 ms advance, and span-age strings diverge
between the test and the code under test.

Add an init() in time_windows_test.go that overrides now and nowTime to
call time.Now() for the test binary. Production code keeps its
high-precision syscall; tests gain fake-clock compatibility without any
change to production behavior.

Signed-off-by: Kemal Akkoyun <kemal.akkoyun@datadoghq.com>
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.

1 participant