From ab44d0eeba0578e0d38bc3dbb962fb36ff4eaa3f Mon Sep 17 00:00:00 2001 From: merefield Date: Thu, 8 Oct 2026 15:44:23 +0100 Subject: [PATCH] FIX: bound CI test runtime and remove flaky benchmark timing --- .github/workflows/ci.yml | 29 +++++++++++++++++++++-- .github/workflows/release.yml | 24 ++++++++++++++++--- README.md | 21 ++++++++++++++-- internal/codex/benchmark_test.go | 7 ++++++ internal/ui/localisation_test.go | 24 ++++++++++++++++--- internal/ui/monitor_approval_test.go | 4 +++- internal/ui/monitor_form_controls_test.go | 10 +++++--- 7 files changed, 105 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 69010d2..d5fb65f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,6 +9,10 @@ on: permissions: contents: read +concurrency: + group: ci-${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: web: name: Experimental web @@ -39,6 +43,7 @@ jobs: test: name: Go ${{ matrix.os }} runs-on: ${{ matrix.os }} + timeout-minutes: 25 strategy: fail-fast: false matrix: @@ -69,8 +74,28 @@ jobs: - name: Vet run: go vet ./... - - name: Test - run: go test -race -cover ./... + # Keep all assertions and race instrumentation. Separate invocations + # give the 17-locale rendering matrix its own package timeout budget. + - name: Core tests (race and coverage) + shell: bash + run: | + set -o pipefail + go test -race -cover -json -timeout 10m -skip '^TestLocalisedScreens$' ./... | tee core-test-results.json + + - name: Localised screens (all locales, race and coverage) + shell: bash + run: | + set -o pipefail + go test -race -cover -json -timeout 10m -run '^TestLocalisedScreens$' ./internal/ui | tee locale-test-results.json + + - name: Preserve test results and timings + if: always() + uses: actions/upload-artifact@v7 + with: + name: go-tests-${{ matrix.os }} + path: '*-test-results.json' + retention-days: 7 + if-no-files-found: ignore - name: Build run: go build -trimpath ./... diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7427510..e4cb8e7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -64,7 +64,7 @@ jobs: name: Test ${{ matrix.os }} needs: validate runs-on: ${{ matrix.os }} - timeout-minutes: 20 + timeout-minutes: 25 permissions: contents: read strategy: @@ -114,8 +114,26 @@ jobs: - name: Vet run: go vet ./... - - name: Test - run: go test -race -cover ./... + - name: Core tests (race and coverage) + shell: bash + run: | + set -o pipefail + go test -race -cover -json -timeout 10m -skip '^TestLocalisedScreens$' ./... | tee core-test-results.json + + - name: Localised screens (all locales, race and coverage) + shell: bash + run: | + set -o pipefail + go test -race -cover -json -timeout 10m -run '^TestLocalisedScreens$' ./internal/ui | tee locale-test-results.json + + - name: Preserve test results and timings + if: always() + uses: actions/upload-artifact@v7 + with: + name: release-go-tests-${{ matrix.os }} + path: '*-test-results.json' + retention-days: 7 + if-no-files-found: ignore - name: Build run: go build -trimpath ./... diff --git a/README.md b/README.md index f45e6e5..0be8bc0 100644 --- a/README.md +++ b/README.md @@ -2918,8 +2918,8 @@ remain in the TUI. Existing regression tests cover English presentation across t and sizes, localisation, responsive layouts, mouse hit regions, session navigation, approvals, reset confirmation and quota learning. Additional launch tests ensure `--web` cannot start the terminal or benchmark discovery and normal -launches never start web mode. CI retains `go test -race -cover ./...`, vet and -build checks on Linux, macOS and Windows; browser tests currently run Chromium +launches never start web mode. CI retains race-instrumented tests and coverage, +vet and build checks on Linux, macOS and Windows; browser tests currently run Chromium on Linux. These checks provide regression evidence, not a guarantee that every terminal emulator or OS/browser combination is covered. @@ -3159,6 +3159,23 @@ Measure test coverage: go test -cover ./... ``` +CI and release workflows run the core suite and localisation matrix separately, +giving each its own explicit ten-minute package timeout without dropping tests, +locales, coverage instrumentation or race detection: + +```sh +go test -race -cover -timeout 10m -skip '^TestLocalisedScreens$' ./... +go test -race -cover -timeout 10m -run '^TestLocalisedScreens$' ./internal/ui +``` + +All 17 locale options run the complete localised-screen suite in isolated test +subprocesses, with at most two locale workers active at once. Each child has a +three-minute test timeout and an outer four-minute process deadline. CI uploads +JSON test results, including per-locale helper timings, for seven days even on +failure. The overall Go job is bounded to 25 minutes. Superseded ordinary CI runs +are cancelled; release runs are not. Benchmark timeout-recovery tests use Go's +virtual clock rather than depending on millisecond wall-clock scheduling. + Codexometer uses: - Go 1.27.1+ diff --git a/internal/codex/benchmark_test.go b/internal/codex/benchmark_test.go index 763c65e..249caeb 100644 --- a/internal/codex/benchmark_test.go +++ b/internal/codex/benchmark_test.go @@ -10,6 +10,7 @@ import ( "os" "strings" "testing" + "testing/synctest" "time" "unicode/utf8" ) @@ -354,6 +355,12 @@ func TestRunBenchmarkFailsClosedWhenUsageIsMissing(t *testing.T) { } func TestRunBenchmarkSuiteInterruptsTimedOutTurnAndContinues(t *testing.T) { + // Virtual time advances only when goroutines are durably blocked. Thread + // setup cannot accidentally exhaust a 20 ms deadline on a busy CI runner. + synctest.Test(t, testRunBenchmarkSuiteInterruptsTimedOutTurnAndContinues) +} + +func testRunBenchmarkSuiteInterruptsTimedOutTurnAndContinues(t *testing.T) { message := string(rawJSON(map[string]string{"code": correctStarlarkSubmission})) server, requests := newFakeBenchmarkServer( benchmarkEnvelope{ID: rawJSON(1), Result: rawJSON(map[string]any{ diff --git a/internal/ui/localisation_test.go b/internal/ui/localisation_test.go index f4f8bab..5a74b99 100644 --- a/internal/ui/localisation_test.go +++ b/internal/ui/localisation_test.go @@ -1,6 +1,7 @@ package ui import ( + "context" "os" "os/exec" "strings" @@ -21,17 +22,34 @@ func TestLocalisedScreens(t *testing.T) { if err != nil { t.Fatal(err) } + // Separate processes keep package-level locale state isolated. Limit the + // workers instead of launching 17 race-instrumented renderers at once on + // small CI runners; every locale still runs the complete surface suite. + workers := make(chan struct{}, 2) for _, code := range []string{"en-GB", "nl", "de", "fr", "it", "es", "ru", "ja", "zh-Hans", "sv", "nb", "tr", "et", "fi", "pt-BR", "da", "pt-PT"} { t.Run(code, func(t *testing.T) { - cmd := exec.Command(exe, "-test.run=^TestLocalisedScreensHelper$") + t.Parallel() + workers <- struct{}{} + defer func() { <-workers }() + // The child timeout prints the active test stack; the outer deadline + // also bounds a child that hangs before the test harness starts. + ctx, cancel := context.WithTimeout(context.Background(), 4*time.Minute) + defer cancel() + cmd := exec.CommandContext(ctx, exe, "-test.run=^TestLocalisedScreensHelper$", "-test.timeout=3m", "-test.v") + cmd.WaitDelay = 5 * time.Second for _, env := range os.Environ() { if !strings.HasPrefix(env, i18n.EnvironmentVariable+"=") && !strings.HasPrefix(env, "CODEXOMETER_LOCALE_HELPER=") { cmd.Env = append(cmd.Env, env) } } cmd.Env = append(cmd.Env, i18n.EnvironmentVariable+"="+code, "CODEXOMETER_LOCALE_HELPER=1") - if output, err := cmd.CombinedOutput(); err != nil { - t.Fatalf("%v\n%s", err, output) + started := time.Now() + output, err := cmd.CombinedOutput() + // Include individual helper timings in go test -json artifacts on + // success too, so the next hotspot need not wait for a timeout. + t.Logf("locale %s completed in %s\n%s", code, time.Since(started), output) + if err != nil { + t.Fatalf("locale subprocess: %v (deadline: %v)", err, ctx.Err()) } }) } diff --git a/internal/ui/monitor_approval_test.go b/internal/ui/monitor_approval_test.go index 4eb6468..d0d4f5a 100644 --- a/internal/ui/monitor_approval_test.go +++ b/internal/ui/monitor_approval_test.go @@ -87,6 +87,8 @@ func TestMonitorApprovalRenderedTargets(t *testing.T) { m.monitorApprovalConfirmUntil = time.Now().Add(monitorApprovalConfirmDuration) } out := m.render() + plain := ansi.Strip(out) + lines := strings.Split(plain, "\n") if lipgloss.Width(out) > width || lipgloss.Height(out) > height { t.Fatalf("overflow %dx%d", width, height) } @@ -99,7 +101,7 @@ func TestMonitorApprovalRenderedTargets(t *testing.T) { action := "decision:" + strconv.Itoa(i) label := approvalShortcutLabel(option.Kind, confirmed && i == 0, i) found := false - for y, line := range strings.Split(ansi.Strip(out), "\n") { + for y, line := range lines { pos := strings.Index(line, label) if pos < 0 { continue diff --git a/internal/ui/monitor_form_controls_test.go b/internal/ui/monitor_form_controls_test.go index 1462042..1e1a5df 100644 --- a/internal/ui/monitor_form_controls_test.go +++ b/internal/ui/monitor_form_controls_test.go @@ -34,7 +34,12 @@ func testScheduleRenderedControlSurfaces(t *testing.T) { copy := m copy.width = g.contentWidth _, controls := copy.scheduleFormDocument() - plain := strings.Split(ansi.Strip(m.render()), "\n") + // Each hover starts from the same immutable model and fixed clock. + // Reuse its whole-screen baseline; still render and compare every + // hovered result, so colour-only and geometry regressions stay covered. + before := m.render() + beforePlain := ansi.Strip(before) + plain := strings.Split(beforePlain, "\n") modes := 0 for _, c := range controls { if strings.HasPrefix(c.key, "mode:") { @@ -62,11 +67,10 @@ func testScheduleRenderedControlSurfaces(t *testing.T) { if strings.HasPrefix(c.key, "day:") && c.value != m.scheduleUI.date.Day() { continue } - before := m.render() next, _ := m.Update(tea.MouseMotionMsg{X: 4 + c.x, Y: row}) n := next.(Model) after := n.render() - if n.scheduleUI.hover != c.key || before == after || ansi.Strip(before) != ansi.Strip(after) { + if n.scheduleUI.hover != c.key || before == after || beforePlain != ansi.Strip(after) { t.Fatal("hover missing or changed layout", width, mode, c.key) } if c.focus != 5 && c.focus != 6 {