diff --git a/modules/mesh-delivery-checker/go.mod b/modules/mesh-delivery-checker/go.mod index 2842eee..a8a6539 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.20261026123512-bb713103371a + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026130105-45564e69a648 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 866b124..565a39a 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.20261026111253-9c61918ac36f h1:9+TZpNq8 git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026111253-9c61918ac36f/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026123512-bb713103371a h1:ZxKOtc8LusZEEpbbu41lYxJiaITN8lZCIcasA5kdgGQ= git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026123512-bb713103371a/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026130105-45564e69a648 h1:UUhWHYnaTJwgkgoYhQtJrm+r5S5UNAQ4DRMlt4x21Hg= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261026130105-45564e69a648/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/examiner_test.go b/modules/mesh-delivery-checker/internal/check/examiner_test.go index 8578f77..c8f5b13 100644 --- a/modules/mesh-delivery-checker/internal/check/examiner_test.go +++ b/modules/mesh-delivery-checker/internal/check/examiner_test.go @@ -292,9 +292,20 @@ func TestOnlyThePackageWithoutAKeptVerdictRuns(t *testing.T) { // 1288/AC1 // Issue 890/AC5: a package needing node-build-node is left unmet where the node lacks it; the check keeps the passes // and asks the integration level again for a node with it, puts it again when a node declines it, and that node tests // the package alone, its kept passes not counted twice. + // fixture/outside has no kept verdict after its pass (it reads outside itself: its digest moves every call), so a + // placed ask that ran more than its unmet packages would run and count it again. b = newBench(t) b.declines = 1 - b.digest[delivery.LevelIntegration] = Digested{Level: "integration", Packages: map[string]string{"fixture/slow": digB, "fixture/node": digC}, + outside := 0 + b.steps.handlers["build_go_digest"] = func(c stepCall) (any, error) { + d := echoBeside(b, b.digest[c.args["level"].(string)]) + if d.Level == "integration" { + outside++ + d.Packages = map[string]string{"fixture/slow": digB, "fixture/node": digC, "fixture/outside": fmt.Sprintf("%064x", outside)} + } + return d, nil + } + b.digest[delivery.LevelIntegration] = Digested{Level: "integration", Needs: []delivery.PackageNeeds{{Package: "fixture/node", Needs: []string{delivery.NeedNodeBuildNode}}}} out = b.judge(t, caseOf(t, pullOf(repoA, 7, head1))) asks := b.asks.Asks() @@ -302,13 +313,13 @@ func TestOnlyThePackageWithoutAKeptVerdictRuns(t *testing.T) { // 1288/AC1 t.Fatalf("verdict %+v, asks %+v: want a pass of three asks, the first naming no need", out.Verdict, asks) } for _, a := range asks[1:] { - if strings.Join(a.Needs, ",") != delivery.NeedNodeBuildNode || strings.Join(a.Check.Levels, ",") != delivery.LevelIntegration { + if strings.Join(a.Needs, ",") != delivery.NeedNodeBuildNode || strings.Join(a.Check.Levels, ",") != delivery.LevelIntegration || strings.Join(a.Check.Packages, ",") != "fixture/node" { t.Errorf("a placed ask %+v, want the integration level needing node-build-node", a) } } tests = b.steps.tool("test") if last := tests[len(tests)-1]; len(tests) != 3 || strings.Join(packagesOf(last), ",") != "fixture/node" || last.args["level"] != "integration" { - t.Errorf("test calls = %+v, want the unit level, the integration level of fixture/slow, then fixture/node alone", tests) + t.Errorf("test calls = %+v, want the unit level, the integration level of fixture/slow and fixture/outside, then fixture/node alone", tests) } slow := captured[Tested](t, "go-test-slow") if i := out.Verdict.Levels[delivery.LevelIntegration]; i.Run != 2*slow.Tests || i.Kept != 0 { diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 938c1af..06f8f11 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -393,15 +393,16 @@ const DefaultDeclineBound = 30 * time.Minute // their tests needing what that node lacks (issue 890), the passes are kept and the levels from the first such one are // asked again of a node that has it (placed). func (st *state) ask(ctx context.Context, t *target, merged *[]Fetched) *delivery.Verdict { - return st.askNeeding(ctx, t, merged, st.levelsOf(), nil) + return st.askNeeding(ctx, t, merged, st.levelsOf(), nil, nil) } -// askNeeding is ask with the levels to run and what the node must have: none for the first ask of a target, the unmet -// packages' needs for the placed one. -func (st *state) askNeeding(ctx context.Context, t *target, merged *[]Fetched, levels, needs []string) *delivery.Verdict { +// askNeeding is ask with the levels to run, what the node must have and the packages it tests: none for the first ask +// of a target; for the placed one the unmet packages' needs and those packages alone, so no package the first ask judged +// is run or counted again (ADR 0552 ยง4). +func (st *state) askNeeding(ctx context.Context, t *target, merged *[]Fetched, levels, needs, packages []string) *delivery.Verdict { p := t.pull placed := len(needs) > 0 - job := &delivery.CheckJob{Fetch: t.fetchArgs, Levels: levels, Kept: true} + job := &delivery.CheckJob{Fetch: t.fetchArgs, Levels: levels, Kept: true, Packages: packages} if t.runner != "" { job.Runner, job.RunnerWhen = t.runner, buildGoDir } @@ -496,7 +497,7 @@ func (st *state) askNeeding(ctx context.Context, t *target, merged *[]Fetched, l return fail(step.Node, fmt.Sprintf("the %s level of %s#%d left %s untested on %s, which was asked for a node with %s", la.Level, p.Repository, p.Number, unmetNames(la.Unmet), step.Node, strings.Join(needs, ", "))) } - la.Kept = nil // what the placed ask took kept, the first ask ran or took: counted there + la.Kept = nil // it names its packages and takes no other's kept verdict; this is only a guard against an older module } v, elapsed := st.level(ctx, t, la) ran += elapsed @@ -509,14 +510,21 @@ func (st *state) askNeeding(ctx context.Context, t *target, merged *[]Fetched, l if unmet := delivery.UnmetNeeds(answer.Levels); len(unmet) > 0 && !placed { // Every level passed, its passes kept, and some packages were left to a node with what their tests need: the levels // from the first that left one are asked of such a node, which takes the kept passes and tests those (issue 890). - var again []string + var again, packages []string + seen := map[string]bool{} for _, la := range answer.Levels { if len(again) > 0 || len(la.Unmet) > 0 { again = append(again, la.Level) } + for _, u := range la.Unmet { + if !seen[u.Package] { + seen[u.Package] = true + packages = append(packages, u.Package) + } + } } st.j.logf("%s#%d: %s on %s left untested; asked of a node with %s", p.Repository, p.Number, unmetOf(answer.Levels), step.Node, strings.Join(unmet, ", ")) - return st.askNeeding(ctx, t, merged, again, unmet) + return st.askNeeding(ctx, t, merged, again, unmet, packages) } return nil }