diff --git a/go/buildstep/askrun.go b/go/buildstep/askrun.go index 9956271..7f0ee11 100644 --- a/go/buildstep/askrun.go +++ b/go/buildstep/askrun.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "path" + "slices" "sort" "strings" @@ -162,19 +163,35 @@ func (s AskSteps) check(ctx context.Context, job delivery.CheckJob) (any, error) if len(f.Conflicts) > 0 { return answer, nil // the asker reads the conflict from the fetch's answer: a verdict, not an error } + if s.Test == nil { + return nil, Refuse("this build module tests nothing") + } runner := "" if job.Runner != "" && (job.RunnerWhen == "" || changesUnder(f.Changed, job.RunnerWhen)) { runner = job.Runner } + for _, level := range job.Levels { + la := delivery.LevelAnswer{Level: level} + passed, 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 +} + +// level runs one level of a check ask and says whether it passed. +func (s AskSteps) level(ctx context.Context, job delivery.CheckJob, level string, f fetched, runner string, la *delivery.LevelAnswer) (bool, error) { withRunner := func(args map[string]any) map[string]any { if runner != "" { args["runner"] = runner } return args } - if s.Test == nil { - return nil, Refuse("this build module tests nothing") - } test := func(args map[string]any) (bool, error) { out, err := s.Test(ctx, args) if err != nil { @@ -184,7 +201,7 @@ func (s AskSteps) check(ctx context.Context, job delivery.CheckJob) (any, error) if err != nil { return false, err } - answer.Tests = append(answer.Tests, r) + la.Tests = append(la.Tests, r) var t struct { Passed bool `json:"passed"` } @@ -198,23 +215,20 @@ func (s AskSteps) check(ctx context.Context, job delivery.CheckJob) (any, error) if changesTestIn(f.Changed, dir) != priority { continue } - passed, err := test(withRunner(map[string]any{"workspace": f.Workspace, "module": dir, "level": job.Level})) + passed, err := test(withRunner(map[string]any{"workspace": f.Workspace, "module": dir, "level": level})) if err != nil { - return nil, fmt.Errorf("the %s level of %s: %w", job.Level, dir, err) + return false, fmt.Errorf("the %s level of %s: %w", level, dir, err) } if !passed { - return answer, nil + return false, nil } } } - return answer, nil - } - todo, all, err := s.plan(ctx, job, f, withRunner, &answer) - if err != nil { - return nil, err + return true, nil } + todo, all := s.plan(ctx, job, level, f, runner, withRunner, la) for _, priority := range []bool{true, false} { - args := withRunner(map[string]any{"workspace": f.Workspace, "level": job.Level}) + args := withRunner(map[string]any{"workspace": f.Workspace, "level": level}) if job.Run != "" { args["run"] = job.Run } @@ -236,13 +250,13 @@ func (s AskSteps) check(ctx context.Context, job delivery.CheckJob) (any, error) } passed, err := test(args) if err != nil { - return nil, fmt.Errorf("the %s level: %w", job.Level, err) + return false, fmt.Errorf("the %s level: %w", level, err) } if !passed { - return answer, nil + return false, nil } } - return answer, nil + return true, nil } // release removes an ask's workspace, saying a failure: the module's sweep removes it later. @@ -255,30 +269,36 @@ func (s AskSteps) release(workspace string) { } } -// plan digests the level and takes the kept verdicts: the packages left to test, or all when the level runs whole (a -// try of some tests, or a digest that named nothing). -func (s AskSteps) plan(ctx context.Context, job delivery.CheckJob, f fetched, withRunner func(map[string]any) map[string]any, answer *delivery.CheckAnswer) ([]string, bool, error) { - if job.Run != "" { - return nil, true, nil // a partial run proves no package: nothing is taken or kept +// plan digests the level and takes the kept verdicts: the packages left to test, or all when the level runs whole. It +// runs whole, digesting nothing, for a try of some tests (a partial run proves no package) and under the candidate's own +// runner (ADR 0535 §4: the candidate decides which verdicts would be taken, so every package runs); it runs whole and +// takes nothing when the digest names no package or does not cover the siblings the fetch placed (ADR 0444, L5). +func (s AskSteps) plan(ctx context.Context, job delivery.CheckJob, level string, f fetched, runner string, withRunner func(map[string]any) map[string]any, la *delivery.LevelAnswer) ([]string, bool) { + if job.Run != "" || runner != "" { + return nil, true } - out, err := s.Digest(ctx, withRunner(map[string]any{"workspace": f.Workspace, "level": job.Level})) + out, err := s.Digest(ctx, withRunner(map[string]any{"workspace": f.Workspace, "level": level})) if err != nil { if s.Logf != nil { - s.Logf("no digest of the %s level, so it runs whole: %v", job.Level, err) + s.Logf("no digest of the %s level, so it runs whole: %v", level, err) } - return nil, true, nil + return nil, true } r, err := raw(out) if err != nil { - return nil, true, nil + return nil, true } - answer.Digest = r var d struct { Packages map[string]string `json:"packages"` + Beside []string `json:"beside"` } if err := json.Unmarshal(r, &d); err != nil || len(d.Packages) == 0 { - return nil, true, nil + return nil, true } + if !slices.Equal(d.Beside, besideOf(job.Fetch)) { + return nil, true // a digest that does not cover the siblings keys no verdict: the level runs whole, keeping nothing + } + la.Digest = r pkgs := make([]string, 0, len(d.Packages)) for p := range d.Packages { pkgs = append(pkgs, p) @@ -287,21 +307,37 @@ func (s AskSteps) plan(ctx context.Context, job delivery.CheckJob, f fetched, wi var todo []string for _, p := range pkgs { if job.Kept && s.Kept != nil { - tests, found, err := s.Kept(ctx, job.Level, d.Packages[p]) + tests, found, err := s.Kept(ctx, level, d.Packages[p]) if err != nil && s.Logf != nil { - s.Logf("the kept verdict of %s at %s cannot be read, so it runs: %v", p, job.Level, err) + s.Logf("the kept verdict of %s at %s cannot be read, so it runs: %v", p, level, err) } if err == nil && found { - answer.Kept = append(answer.Kept, delivery.KeptTaken{Package: p, Digest: d.Packages[p], Tests: tests}) + la.Kept = append(la.Kept, delivery.KeptTaken{Package: p, Digest: d.Packages[p], Tests: tests}) continue } } todo = append(todo, p) } - if len(todo) == 0 { - return nil, false, nil + return todo, false +} + +// besideOf are the siblings a fetch's arguments place, as a digest echoes them: ` `, sorted. +func besideOf(fetch map[string]any) []string { + raw, err := json.Marshal(fetch["beside"]) + if err != nil { + return nil } - return todo, false, nil + var refs []struct { + Repository string `json:"repository"` + Commit string `json:"commit"` + } + _ = json.Unmarshal(raw, &refs) + var out []string + for _, r := range refs { + out = append(out, r.Repository+" "+r.Commit) + } + sort.Strings(out) + return out } // changesUnder says whether a path the merge changes is under prefix. diff --git a/go/buildstep/askrun_test.go b/go/buildstep/askrun_test.go index 34c5e26..473c2a1 100644 --- a/go/buildstep/askrun_test.go +++ b/go/buildstep/askrun_test.go @@ -109,28 +109,57 @@ func TestRunAskRefusesAnotherToolchain(t *testing.T) { } } -// 1887/A3, issue 1887 design limit 1: a check ask digests the level, takes the kept verdicts, tests the packages whose -// tests changed first, and stops at the first failure. -func TestRunAskChecksAGoLevel(t *testing.T) { +// 1887/A3, A5a, issue 1887 design limit 1: 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 stops at the first failure, running +// no later level. +func TestRunAskChecksTheLevelsOfOneCheck(t *testing.T) { r := &recorder{ fetch: map[string]any{"workspace": "/w", "changed": []string{"pkg/b/b_test.go"}}, digests: map[string]string{"x/pkg/a": "d-a", "x/pkg/b": "d-b", "x/pkg/k": "d-kept"}, tests: map[string]bool{"x/pkg/a": true, "x/pkg/b": false}, } - out, err := RunAsk(context.Background(), r.steps(), ask(delivery.CheckJob{Level: delivery.LevelUnit, Kept: true, - Runner: "workspace", RunnerWhen: "pkg/b/", Fetch: map[string]any{"repository": "novox/x", "commit": runCommit, "base": runCommit}})) + out, err := RunAsk(context.Background(), r.steps(), ask(delivery.CheckJob{Levels: []string{delivery.LevelUnit, delivery.LevelIntegration}, Kept: true, + Runner: "workspace", RunnerWhen: "modules/mesh-build-go/", Fetch: map[string]any{"repository": "novox/x", "commit": runCommit, "base": runCommit}})) if err != nil { t.Fatal(err) } - got := strings.Join(r.calls[1:], "\n") - want := `digest {"level":"unit","runner":"workspace","workspace":"/w"} -test {"level":"unit","packages":["x/pkg/b"],"runner":"workspace","workspace":"/w"}` + 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"}` if got != want { t.Fatalf("calls:\n%s\nwant:\n%s", got, want) } a := out.(delivery.CheckAnswer) - if len(a.Kept) != 1 || a.Kept[0].Package != "x/pkg/k" || a.Kept[0].Tests != 7 || len(a.Tests) != 1 || len(a.Digest) == 0 { - t.Fatalf("answer %+v", a) + if len(a.Levels) != 1 { + t.Fatalf("levels %+v, want the unit level alone", 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 || len(u.Digest) == 0 { + t.Fatalf("the unit level %+v", u) + } +} + +// ADR 0535 §4: a merge that changes the build module's own code is judged by the candidate's runner, every package run +// and nothing digested or kept; a digest that does not cover the fetch's siblings runs the level whole too. +func TestRunAskRunsWholeUnderTheRunnerOrWithoutTheSiblings(t *testing.T) { + r := &recorder{fetch: map[string]any{"workspace": "/w", "changed": []string{"modules/mesh-build-go/x.go"}}, + digests: map[string]string{"x/pkg/a": "d-kept"}, tests: map[string]bool{}} + if _, err := RunAsk(context.Background(), r.steps(), ask(delivery.CheckJob{Levels: []string{delivery.LevelUnit}, Kept: true, + Runner: "workspace", RunnerWhen: "modules/mesh-build-go/", Fetch: map[string]any{"repository": "novox/x", "commit": runCommit}})); err != nil { + t.Fatal(err) + } + if got := strings.Join(r.calls[1:], "\n"); got != `test {"level":"unit","runner":"workspace","workspace":"/w"}` { + t.Fatalf("under the runner: %s", got) + } + r = &recorder{fetch: map[string]any{"workspace": "/w"}, digests: map[string]string{"x/pkg/a": "d-kept"}, tests: map[string]bool{}} + out, err := RunAsk(context.Background(), r.steps(), ask(delivery.CheckJob{Levels: []string{delivery.LevelUnit}, Kept: true, + Fetch: map[string]any{"repository": "novox/x", "commit": runCommit, "beside": []any{map[string]any{"repository": "novox/mesh-sdk", "commit": runCommit}}}})) + if err != nil { + t.Fatal(err) + } + if l := out.(delivery.CheckAnswer).Levels[0]; len(l.Kept) != 0 || len(l.Digest) != 0 { + t.Fatalf("a digest without the siblings kept %+v", l) } } @@ -141,15 +170,15 @@ func TestRunAskChecksNodeModulesAndConflicts(t *testing.T) { tests: map[string]bool{"a": true, "b": true}} s := r.steps() s.Digest = nil - if _, err := RunAsk(context.Background(), s, ask(delivery.CheckJob{Level: delivery.LevelUnit, Fetch: map[string]any{"repository": "novox/x", "commit": runCommit}})); err != nil { + if _, err := RunAsk(context.Background(), s, ask(delivery.CheckJob{Levels: []string{delivery.LevelUnit}, Fetch: map[string]any{"repository": "novox/x", "commit": runCommit}})); err != nil { t.Fatal(err) } if fmt.Sprint(r.calls[1:]) != `[test {"level":"unit","module":"b","workspace":"/w"} test {"level":"unit","module":"a","workspace":"/w"}]` { t.Fatalf("calls %v", r.calls) } r = &recorder{fetch: map[string]any{"workspace": "/w", "conflicts": []string{"go.mod"}}} - out, err := RunAsk(context.Background(), r.steps(), ask(delivery.CheckJob{Level: delivery.LevelUnit, Fetch: map[string]any{"repository": "novox/x", "commit": runCommit}})) - if err != nil || len(r.calls) != 1 || len(out.(delivery.CheckAnswer).Tests) != 0 { + out, err := RunAsk(context.Background(), r.steps(), ask(delivery.CheckJob{Levels: []string{delivery.LevelUnit}, Fetch: map[string]any{"repository": "novox/x", "commit": runCommit}})) + if err != nil || len(r.calls) != 1 || len(out.(delivery.CheckAnswer).Levels) != 0 { t.Fatalf("a conflict: %v %v", err, r.calls) } } diff --git a/go/buildstep/pull_test.go b/go/buildstep/pull_test.go index e0d7af6..10fdedb 100644 --- a/go/buildstep/pull_test.go +++ b/go/buildstep/pull_test.go @@ -46,7 +46,7 @@ func testAsk(kind string) delivery.StepAsk { b.Fetch = fetch a.Build = &b } else { - a.Check = &delivery.CheckJob{Fetch: fetch, Level: delivery.LevelUnit} + a.Check = &delivery.CheckJob{Fetch: fetch, Levels: []string{delivery.LevelUnit}} } return a } diff --git a/go/delivery/stepasks.go b/go/delivery/stepasks.go index f7ef11f..5cda634 100644 --- a/go/delivery/stepasks.go +++ b/go/delivery/stepasks.go @@ -183,21 +183,24 @@ type AskArtifact struct { From string `json:"from,omitempty"` } -// CheckJob is a check ask's job: fetch the head merged onto its trunk, digest the level's packages, take the kept -// verdicts for the digests that have one, and test the rest at Level, the packages the change's own tests are in first -// (CheckAnswer). +// 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). 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"` - Level string `json:"level"` + // Levels are run in this order (LevelUnit, LevelIntegration); a try may name one. + Levels []string `json:"levels"` // Kept says the build module takes the kept verdicts of the checker's kept bucket (CheckerKeptBucket, by KeptKey) - // for the packages whose digest has one, and tests only the others. A try that runs some tests (Run) keeps nothing. + // for the packages whose digest has one and covers the siblings the fetch placed, and tests only the others (issue + // 1887's design limit 1). A try that runs some tests (Run), and a level the candidate's own build module judges + // (Runner), keep nothing. Kept bool `json:"kept,omitempty"` // Runner is the test step's runner ("workspace": the candidate's own build module judges it, ADR 0535 §4), used // when the merge changes a path under RunnerWhen (every merge when RunnerWhen is empty). Runner string `json:"runner,omitempty"` RunnerWhen string `json:"runner_when,omitempty"` - // Run is a try's test pattern (go test -run): the level runs it alone, digesting nothing. + // Run is a try's test pattern (go test -run): each level runs it alone, digesting nothing. Run string `json:"run,omitempty"` } @@ -259,8 +262,13 @@ func (b BuildJob) check(id string) error { } func (c CheckJob) check(id string) error { - if err := checkLevel(c.Level); err != nil { - return err + if len(c.Levels) == 0 { + return fmt.Errorf("delivery: the check ask %s names no level", id) + } + for _, l := range c.Levels { + if err := checkLevel(l); err != nil { + return err + } } return checkFetchArgs(id, c.Fetch) } @@ -327,11 +335,18 @@ type AskBuilt struct { Reference string `json:"reference,omitempty"` } -// CheckAnswer is a check ask's step result, each part the build module's own tool's answer whole: the fetch's, the -// digest's (absent when nothing was digested), the kept verdicts taken, and one test result per test step run, in the -// order run. The checker judges and keeps from it as it did from the tools' answers. +// 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 +// did from the tools' answers. type CheckAnswer struct { - Fetch json.RawMessage `json:"fetch"` + Fetch json.RawMessage `json:"fetch"` + Levels []LevelAnswer `json:"levels,omitempty"` +} + +// LevelAnswer is one level of a check ask: the digest's answer (absent when nothing was digested: the level ran whole and +// keeps nothing), the kept verdicts taken, and one test result per test step run, in the order run. +type LevelAnswer struct { + Level string `json:"level"` Digest json.RawMessage `json:"digest,omitempty"` Kept []KeptTaken `json:"kept,omitempty"` Tests []json.RawMessage `json:"tests,omitempty"` diff --git a/go/delivery/stepasks_test.go b/go/delivery/stepasks_test.go index 1cf1ad6..9ac39a5 100644 --- a/go/delivery/stepasks_test.go +++ b/go/delivery/stepasks_test.go @@ -23,7 +23,7 @@ func checkAsk() StepAsk { return StepAsk{ID: "mh3k2x9qa1b2c4", Kind: AskCheck, Language: AskNode, Asker: "mesh-delivery-checker", At: time.Date(2026, 10, 23, 13, 0, 0, 0, time.UTC), Check: &CheckJob{Fetch: map[string]any{"repository": "novox/mesh-catalog", "commit": askCommit, "ref": "refs/pull/3/head", "base": askCommit}, - Level: LevelUnit, Kept: true, Runner: "workspace", RunnerWhen: "modules/mesh-build-go/", Run: ""}} + Levels: []string{LevelUnit, LevelIntegration}, Kept: true, Runner: "workspace", RunnerWhen: "modules/mesh-build-go/", Run: ""}} } // 1887/A1a: an ask encodes and decodes with every field. @@ -51,7 +51,7 @@ func TestAskRefusesUnknownKindAndLanguage(t *testing.T) { "language": func(a *StepAsk) { a.Language = "typescript" }, "id": func(a *StepAsk) { a.ID = "postgres" }, "neither": func(a *StepAsk) { a.Build = nil }, - "both": func(a *StepAsk) { a.Check = &CheckJob{Level: LevelUnit, Fetch: a.Build.Fetch} }, + "both": func(a *StepAsk) { a.Check = &CheckJob{Levels: []string{LevelUnit}, Fetch: a.Build.Fetch} }, "recheck": func(a *StepAsk) { a.Kind = AskRecheck }, "commit": func(a *StepAsk) { a.Build.Fetch = map[string]any{"repository": "novox/x", "commit": "main"} }, "path": func(a *StepAsk) { a.Build.Path = "" },