diff --git a/go/buildstep/askrun.go b/go/buildstep/askrun.go index 06156e1..2a0836b 100644 --- a/go/buildstep/askrun.go +++ b/go/buildstep/askrun.go @@ -213,16 +213,15 @@ func (s AskSteps) check(ctx context.Context, job delivery.CheckJob) (any, error) if job.Runner != "" && (job.RunnerWhen == "" || changesUnder(f.Changed, job.RunnerWhen)) { runner = job.Runner } + // Every level runs, also after one that did not pass: one check names every failure, not only the first (the operator, + // 2026-11-02: "A failed check shows only its first fault"). for _, level := range job.Levels { la := delivery.LevelAnswer{Level: level} - passed, err := s.level(ctx, job, level, f, runner, &la) + _, err := s.level(ctx, job, level, f, runner, &la) answer.Levels = append(answer.Levels, la) if err != nil { return nil, err } - if !passed { - return answer, nil // the first level that did not pass ends the check (ADR 0484 clause 13) - } } return answer, nil } @@ -261,10 +260,9 @@ func (s AskSteps) level(ctx context.Context, job delivery.CheckJob, level string _ = json.Unmarshal(r, &t) return t.Passed, nil } - // The packages (or Node directories) whose tests the change touches run first, and a failure among them ends the level - // there, so a change's own failing test is answered without the rest of the suite (issue 1289; ADR 0444). Past them every - // test step of the level runs, also after one that did not pass, so one check names every failing test of the rest of - // the level and not only the first (issue 1921); the level passes only when each of its steps did. + // The packages (or Node directories) whose tests the change touches run first (issue 1289; ADR 0444), and then every + // other test step of the level, also after one that did not pass, so one check names every failing test of the level + // and not only the first (issue 1921; the operator, 2026-11-02); the level passes only when each of its steps did. all := true if s.Digest == nil { // A module with no digest (Node) tests each module directory of the tree, those whose tests changed first. @@ -279,9 +277,6 @@ func (s AskSteps) level(ctx context.Context, job delivery.CheckJob, level string } all = all && passed } - if !all { - return false, nil // the change's own tests failed: the others are not run (issue 1289) - } } return all, nil } @@ -317,10 +312,7 @@ func (s AskSteps) level(ctx context.Context, job delivery.CheckJob, level string if err != nil { return false, fmt.Errorf("the %s level: %w", level, err) } - all = all && passed // past the change's own packages the others still run (issue 1921) - if priority && !all { - return false, nil // the change's own tests failed: the others are not run (issue 1289) - } + all = all && passed // the others still run after a failure (issue 1921) } return all, nil } diff --git a/go/buildstep/askrun_test.go b/go/buildstep/askrun_test.go index 30d4072..5616b18 100644 --- a/go/buildstep/askrun_test.go +++ b/go/buildstep/askrun_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "slices" "strings" "testing" "time" @@ -125,8 +126,8 @@ func TestRunAskRefusesAnotherToolchain(t *testing.T) { } // 1887/A3, A5a: a check ask runs the levels of one check after one fetch: it digests the unit level, takes the kept -// verdicts, tests the packages whose tests changed first and, when one of them fails, no other (issue 1289), and runs no -// level after one that did not pass (ADR 0484 clause 13). +// verdicts, tests the packages whose tests changed first and then the others, also after one of them failed, and runs +// every level, also after one that did not pass, so one check names every failure (the operator, 2026-11-02). func TestRunAskChecksTheLevelsOfOneCheck(t *testing.T) { r := &recorder{ fetch: map[string]any{"workspace": "/w", "changed": []string{"pkg/b/b_test.go"}}, @@ -141,16 +142,20 @@ func TestRunAskChecksTheLevelsOfOneCheck(t *testing.T) { got := strings.Join(r.calls, "\n") want := `fetch {"base":"` + runCommit + `","commit":"` + runCommit + `","repository":"novox/x"} digest {"level":"unit","workspace":"/w"} -test {"level":"unit","packages":["x/pkg/b"],"workspace":"/w"}` +test {"level":"unit","packages":["x/pkg/b"],"workspace":"/w"} +test {"level":"unit","packages":["x/pkg/a"],"workspace":"/w"} +digest {"level":"integration","workspace":"/w"} +test {"level":"integration","packages":["x/pkg/b"],"workspace":"/w"} +test {"level":"integration","packages":["x/pkg/a"],"workspace":"/w"}` if got != want { t.Fatalf("calls:\n%s\nwant:\n%s", got, want) } a := out.(delivery.CheckAnswer) - if len(a.Levels) != 1 { - t.Fatalf("levels %+v, want the unit level alone", a.Levels) + if len(a.Levels) != 2 { + t.Fatalf("levels %+v, want both levels", a.Levels) } u := a.Levels[0] - if u.Level != delivery.LevelUnit || len(u.Kept) != 1 || u.Kept[0].Package != "x/pkg/k" || u.Kept[0].Tests != 7 || len(u.Tests) != 1 || u.Digest == nil { + if u.Level != delivery.LevelUnit || len(u.Kept) != 1 || u.Kept[0].Package != "x/pkg/k" || u.Kept[0].Tests != 7 || len(u.Tests) != 2 || u.Digest == nil { t.Fatalf("the unit level %+v", u) } @@ -265,17 +270,17 @@ func TestRunAskChecksNodeModulesAndConflicts(t *testing.T) { } } -// Issue 1289: a failure in the packages whose tests changed ends the level there, the others are not run, and no later level -// runs. Issue 1921: past the change's own packages every package still runs, so two failing packages none of which the -// change touched are two test results of the one level. A Node check does the same with its module directories. -func TestRunAskStopsAtTheChangesOwnFailureAndRunsTheRestAfterOthers(t *testing.T) { +// Issue 1289: the packages whose tests changed run first. Since 2026-11-02 (the operator: "A failed check shows only its +// first fault") a failure among them ends nothing: every package of the level and every level still runs, so one check +// names every failing test (issue 1921). A Node check does the same with its module directories. +func TestRunAskRunsEveryPackageAndLevelAfterAFailure(t *testing.T) { for _, row := range []struct { name string changed []string tests map[string]bool - want []string // the packages tested, in order + want []string // the packages tested at each level, in order }{ - {"the change's own test fails: nothing else runs", []string{"pkg/b/b_test.go"}, map[string]bool{"x/pkg/a": false, "x/pkg/b": false}, []string{"x/pkg/b"}}, + {"the change's own test fails: the rest still runs", []string{"pkg/b/b_test.go"}, map[string]bool{"x/pkg/a": false, "x/pkg/b": false}, []string{"x/pkg/b", "x/pkg/a"}}, {"the change's own test passes: the rest runs, also after a failure", []string{"pkg/b/b_test.go"}, map[string]bool{"x/pkg/a": false, "x/pkg/b": true, "x/pkg/c": false}, []string{"x/pkg/b", "x/pkg/a x/pkg/c"}}, {"no test changed: every failure is named", nil, map[string]bool{"x/pkg/a": false, "x/pkg/b": false}, []string{"x/pkg/a x/pkg/b"}}, } { @@ -297,19 +302,11 @@ func TestRunAskStopsAtTheChangesOwnFailureAndRunsTheRestAfterOthers(t *testing.T got = append(got, strings.Join(a.Packages, " ")) } } - // the passing row goes on to the integration level, which tests the same packages again; the first level is what is judged - if n := len(row.want); len(got) < n || fmt.Sprint(got[:n]) != fmt.Sprint(row.want) { - t.Errorf("%s: tested %v, want %v first", row.name, got, row.want) + if want := append(slices.Clone(row.want), row.want...); fmt.Sprint(got) != fmt.Sprint(want) { + t.Errorf("%s: tested %v, want %v", row.name, got, want) } - a := out.(delivery.CheckAnswer) - failed := false - for _, raw := range a.Levels[0].Tests { - var res struct{ Passed bool } - _ = json.Unmarshal(raw, &res) - failed = failed || !res.Passed - } - if failed && (len(a.Levels) != 1 || len(got) != len(row.want)) { - t.Errorf("%s: a failed level went on: levels %d, tests %v", row.name, len(a.Levels), got) + if a := out.(delivery.CheckAnswer); len(a.Levels) != 2 || len(a.Levels[0].Tests) != len(row.want) { + t.Errorf("%s: levels %+v, want both, each with every test result", row.name, a.Levels) } } @@ -322,7 +319,7 @@ func TestRunAskStopsAtTheChangesOwnFailureAndRunsTheRestAfterOthers(t *testing.T if err != nil { t.Fatal(err) } - if a := out.(delivery.CheckAnswer); len(r.calls) != 2 || len(a.Levels) != 1 || len(a.Levels[0].Tests) != 1 { - t.Fatalf("calls %v, levels %+v: want the changed directory b alone and no integration level", r.calls, a.Levels) + if a := out.(delivery.CheckAnswer); len(r.calls) != 5 || len(a.Levels) != 2 || len(a.Levels[0].Tests) != 2 { + t.Fatalf("calls %v, levels %+v: want b then a at each of the two levels", r.calls, a.Levels) } } diff --git a/go/delivery/checker.go b/go/delivery/checker.go index 6b07ede..b5f5bad 100644 --- a/go/delivery/checker.go +++ b/go/delivery/checker.go @@ -143,7 +143,7 @@ type FailedFirst struct { Words string `json:"words,omitempty"` } -// MaxFailures is the most entries a verdict's Failures holds (issue 1921): fifty tests named one by one, and an entry +// MaxFailures is the most entries a verdict's Failures holds (issue 1921): fifty failures named one by one, and an entry // after them saying how many more failed. const MaxFailures = 50 @@ -157,10 +157,11 @@ type Verdict struct { Trunk map[string]string `json:"trunk,omitempty"` State string `json:"state"` Failed FailedFirst `json:"failed,omitzero"` - // Failures names every test that failed, in the order judged, each with the first line of its words (issue 1921): - // Failed keeps the first one's words whole, and a check that ran every test says all it found here. At most - // MaxFailures entries, the last of a longer list saying how many more failed. Absent in a value written before it, - // and from a verdict whose failure is no test. + // Failures names everything that failed, in the order judged, each with the first line of its words (issue 1921): every + // failed test, and every member's refused step, conflict, manifest refusal and the composition's failure, since a check + // runs every member, level and the composition to its end (the operator, 2026-11-02). Failed keeps the first one's + // words whole. At most MaxFailures entries, the last of a longer list saying how many more failed. Absent in a value + // written before it, and from a verdict with one failure that is no test. Failures []FailedFirst `json:"failures,omitempty"` Levels map[string]LevelResult `json:"levels,omitempty"` Took Took `json:"took,omitzero"` diff --git a/go/delivery/stepasks.go b/go/delivery/stepasks.go index 155a685..f1d932d 100644 --- a/go/delivery/stepasks.go +++ b/go/delivery/stepasks.go @@ -344,7 +344,8 @@ type AskArtifact struct { // CheckJob is a check ask's job, the levels of one check (ADR 0525 ยง3): fetch the head merged onto its trunk once, then // for each level in order digest its packages, take the kept verdicts for the digests that have one, and test the rest, -// the packages the change's own tests are in first, stopping at the first level that does not pass (CheckAnswer). +// the packages the change's own tests are in first, every level and package running to its end also after a failure, +// so one check names every failure (CheckAnswer). type CheckJob struct { // Fetch is the fetch tool's arguments (repository, commit, ref, base, and the build module's own: beside, package). Fetch map[string]any `json:"fetch"` @@ -564,7 +565,7 @@ type AskBuilt struct { } // CheckAnswer is a check ask's step result, each part the build module's own tool's answer whole: the fetch's, then one -// LevelAnswer per level run, in order; a level that did not pass is the last. The checker judges and keeps from it as it +// LevelAnswer per level run, in order, every asked level also after one that did not pass. The checker judges and keeps from it as it // did from the tools' answers. type CheckAnswer struct { Fetch json.RawMessage `json:"fetch"`