From 84395420ee90391c59cc4b5ff007cbc04ce5366c Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 01:24:37 +0200 Subject: [PATCH] mesh-delivery-checker: take an ask's longest level off its time for the clone, as the levels now run side by side, and pin mesh-sdk's side-by-side levels (issue 1921, try speed S10) --- modules/mesh-delivery-checker/go.mod | 2 +- modules/mesh-delivery-checker/go.sum | 2 ++ .../internal/check/ask_all_test.go | 16 ++++++++++ .../internal/check/examiner_test.go | 30 +++++++++++++------ .../internal/check/judge.go | 2 +- .../internal/check/judged_test.go | 4 +-- 6 files changed, 43 insertions(+), 13 deletions(-) diff --git a/modules/mesh-delivery-checker/go.mod b/modules/mesh-delivery-checker/go.mod index faec5b8..144298d 100644 --- a/modules/mesh-delivery-checker/go.mod +++ b/modules/mesh-delivery-checker/go.mod @@ -3,7 +3,7 @@ module mesh-delivery-checker go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024231716-571ac521f053 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024232250-9b408db24a58 github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-delivery-checker/go.sum b/modules/mesh-delivery-checker/go.sum index c5c11c1..6880727 100644 --- a/modules/mesh-delivery-checker/go.sum +++ b/modules/mesh-delivery-checker/go.sum @@ -2,6 +2,8 @@ git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024214154-bc16740feb0d h1:oJREw0gg git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024214154-bc16740feb0d/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024231716-571ac521f053 h1:8Kbp3aJG3jGndDOfmGdMSDgJVEcc1hyu921hVnnlnlQ= git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024231716-571ac521f053/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024232250-9b408db24a58 h1:4Damtdqlf6u6nJhvMA99peDy1jmjixtcwFXJ4bWEkLk= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024232250-9b408db24a58/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= diff --git a/modules/mesh-delivery-checker/internal/check/ask_all_test.go b/modules/mesh-delivery-checker/internal/check/ask_all_test.go index b96e2c6..93de817 100644 --- a/modules/mesh-delivery-checker/internal/check/ask_all_test.go +++ b/modules/mesh-delivery-checker/internal/check/ask_all_test.go @@ -59,3 +59,19 @@ func TestAGroupPutsEveryMembersAskBeforeTheFirstAnswer(t *testing.T) { t.Errorf("%d asks were put, want 3", counting.puts) } } + +// 1921/S10: the levels of an ask run side by side, so what of the ask's time is its fetch (Took.Clone) is the ask's time +// less its longest level, not less the sum of its levels. +func TestTheCloneTimeTakesTheLongestLevelOffNotTheSum(t *testing.T) { + b := newBench(t) + b.test = func(c stepCall) (any, error) { + r := captured[Tested](t, "go-test-slow") + r.Level, _ = c.args["level"].(string) + r.Elapsed, r.Setup, r.Lint = 1.5, 0, 0 // the ask's step takes 2 s (bench.answer) + return r, nil + } + v := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))).Verdict + if v.State != delivery.VerdictPass || v.Took.Clone != 500 { + t.Errorf("clone = %d ms, want 500 (2 s less the longest level's 1.5 s): %+v", v.Took.Clone, v) + } +} diff --git a/modules/mesh-delivery-checker/internal/check/examiner_test.go b/modules/mesh-delivery-checker/internal/check/examiner_test.go index 25c9c1e..8a3d3ff 100644 --- a/modules/mesh-delivery-checker/internal/check/examiner_test.go +++ b/modules/mesh-delivery-checker/internal/check/examiner_test.go @@ -74,6 +74,18 @@ func (f *fakeSteps) tool(name string) []stepCall { return out } +// toolAt is tool's calls of one level: a check's levels run side by side (issue 1921, try speed S10), so only the calls of +// one level keep an order. +func (f *fakeSteps) toolAt(name, level string) []stepCall { + var out []stepCall + for _, c := range f.tool(name) { + if c.args["level"] == level { + out = append(out, c) + } + } + return out +} + type entries []delivery.NodeToolchain func (e entries) Entries(language string) []delivery.NodeToolchain { @@ -269,9 +281,9 @@ func TestOnlyThePackageWithoutAKeptVerdictRuns(t *testing.T) { // 1288/AC1 t.Fatal(err) } out := b.judge(t, caseOf(t, pullOf(repoA, 7, head1))) - tests := b.steps.tool("test") - if len(tests) != 2 || strings.Join(packagesOf(tests[0]), ",") != "fixture/slow" || tests[0].args["level"] != "unit" { - t.Fatalf("test calls = %+v, want the unit level of fixture/slow alone, then the integration level", tests) + tests := b.steps.toolAt("test", "unit") + if len(tests) != 1 || strings.Join(packagesOf(tests[0]), ",") != "fixture/slow" || len(b.steps.toolAt("test", "integration")) != 1 { + t.Fatalf("test calls = %+v, want the unit level of fixture/slow alone, and the integration level", b.steps.tool("test")) } if u := out.Verdict.Levels[delivery.LevelUnit]; u.Run != 2 || u.Kept != 40 { t.Errorf("unit = %+v, want 2 run and 40 kept", u) @@ -302,7 +314,7 @@ func TestAReAskRunsOnlyWhatFailedAndWhatChanged(t *testing.T) { // 1288/AC2, AC3 // Asked again with nothing changed: only the failed package runs; the good one is taken. b.steps.calls = nil b.judge(t, caseOf(t, pullOf(repoA, 7, head1))) - tests := b.steps.tool("test") + tests := b.steps.toolAt("test", "unit") if len(tests) == 0 || strings.Join(packagesOf(tests[0]), ",") != "fixture/failing/bad" { t.Fatalf("the re-ask ran %+v, want only fixture/failing/bad", tests) } @@ -310,7 +322,7 @@ func TestAReAskRunsOnlyWhatFailedAndWhatChanged(t *testing.T) { // 1288/AC2, AC3 b.digest[delivery.LevelUnit] = Digested{Level: "unit", Packages: map[string]string{"fixture/failing/bad": digA, "fixture/failing/good": digD}} b.steps.calls = nil b.judge(t, caseOf(t, pullOf(repoA, 7, head1))) - tests = b.steps.tool("test") + tests = b.steps.toolAt("test", "unit") if got := strings.Join(packagesOf(tests[0]), ","); got != "fixture/failing/bad,fixture/failing/good" && got != "fixture/failing/good,fixture/failing/bad" { t.Errorf("after a digest changed the run was %q, want the failed package and the changed one", got) } @@ -342,7 +354,7 @@ func TestANodePackageIsNeverKeptAndRunsEveryCheck(t *testing.T) { // 1288/AC4, 1 } // 1289/AC1, AC2 as issue 1921 changed it: the change's own failing test runs first, the level's other packages still run -// after it (so one check names every failure), and no later level runs. +// after it (so one check names every failure); the later level ran beside it (S10) and is not judged. func TestTheChangesOwnFailingTestRunsFirstThenTheRestAndNoLaterLevel(t *testing.T) { b := newBench(t) b.fetch.Changed = []string{"modules/m/internal/b/b_test.go"} @@ -358,12 +370,12 @@ func TestTheChangesOwnFailingTestRunsFirstThenTheRestAndNoLaterLevel(t *testing. if out.Verdict.State != delivery.VerdictFail || !strings.Contains(out.Verdict.Failed.Test, "TestThreeIsFour") { t.Fatalf("verdict = %+v", out.Verdict) } - tests := b.steps.tool("test") + tests := b.steps.toolAt("test", "unit") if len(tests) != 2 || strings.Join(packagesOf(tests[0]), ",") != "m/internal/b" || strings.Join(packagesOf(tests[1]), ",") != "m/internal/a" { t.Fatalf("test calls = %+v, want m/internal/b, then m/internal/a, each once", tests) } - if n := len(b.steps.tool("digest")); n != 1 { - t.Errorf("%d digest calls, want the unit level's alone (the check stopped)", n) + if out.Verdict.Levels[delivery.LevelIntegration].Run != 0 { + t.Errorf("the integration level was judged after the unit level failed: %+v", out.Verdict.Levels) } if out.Verdict.Composition == nil || out.Verdict.Composition.State != delivery.CompositionNotRun { t.Errorf("composition = %+v", out.Verdict.Composition) diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 9b7ff24..9faeb4d 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -473,7 +473,7 @@ func (st *state) judgeAsk(ctx context.Context, t *target, merged *[]Fetched) *de ran := 0.0 for _, la := range answer.Levels { v, elapsed := st.level(ctx, t, la) - ran += elapsed + ran = max(ran, elapsed) // the levels run side by side (issue 1921, try speed S10): the longest is what they took if v != nil { st.clone += max(0, took-time.Duration(ran*float64(time.Second))) return v diff --git a/modules/mesh-delivery-checker/internal/check/judged_test.go b/modules/mesh-delivery-checker/internal/check/judged_test.go index e915b39..39b71db 100644 --- a/modules/mesh-delivery-checker/internal/check/judged_test.go +++ b/modules/mesh-delivery-checker/internal/check/judged_test.go @@ -177,8 +177,8 @@ func TestACandidateThatDoesNotCompileFailsTheCheckWithTheCompilersWords(t *testi if out.Verdict.State != delivery.VerdictFail || !strings.Contains(out.Verdict.Failed.Words, "undefined: x") { t.Fatalf("verdict = %+v, want a failure naming the compile error", out.Verdict) } - if n := len(b.steps.tool("test")); n != 1 { - t.Errorf("%d test steps, want the first one's failure to end the check", n) + if n := len(b.steps.toolAt("test", "unit")); n != 1 { + t.Errorf("%d unit test steps, want the first one's failure to end the level", n) } }