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

mesh-delivery-checker: write a member's preview whenever its fetch merged, and do not wait on a composition that cannot come

Browse Source 
Review of 650a1d0: a member that only failed its tests got no preview, so the indexer held no manifest
of it, the assigner's composition only said it waits, and the check waited for ever. The preview is now
written whenever every build module's fetch merged; a member with no preview of a merge (a conflict, a
fetch that did not answer) makes the composition not run, said with the members it names, unawaited.

Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL
This commit is contained in:
jschoubben
2026-11-02 18:56:19 +01:00
parent 290e1696b2
commit fa85de8ef6
3 changed files with 104 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/mesh-delivery-checker/internal/check/composition_test.go
+8 -1
View File
@@ -14,10 +14,17 @@ type fakeCompositions struct {
v delivery.CompositionVerdict
err error
asked []string
// waits answers only that the composition waits, as the assigner does for a member the indexer holds no manifest of:
// Await returns only when its context ends.
waits bool
}
func (f *fakeCompositions) Await(_ context.Context, key string) (delivery.CompositionVerdict, error) {
func (f *fakeCompositions) Await(ctx context.Context, key string) (delivery.CompositionVerdict, error) {
f.asked = append(f.asked, key)
if f.waits {
<-ctx.Done()
return delivery.CompositionVerdict{}, ctx.Err()
}
return f.v, f.err
}
modules/mesh-delivery-checker/internal/check/examiner_test.go
+76 -55
View File
@@ -10,6 +10,7 @@ import (
"os"
"path/filepath"
"reflect"
"slices"
"sort"
"strings"
"sync"
@@ -433,66 +434,86 @@ func TestANodePackageIsNeverKeptAndRunsEveryCheck(t *testing.T) { // 1288/AC4, 1
}
// The operator, 2026-11-02 ("A failed check shows only its first fault"): a check runs every member, every level and the
// composition to its end and its verdict names every failure: a member's head that does not merge, the other member's
// failing test at the unit level and again at the integration level, and the failed composition, in the order judged.
// Failed keeps the first whole; the change's own tests still run first (issue 1289), and the rest of the level after them.
// composition to its end and its verdict names every failure in the order judged, Failed keeping the first whole; the
// 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).
func TestEveryFailureOfEveryMemberLevelAndTheCompositionIsNamed(t *testing.T) {
b := newBench(t)
b.ex.Compositions = &fakeCompositions{v: delivery.CompositionVerdict{Group: "feat/x", Heads: "0123456789ab", State: delivery.CompositionFail, SDK: "x",
Machine: "g14", Words: "route-proxy is defined twice"}}
b.fetch.Changed = []string{"modules/m/internal/b/b_test.go"}
b.digest[delivery.LevelUnit] = Digested{Level: "unit", Packages: map[string]string{"m/internal/a": digA, "m/internal/b": digB}}
conflict := captured[Fetched](t, "go-fetch-conflict")
b.steps.handlers["build_go_fetch"] = func(c stepCall) (any, error) {
if c.args["repository"] == repoSDK {
return conflict, nil
for _, conflicts := range []bool{false, true} {
b := newBench(t)
comps := &fakeCompositions{v: delivery.CompositionVerdict{Group: "feat/x", Heads: "0123456789ab", State: delivery.CompositionFail, SDK: "x",
Machine: "g14", Words: "route-proxy is defined twice"}, waits: conflicts}
b.ex.Compositions = comps
b.fetch.Changed = []string{"modules/m/internal/b/b_test.go"}
b.digest[delivery.LevelUnit] = Digested{Level: "unit", Packages: map[string]string{"m/internal/a": digA, "m/internal/b": digB}}
conflict := captured[Fetched](t, "go-fetch-conflict")
b.steps.handlers["build_go_fetch"] = func(c stepCall) (any, error) {
if conflicts && c.args["repository"] == repoSDK {
return conflict, nil
}
return b.fetch, nil
}
return b.fetch, nil
}
failing := captured[Tested](t, "go-test-failing")
b.test = func(c stepCall) (any, error) {
if got := packagesOf(c); c.args["level"] == "integration" || (len(got) == 1 && got[0] == "m/internal/b") {
r := failing
r.Level = c.args["level"].(string)
return r, nil
failing := captured[Tested](t, "go-test-failing")
b.test = func(c stepCall) (any, error) {
if got := packagesOf(c); c.args["level"] == "integration" || (len(got) == 1 && got[0] == "m/internal/b") {
r := failing
r.Level = c.args["level"].(string)
return r, nil
}
return captured[Tested](t, "go-test-slow"), nil
}
return captured[Tested](t, "go-test-slow"), nil
}
v := b.judge(t, caseOf(t, pullOf(repoA, 7, head1), pullOf(repoSDK, 12, head2))).Verdict
var got []string
for _, f := range v.Failures {
got = append(got, firstFailed(delivery.Verdict{Failed: f}))
}
want := []string{"shanks: novox/mesh-sdk#12 does not merge into the trunk: files in conflict: a.go", "unit: fixture/failing/bad.TestThreeIsFour",
"integration: fixture/failing/bad.TestThreeIsFour", "g14: route-proxy is defined twice"}
if v.State != delivery.VerdictFail || len(got) != len(want) {
t.Fatalf("verdict %+v: failures\n%s\nwant\n%s", v, strings.Join(got, "\n"), strings.Join(want, "\n"))
}
for i := range want {
if !strings.HasPrefix(got[i], want[i]) {
t.Errorf("failure %d is %q, want %q", i, got[i], want[i])
cs := caseOf(t, pullOf(repoA, 7, head1), pullOf(repoSDK, 12, head2))
done := make(chan delivery.Verdict, 1)
go func() { done <- b.judge(t, cs).Verdict }()
var v delivery.Verdict
select {
case v = <-done:
case <-time.After(5 * time.Second):
t.Fatalf("conflicts %v: the check waits on a composition that only waits", conflicts)
}
}
if v.Failed.Machine != "shanks" || !strings.Contains(v.Failed.Words, "files in conflict") || v.Composition == nil || v.Composition.State != delivery.CompositionFail {
t.Errorf("failed %+v, composition %+v", v.Failed, v.Composition)
}
var unit []string
for _, c := range b.steps.tool("test") {
if c.args["level"] == "unit" {
unit = append(unit, strings.Join(packagesOf(c), ","))
var got []string
for _, f := range v.Failures {
got = append(got, firstFailed(delivery.Verdict{Failed: f}))
}
tests := []string{"unit: fixture/failing/bad.TestThreeIsFour", "integration: fixture/failing/bad.TestThreeIsFour"}
want := append(append(slices.Clone(tests), tests...), "g14: route-proxy is defined twice") // both members fail their tests
if conflicts {
want = append([]string{"shanks: novox/mesh-sdk#12 does not merge into the trunk: files in conflict: a.go"}, tests...)
}
if v.State != delivery.VerdictFail || len(got) != len(want) {
t.Fatalf("conflicts %v: verdict %+v: failures\n%s\nwant\n%s", conflicts, v, strings.Join(got, "\n"), strings.Join(want, "\n"))
}
for i := range want {
if !strings.HasPrefix(got[i], want[i]) {
t.Errorf("conflicts %v: failure %d is %q, want %q", conflicts, i, got[i], want[i])
}
}
keys, _ := b.prev.Keys(context.Background())
switch {
case !conflicts && (len(keys) != 2 || v.Composition == nil || v.Composition.State != delivery.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 != delivery.CompositionNotRun ||
!strings.Contains(v.Composition.Words, "novox/mesh-sdk#12")):
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
for _, c := range b.steps.tool("test") {
if c.args["level"] == "unit" && c.args["workspace"] != nil {
unit = append(unit, strings.Join(packagesOf(c), ","))
}
}
if conflicts && (strings.Join(unit, " | ") != "m/internal/b | m/internal/a" || len(b.steps.tool("digest")) != 2) {
t.Errorf("the unit level ran %v and %d digests, want the changed test's package, then the rest, at both levels", unit, len(b.steps.tool("digest")))
}
v.Group, v.Heads = "feat/x", "0123456789ab"
if _, err := delivery.EncodeVerdict(v); err != nil {
t.Errorf("conflicts %v: the verdict cannot be written: %v", conflicts, err)
}
if line := Line(v, cs, cs.Pulls[0], nil, nil, nil); !strings.HasPrefix(line, fmt.Sprintf("failed (%d): %s", len(want), want[0][:20])) ||
!strings.Contains(line, fmt.Sprintf("; and %d more | ", len(want)-1)) || utf8.RuneCountInString(line) > MaxLine {
t.Errorf("conflicts %v: line %q", conflicts, line)
}
}
if strings.Join(unit, " | ") != "m/internal/b | m/internal/a" || len(b.steps.tool("digest")) != 2 {
t.Errorf("the unit level ran %v and %d digests, want the changed test's package, then the rest, at both levels", unit, len(b.steps.tool("digest")))
}
v.Group, v.Heads = "feat/x", "0123456789ab"
if _, err := delivery.EncodeVerdict(v); err != nil {
t.Errorf("the verdict cannot be written: %v", err)
}
cs := caseOf(t, pullOf(repoA, 7, head1), pullOf(repoSDK, 12, head2))
if line := Line(v, cs, cs.Pulls[0], nil, nil, nil); !strings.HasPrefix(line, "failed (4): shanks: novox/mesh-sdk#12 does not merge") ||
!strings.Contains(line, "; and 3 more | ") || utf8.RuneCountInString(line) > MaxLine {
t.Errorf("line %q", line)
}
}
modules/mesh-delivery-checker/internal/check/judge.go
+20 -5
View File
@@ -232,6 +232,9 @@ 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 []string
}
// Judge runs the case (it is the Judge of a Checker). A failure of the change, of a merge or of a step is a verdict; an error is only a case that cannot
@@ -271,6 +274,12 @@ func (st *state) composed(ctx context.Context, v *delivery.Verdict) {
if st.j.Compositions == nil || st.c.Try != nil {
return
}
if len(st.unpreviewed) > 0 {
// The assigner would only say it waits for a manifest the indexer will not have: the check does not wait for it.
v.Composition = &delivery.Composition{State: delivery.CompositionNotRun, Words: "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"}
return
}
comp, err := st.j.Compositions.Await(ctx, st.c.Key)
switch {
case errors.Is(err, ErrAssignerNotRunning):
@@ -378,8 +387,8 @@ func (st *state) judgeAll(ctx context.Context) *delivery.Verdict {
break
}
var merged []Fetched
memberFailed := false
for _, bm := range st.j.Modules.BuildModules(p.Repository) {
modules := st.j.Modules.BuildModules(p.Repository)
for _, bm := range modules {
t := &target{pull: p, module: bm}
if tc, none := delivery.AgreedToolchain(st.j.Toolchains.Entries(buildrun.LanguageOf[bm])); none == "" {
t.toolchain = tc.Version
@@ -392,7 +401,7 @@ func (st *state) judgeAll(ctx context.Context) *delivery.Verdict {
if bm == buildrun.GoBuildModule {
beside, err := st.j.besideFor(ctx, st.c, p)
if err != nil {
failures, memberFailed = append(failures, fail(forgeMachine(err, bm), err.Error()+unheardWords(err))), true
failures = append(failures, fail(forgeMachine(err, bm), err.Error()+unheardWords(err)))
continue
}
if len(beside) > 0 {
@@ -407,11 +416,17 @@ func (st *state) judgeAll(ctx context.Context) *delivery.Verdict {
t.fetchArgs = args
st.targets = append(st.targets, t)
if v := st.ask(ctx, t, &merged); v != nil {
failures, memberFailed = append(failures, v), true
failures = append(failures, v)
}
}
if st.c.Try == nil && !memberFailed { // a try owes the indexer no preview: no pull request holds its head
// 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).
switch {
case st.c.Try != nil: // a try owes the indexer no preview: no pull request holds its head
case len(merged) == len(modules):
st.j.writePreview(ctx, p, merged)
default:
st.unpreviewed = append(st.unpreviewed, pullKey(p))
}
st.judgedByMember(p, merged)
if p.Repository == SDKRepository && len(st.c.Pulls) > 1 && st.j.changesPackage(p, merged) {