[tests] Use testing/synctest to drop time.Sleep + deadline magic in TestEval_CancellationHaltsExecution

[robbyt]

Summary

TestEval_CancellationHaltsExecution exists in both engines/risor/evaluator/evaluator_test.go and engines/starlark/evaluator/evaluator_test.go (shipped in PR #127). Each currently uses a magic time.Sleep(50ms) to give the engine time to enter the spin loop before cancelling, and a 2s deadline to catch unresponsive cancellation:

ctx, cancel := context.WithCancel(t.Context())
done := make(chan error, 1)
go func() {
    _, err := eval.Eval(ctx)
    done <- err
}()

time.Sleep(50 * time.Millisecond)   // hope the engine is mid-script
cancel()

select {
case err := <-done:
    require.Error(t, err)
    // assert cancellation-shaped error
case <-time.After(2 * time.Second):
    t.Fatal("Eval did not return within 2s after cancel")
}

The 50ms is a best-guess flake risk: too short and the cancel fires before the engine enters the loop; too long and the test is slow. The 2s deadline is sized for CI slack but still arbitrary. testing/synctest (stable in Go 1.25+, available in this repo's Go 1.26) eliminates both magic numbers.

Proposal

Wrap each test in synctest.Run. Use synctest.Wait() instead of time.Sleep. The bubble-aware semantics give us:

1. The eval goroutine spawned inside the bubble is visible.
2. synctest.Wait() returns only when every goroutine in the bubble is durably blocked (chan recv, mutex, time.Sleep) — for the evaluator that means it has entered its tight script loop and is only blocked on the periodic ctx check.
3. After cancel, the next synctest.Wait() (or just the original wg.Wait()-shape) returns when the eval goroutine is done — no 2s timeout magic.

Sketch:

import "testing/synctest"

func TestEval_CancellationHaltsExecution(t *testing.T) {
    t.Parallel()

    synctest.Run(func() {
        // setup ...
        ctx, cancel := context.WithCancel(t.Context())
        done := make(chan error, 1)
        go func() {
            _, err := eval.Eval(ctx)
            done <- err
        }()

        synctest.Wait()  // engine is mid-script
        cancel()

        err := <-done    // returns deterministically when eval halts
        require.Error(t, err)
        require.True(t,
            errors.Is(err, context.Canceled) ||
                strings.Contains(err.Error(), "context canceled") ||
                strings.Contains(err.Error(), "cancel"),
            "expected cancellation-shaped error, got: %v", err,
        )
    })
}

Caveat to verify before merging

A goroutine that's running CPU-bound code (not blocked on a sync primitive) does NOT count as "durably blocked" — synctest.Wait() will block forever. Both engines need to be checked:

• Risor v2: its VM has a background goroutine watching <-ctx.Done() (vm.go:285-289) plus periodic deterministic ctx checks every 1000 instructions. The deterministic check is a ctx.Err() poll, not a chan recv, so the main eval goroutine is CPU-bound — synctest.Wait() won't see it as durably blocked. synctest may not actually work for Risor.
• Starlark: AfterFunc-registered thread.Cancel is the cancellation path. Same issue — the Starlark interpreter is CPU-bound in its instruction loop.

If both engines are CPU-bound in their inner loops, this conversion is a no-go. Worth a 30-min spike to verify before committing to it. If only the timer / channel-blocked code paths benefit from synctest, this issue might still resolve as "tested, didn't apply" rather than the conversion landing.

Out of scope

• Removing -race. synctest does not replace race-detector coverage; both layers serve different purposes. The cancellation test would still be expected to pass under -race -count=20 as it does today.
• Other engine tests — only this specific test pair.

Files

• engines/risor/evaluator/evaluator_test.go — wrap TestEval_CancellationHaltsExecution
• engines/starlark/evaluator/evaluator_test.go — same
• May need import "testing/synctest" and a t.Parallel reconsideration (synctest.Run + t.Parallel is supported in Go 1.25+).

Verification

1. go test -race -count=20 -run TestEval_CancellationHaltsExecution ./engines/{risor,starlark}/evaluator/... — must still pass, no flakes.
2. go test -count=20 (without -race) — should also be fast and deterministic.
3. Negative check: temporarily comment out cancel() — both tests must now FAIL deterministically (rather than time-out at 2s as they do today).

Why this is useful

• Drops the magic 50ms Sleep and arbitrary 2s deadline.
• Makes the test purely behavioral: "after cancel, eval halts" — no timing assertion required.
• If the spike confirms it works for both engines, the same pattern can apply to any future cancellation regression tests.

Pairs with #126's note that testing/synctest is the right tool for tests around timers/deadlines. This issue is the specific application.