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

modules/mesh-delivery-checker/internal/check/examiner_test.go

@@ -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

@@ -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
+	// unpreviewed are the members (pullKey) with no preview of a merge, each with which of the two it is: a head that does not
-	// answer. The indexer holds no manifest of them, so the assigner's composition would only wait: it is not awaited.
+	// 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))
 		}