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

Merge pull request 'The checker previews a member whose fetch merged, whatever its later steps ended with' (#208) from fix/checker-preview-after-failed-step into main

Browse Source
This commit was merged in pull request #208.
This commit is contained in:
mesh-tools
2026-11-12 14:34:09 +00:00
parent 76db40eb6b 24d9c6cdf8
commit a5d7108631
2 changed files with 43 additions and 11 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
@@ -245,11 +245,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
}
@@ -589,9 +590,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
@@ -606,6 +609,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)
@@ -622,6 +628,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}}))
@@ -644,7 +658,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
@@ -284,11 +284,14 @@ 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
// tags are the group's tagged versions its members are built against (ADR 0607 §3), once the check heard them.
tags groupVersions
// 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
@@ -344,7 +347,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 {
@@ -425,7 +428,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
@@ -593,13 +596,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 old form (ADR 0500 §5): the group's package member packed by the Node fetch, kept only while no tagged
@@ -730,6 +736,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
}
@@ -743,6 +757,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))
}