Run the levels of one check in one ask after one fetch, keeping nothing under the candidate's runner or a digest without the siblings, as ADR 0525 §3 says a check ask is (issue 1887 A5a)
This commit is contained in:
+69
-33
@@ -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: `<owner/repo> <commit>`, 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.
|
||||
|
||||
+42
-13
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
+27
-12
@@ -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"`
|
||||
|
||||
@@ -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 = "" },
|
||||
|
||||
Reference in New Issue
Block a user