From 45564e69a648306523d2c4d2e15b29c60aa1a1f3 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 14:01:05 +0100 Subject: [PATCH] =?UTF-8?q?A=20placed=20check=20ask=20names=20the=20packag?= =?UTF-8?q?es=20it=20tests,=20so=20the=20first=20ask's=20other=20packages?= =?UTF-8?q?=20are=20not=20run=20or=20counted=20again=20(issue=20890,=20ADR?= =?UTF-8?q?=200552=20=C2=A74;=20review=20round=201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- go/buildstep/askrun.go | 10 ++++++++++ go/buildstep/askrun_test.go | 16 ++++++++++++++++ go/delivery/stepasks.go | 8 ++++++++ go/delivery/stepasks_test.go | 5 +++++ 4 files changed, 39 insertions(+) diff --git a/go/buildstep/askrun.go b/go/buildstep/askrun.go index 12a87e3..d189578 100644 --- a/go/buildstep/askrun.go +++ b/go/buildstep/askrun.go @@ -234,6 +234,9 @@ func (s AskSteps) level(ctx context.Context, job delivery.CheckJob, level string if !keepRunner { runner = "" // an installed module that ignores the runner judges the level as it judges any other (ADR 0535 §2) } + if whole && len(job.Packages) > 0 { + todo, whole = job.Packages, false // a placed ask tests its packages alone, whatever the digest said (ADR 0552 §4) + } for _, priority := range []bool{true, false} { args := withRunner(map[string]any{"workspace": f.Workspace, "level": level}) if job.Run != "" { @@ -310,9 +313,16 @@ func (s AskSteps) plan(ctx context.Context, job delivery.CheckJob, level string, return nil, true, runner != "" // a digest that names nothing or not the siblings keys no verdict: the level runs whole } la.Digest = &d + only := map[string]bool{} + for _, p := range job.Packages { + only[p] = true + } var todo []string for _, pd := range d.Packages { p := pd.Package + if len(only) > 0 && !only[p] { + continue // a placed ask's other packages were judged by the first ask (ADR 0552 §4) + } if job.Kept && s.Kept != nil { tests, found, err := s.Kept(ctx, level, pd.Digest) if err != nil && s.Logf != nil { diff --git a/go/buildstep/askrun_test.go b/go/buildstep/askrun_test.go index 37d3a76..19bed04 100644 --- a/go/buildstep/askrun_test.go +++ b/go/buildstep/askrun_test.go @@ -163,6 +163,22 @@ test {"level":"unit","packages":["x/pkg/a"],"workspace":"/w"}` t.Fatalf("has %v: tested x/pkg/a %v, unmet %+v, calls %v", has, tested, u.Unmet, r.calls) } } + // A placed ask naming its packages tests those alone and takes no other's kept verdict, also when the level would + // run whole (ADR 0552 §4). + for _, digests := range []map[string]string{r.digests, nil} { + r.calls, r.digests = nil, digests + steps := r.steps() + steps.Has = func(string) bool { return true } + out, err := RunAsk(context.Background(), steps, ask(delivery.CheckJob{Levels: []string{delivery.LevelUnit}, Kept: true, Packages: []string{"x/pkg/a"}, + Fetch: map[string]any{"repository": "novox/x", "commit": runCommit}})) + if err != nil { + t.Fatal(err) + } + u := out.(delivery.CheckAnswer).Levels[0] + if last := r.calls[len(r.calls)-1]; !strings.HasSuffix(last, `"packages":["x/pkg/a"],"workspace":"/w"}`) || len(u.Kept) != 0 || len(u.Tests) != 1 { + t.Fatalf("digests %v: calls %v, kept %+v: want x/pkg/a alone tested, nothing taken kept", digests != nil, r.calls, u.Kept) + } + } } // ADR 0535 §2, §4: a merge that changes the build module's own code is judged by the candidate's runner: its digest says diff --git a/go/delivery/stepasks.go b/go/delivery/stepasks.go index ad00703..c9ae243 100644 --- a/go/delivery/stepasks.go +++ b/go/delivery/stepasks.go @@ -337,6 +337,9 @@ type CheckJob struct { RunnerWhen string `json:"runner_when,omitempty"` // Run is a try's test pattern (go test -run): each level runs it alone, digesting nothing. Run string `json:"run,omitempty"` + // Packages, when set, are the only packages each level tests (issue 890, ADR 0552 §4): a placed ask names the + // packages the first ask left unmet, and the others are neither tested nor taken kept, so nothing is judged twice. + Packages []string `json:"packages,omitempty"` } func (a StepAsk) check() error { @@ -439,6 +442,11 @@ func (c CheckJob) check(id string) error { return err } } + for _, p := range c.Packages { + if p == "" || strings.ContainsAny(p, " \t\n") { + return fmt.Errorf("delivery: the check ask %s names a package %q that is no import path", id, p) + } + } return checkFetchArgs(id, c.Fetch) } diff --git a/go/delivery/stepasks_test.go b/go/delivery/stepasks_test.go index 19caa85..2f10122 100644 --- a/go/delivery/stepasks_test.go +++ b/go/delivery/stepasks_test.go @@ -83,6 +83,11 @@ func TestAskRefusesUnknownKindAndLanguage(t *testing.T) { } // Issue 890: a check names a known need and round-trips it; an unknown need is refused. c = checkAsk() + c.Check.Packages = []string{"a b"} + if _, err := EncodeStepAsk(c); err == nil { + t.Error("a package that is no import path encoded") + } + c.Check.Packages = []string{"x/pkg/a"} c.Needs = []string{"gpu"} if _, err := EncodeStepAsk(c); err == nil { t.Error("an unknown need encoded")