novox/mesh-delivery
10
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

A-286: a member whose fetch merged is previewed and judged whatever its later steps ended with

Browse Source 
A check step that errors after its fetch merged ends failed with its answer (the fetch and the levels so far)
beside its error (steprun askrun.go), and the checker previews the merge from it (judge.go), so a failed level
no longer hides the manifest validator and the composition (try 2: catalog, contracts and delivery were
unpreviewed by a seatgen bootstrap failure, not a conflict). Only a head that does not merge or a fetch that
did not answer stays unpreviewed, and the check says which.

Builds: design 47 G A-286 (hq 41dd30fe)
Claude-Session: https://claude.ai/code/session_01NcbgfGKaxw984XyAPfLm4h
This commit is contained in:
jschoubben
2026-11-12 15:19:14 +01:00
parent 01fb2e13ca
commit 5fbc366f1b
4 changed files with 60 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/mesh-delivery-checker/internal/check/examiner_test.go
+19 -5
View File
@@ -244,11 +244,12 @@ func (b *bench) answer(a buildstep.StepAsk) buildstep.Step {
}
out, err := gobuild.RunAsk(context.Background(), steps, a)
step := buildstep.Step{ID: a.ID, Tool: a.Kind + " ask", Node: node, State: buildstep.StepPassed, Started: began, Ended: began.Add(2 * time.Second)}
if out != nil {
step.Result, _ = json.Marshal(out) // a failed step keeps what it answered beside its error, as the step runner does
}
if err != nil {
step.State, step.Reason = buildstep.StepFailed, err.Error()
return step
}
step.Result, _ = json.Marshal(out)
return step
}
@@ -588,9 +589,11 @@ func TestANodePackageIsNeverKeptAndRunsEveryCheck(t *testing.T) { // 1288/AC4, 1
// change's own tests still run first (issue 1289), and the rest of the level after them. A member that only failed its
// tests still gets its preview, so the composition is read and its failure named after the tests'. A member whose head
// does not merge has no preview of a merge, so the assigner's composition would only wait: the check does not wait for it
// and says why it was not run (review of 650a1d0).
// and says why it was not run (review of 650a1d0). A member whose step failed after its fetch merged is previewed from the
// fetch the failed step answered, and the composition is read (design 47 G A-286).
func TestEveryFailureOfEveryMemberLevelAndTheCompositionIsNamed(t *testing.T) {
for _, conflicts := range []bool{false, true} {
for _, kind := range []string{"tests", "conflicts", "errors"} {
conflicts, errs := kind == "conflicts", kind == "errors"
b := newBench(t)
comps := &fakeCompositions{v: refusedOn("x", "g14", "route-proxy is defined twice"), waits: conflicts}
b.ex.Compositions = comps
@@ -605,6 +608,9 @@ func TestEveryFailureOfEveryMemberLevelAndTheCompositionIsNamed(t *testing.T) {
}
failing := captured[Tested](t, "go-test-failing")
b.test = func(c stepCall) (any, error) {
if errs && c.args["level"] == "unit" {
return nil, errors.New("the seat definitions in . do not generate")
}
if got := packagesOf(c); c.args["level"] == "integration" || (len(got) == 1 && got[0] == "m/internal/b") {
r := failing
r.Level = c.args["level"].(string)
@@ -621,6 +627,14 @@ func TestEveryFailureOfEveryMemberLevelAndTheCompositionIsNamed(t *testing.T) {
case <-time.After(5 * time.Second):
t.Fatalf("conflicts %v: the check waits on a composition that only waits", conflicts)
}
if errs {
keys, _ := b.prev.Keys(context.Background())
if len(keys) != 2 || len(comps.asked) == 0 || v.Composition == nil || v.Composition.State == meshdeliverycheckerclient.CompositionNotRun {
t.Errorf("a step failed after its fetch merged: previews %v, asked %v, composition %+v, want both previews and the composition read",
keys, comps.asked, v.Composition)
}
continue
}
var got []string
for _, f := range v.Failures {
got = append(got, firstFailed(meshdeliverychecker.VerdictsByGroupHeadsValue{Failed: &FailedFirst{Test: f.Test, Machine: f.Machine, Words: f.Words}}))
@@ -643,7 +657,7 @@ func TestEveryFailureOfEveryMemberLevelAndTheCompositionIsNamed(t *testing.T) {
case !conflicts && (len(keys) != 2 || v.Composition == nil || v.Composition.State != meshdeliverycheckerclient.CompositionFail):
t.Errorf("tests alone failed: previews %v, composition %+v, want both previews and the composition read", keys, v.Composition)
case conflicts && (len(comps.asked) != 0 || v.Composition == nil || v.Composition.State != meshdeliverycheckerclient.CompositionNotRun ||
!strings.Contains(v.Composition.Words, "novox/mesh-sdk#12")):
!strings.Contains(v.Composition.Words, "novox/mesh-sdk#12 (its head does not merge)")):
t.Errorf("a member did not merge: asked %v, composition %+v, want not run naming novox/mesh-sdk#12", comps.asked, v.Composition)
}
var unit []string
modules/mesh-delivery-checker/internal/check/judge.go
+24 -6
View File
@@ -278,9 +278,12 @@ type state struct {
judgeHead string
// predefinedHead is the catalogue member's head when its merge changes predefined.json: the list is read there.
predefinedHead string
// unpreviewed are the members (pullKey) with no preview of a merge: a head that does not merge, or a fetch that did not
// answer. The indexer holds no manifest of them, so the assigner's composition would only wait: it is not awaited.
// unpreviewed are the members (pullKey) with no preview of a merge, each with which of the two it is: a head that does not
// merge, or a fetch that did not answer (design 47 G A-286). The indexer holds no manifest of them, so the assigner's
// composition would only wait: it is not awaited. A member whose fetch merged is previewed whatever its later steps ended with.
unpreviewed []string
// conflicted are the members (pullKey) whose head does not merge into its trunk.
conflicted map[string]bool
// outOfStep are, by pull request (pullKey), the generated files a level's compare step found out of step (design 26).
outOfStep map[string][]string
// noCandidate says why the case has no previewed candidate of its heads from the indexer within the bound: for a whole
@@ -336,7 +339,7 @@ func (st *state) takeTryCandidate(ctx context.Context) {
return
}
if len(st.unpreviewed) > 0 {
no("no preview of a merge of " + strings.Join(st.unpreviewed, ", ") + " (its head does not merge or its fetch did not answer)")
no("no preview of a merge of " + strings.Join(st.unpreviewed, ", "))
return
}
if st.c.Wait != nil {
@@ -417,7 +420,7 @@ func (st *state) compose(ctx context.Context) {
if len(st.unpreviewed) > 0 {
// An engine would only say it cannot read a manifest the indexer will not have: the check does not ask it.
st.composition = &rules.Composition{NotRun: "no preview of a merge of " + strings.Join(st.unpreviewed, ", ") +
" (its head does not merge or its fetch did not answer), so the change cannot be composed"}
", so the change cannot be composed"}
return
}
// ADR 0603 §10: the check composes with the candidate's builds; whether it failed already is the running rules' word
@@ -573,13 +576,16 @@ func (st *state) judgeAll(ctx context.Context) {
st.ask(ctx, t, &merged)
}
// Every build module's fetch merged (each is in merged once): the preview is written, also when the member's tests
// failed, so the indexer holds its manifests and the assigner can compose the change (review of 650a1d0).
// or a later step failed, so the indexer holds its manifests and the assigner can compose the change (review of
// 650a1d0; design 47 G A-286).
switch {
case st.narrowTry(): // a try of a level or some tests owes the indexer no preview: it judges no manifest
case len(merged) == len(modules):
st.j.writePreview(ctx, p, merged)
case st.conflicted[pullKey(p)]:
st.unpreviewed = append(st.unpreviewed, pullKey(p)+" (its head does not merge)")
default:
st.unpreviewed = append(st.unpreviewed, pullKey(p))
st.unpreviewed = append(st.unpreviewed, pullKey(p)+" (its fetch did not answer)")
}
st.judgedByMember(p, merged)
// The group's package member packed by the Node fetch (ADR 0500 §5).
@@ -708,6 +714,14 @@ func (st *state) askNeeding(ctx context.Context, t *target, merged *[]Fetched, l
st.facts = append(st.facts, rules.Step{Repository: p.Repository, Number: p.Number, Module: t.module, Node: step.Node})
st.c.Progress.Set(fmt.Sprintf("judge the check of %s#%d", p.Repository, p.Number), step.Node)
if step.State != buildstep.StepPassed {
// A step that failed after its fetch merged answers the fetch beside its error (design 47 G A-286): the merge is
// previewed from it, so its manifests are judged and the change composed whatever the step ended with.
var failed buildstep.CheckAnswer
if !placed && json.Unmarshal(step.Result, &failed) == nil && failed.Fetch.Workspace != "" && len(failed.Fetch.Conflicts) == 0 {
t.fetched, t.changed = failed.Fetch, failed.Fetch.Changed
*merged = append(*merged, t.fetched)
st.checkBinariesOf(t)
}
st.fail(step.Node, fmt.Sprintf("the check of %s#%d %s: %s", p.Repository, p.Number, step.State, step.Reason))
return
}
@@ -721,6 +735,10 @@ func (st *state) askNeeding(ctx context.Context, t *target, merged *[]Fetched, l
return
}
if len(t.fetched.Conflicts) > 0 {
if st.conflicted == nil {
st.conflicted = map[string]bool{}
}
st.conflicted[pullKey(p)] = true
if !st.narrowTry() {
st.j.writePreview(ctx, p, append(*merged, t.fetched))
}
modules/mesh-delivery-sdk/steprun/askrun.go
+3 -1
View File
@@ -321,7 +321,9 @@ func (s AskSteps) check(ctx context.Context, job sdkstep.CheckJob) (any, error)
_, err := s.level(ctx, job, level, f, runner, &la)
answer.Levels = append(answer.Levels, la)
if err != nil {
return nil, err
// The fetch merged: its answer is kept on the failed step's end, so the checker previews the merge and judges its
// manifests whatever the level ended with (design 47 G A-286).
return answer, err
}
}
return answer, nil
modules/mesh-delivery-sdk/steprun/askrun_test.go
+14
View File
@@ -176,6 +176,20 @@ test {"level":"integration","packages":["x/pkg/a"],"workspace":"/w"}`
t.Fatalf("the unit level %+v", u)
}
// Design 47 G A-286: a level that errors after the fetch merged ends the step failed with its answer all the same, the
// fetch and the levels so far beside the error, so the checker previews the merge from it.
r.calls = nil
steps := r.steps()
steps.Test = func(context.Context, map[string]any) (any, error) {
return nil, errors.New("the seat definitions in . do not generate")
}
out, err = RunAsk(context.Background(), steps, ask(sdkstep.CheckJob{Levels: []string{sdkstep.LevelUnit, sdkstep.LevelIntegration},
Fetch: sdkstep.Fetch{Repository: "novox/x", Commit: runCommit}}))
if failed, _ := out.(sdkstep.CheckAnswer); err == nil || !strings.Contains(err.Error(), "the unit level") || failed.Fetch.Workspace != "/w" ||
len(failed.Levels) != 1 || failed.Levels[0].Level != sdkstep.LevelUnit {
t.Fatalf("a level that errors: answer %+v, err %v; want the fetch and the unit level beside the error", out, err)
}
// ADR 0567 §12: a check's node names reach the digest and test tools, for the names check (a kept verdict misses when
// they change).
r.calls = nil