Skip to content

Replace time.Sleep with channel synchronization in dial timeout test #11

Description

@lanycrost

Problem statement

TestCallWithTimeoutPropagatesParentCancellationWhenCallbackIgnoresContext in dial_test.go:110 uses time.Sleep(10 * time.Millisecond) to wait for the callback goroutine to start before cancelling the parent context. This is a race condition — on slow CI machines, the goroutine might not have started in 10ms, causing the cancellation to fire before the callback is running.

Proposed change

Replace the sleep with a started channel for deterministic synchronization:

func TestCallWithTimeoutPropagatesParentCancellationWhenCallbackIgnoresContext(t *testing.T) {
    ctx, cancel := context.WithCancel(context.Background())
    release := make(chan struct{})
    started := make(chan struct{})
    defer close(release)

    done := make(chan error, 1)
    go func() {
        done <- CallWithTimeout(ctx, time.Second, "call", func(context.Context) error {
            close(started) // Signal that callback is running
            <-release
            return nil
        })
    }()

    <-started // Deterministic: wait for callback to actually start
    cancel()

    select {
    case err := <-done:
        if !errors.Is(err, context.Canceled) {
            t.Fatalf("expected context canceled, got %v", err)
        }
    case <-time.After(200 * time.Millisecond):
        t.Fatal("expected cancellation to return before callback release")
    }
}

Affected area

  • runtime/*

Compatibility / migration

Test-only change. No behavior change.

Alternatives considered

  • Increasing the sleep duration — still non-deterministic, just less likely to fail.

Additional context

The other timing-based tests use 20x safety margins (10ms timeout, 200ms assertion threshold) which is reasonable. This specific test is the only one using sleep-based synchronization. Identified during test quality review.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/transportBinding, protocol, connector, or transport runtime changes.good first issueSmall, well-scoped tasks for new contributors.help wantedLooking for community contributions.kind/testsTesting, CI, or verification-only changes.priority/lowNice-to-have or backlog item.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions