From 69577f739d073d8a160ade678ae55ccc4a171d43 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 16:11:32 +0200 Subject: [PATCH] =?UTF-8?q?Judge=20hq=20as=20any=20repository,=20so=20its?= =?UTF-8?q?=20pull=20requests=20get=20a=20verdict=20and=20mesh/check=20fro?= =?UTF-8?q?m=20its=20Go=20tests=20and=20no=20script=20judges=20it=20(issue?= =?UTF-8?q?=201887=20A10,=20ADR=200527=20=C2=A72)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../cmd/mesh-delivery-checker-tools/main.go | 5 ++-- .../mesh-delivery-checker/integration_test.go | 21 +++++++++++---- .../internal/check/case.go | 15 +++-------- .../internal/check/checker.go | 2 +- .../internal/check/checker_test.go | 26 ++++++++++++------- .../internal/check/gate_test.go | 4 +-- .../internal/check/read.go | 20 ++------------ .../internal/check/read_test.go | 21 ++++++++------- .../internal/check/try.go | 8 +++--- .../internal/check/try_test.go | 4 +-- 10 files changed, 60 insertions(+), 66 deletions(-) diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go index 700988d..c43f8cc 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go @@ -50,7 +50,7 @@ func TryTools(tr Trying) []stdio.Tool { Description: "Try a pushed head before its pull request is opened: the checker judges it as it judges a pull request's head, merged onto its trunk's head, " + "its unit and integration levels through the build modules, and owes the forge nothing. Answers the try's id at once; " + check.TryReadTool + " reads " + "where it stands. A try of the same head that waits or runs is the answer to a second ask; at most 4 wait or run at once, mesh-wide. A repository " + - "the checker checks no level of (hq, one whose trunk the gate does not name) is refused.", + "the checker checks no level of (one whose trunk the gate does not name) is refused.", Input: map[string]any{"type": "object", "required": []string{"repository", "head"}, "properties": map[string]any{ "repository": map[string]any{"type": "string", "description": "/"}, "head": map[string]any{"type": "string", "description": "the full commit to try, 40 or 64 hex digits"}, @@ -104,8 +104,7 @@ func Tools(r check.Reader) []stdio.Tool { return map[string]any{"verdicts": vs}, err }}, {Name: check.PullReadTool, - Description: "The verdict that judges one pull request and the full report of its status mesh/check; for an hq pull request, that hq is judged by " + - "merge-check.sh until issue 1296. Read-only.", + Description: "The verdict that judges one pull request and the full report of its status mesh/check. Read-only.", Input: map[string]any{"type": "object", "required": []string{"repository", "number"}, "properties": map[string]any{ "repository": map[string]any{"type": "string", "description": "/"}, "number": map[string]any{"type": "integer", "description": "the pull request's number"}}}, diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go index 632bbf1..b7611e1 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go @@ -531,14 +531,25 @@ func TestAHeadThatDoesNotMergeFailsNamingTheFilesAndRunsNoTest(t *testing.T) { / // TestAnHqPullRequestGetsNoVerdictAndAnotherDoes: hq's pull request is judged by no one (ADR 0484 clause 8), the other one in // the same run is. -func TestAnHqPullRequestGetsNoVerdictAndAnotherDoes(t *testing.T) { // ADR 0484 clause 8 +// 1887/A10, ADR 0527 §2: a pull request into hq gets a verdict and a mesh/check from the checker, as any repository's. +func TestAnHqPullRequestGetsAVerdictAndAStatus(t *testing.T) { b := startBed(t) + b.setTrunk(t, map[string]string{repo: trunk, "novox/hq": trunk}) hq := b.candidate(t, "feat/hq", pull("novox/hq", 3, head1)) - other := b.candidate(t, "feat/x", pull(repo, 7, head2)) b.start(t, nil) - b.verdict(t, other, ended) - if _, err := b.buckets[delivery.CheckerVerdictsBucket].Get(b.ctx, hq); err == nil { - t.Error("hq's pull request has a verdict") + if v := b.verdict(t, hq, ended); v.State != delivery.VerdictPass { + t.Fatalf("hq's verdict = %+v", v) + } + key, _ := delivery.CheckerOwedKey("novox/hq", 3, head1) + for deadline := time.Now().Add(20 * time.Second); ; time.Sleep(50 * time.Millisecond) { + if e, err := b.buckets[delivery.CheckerOwedBucket].Get(b.ctx, key); err == nil { + if o, err := delivery.DecodeCheckerOwed(e.Value()); err == nil && o.State == "success" { + return + } + } + if time.Now().After(deadline) { + t.Fatalf("hq#3 owes no success within 20 s") + } } } diff --git a/modules/mesh-delivery-checker/internal/check/case.go b/modules/mesh-delivery-checker/internal/check/case.go index e7e8654..22850d4 100644 --- a/modules/mesh-delivery-checker/internal/check/case.go +++ b/modules/mesh-delivery-checker/internal/check/case.go @@ -11,13 +11,6 @@ import ( "git.novox.be/novox/mesh-sdk/go/delivery" ) -// HQRepository is the repository the dark checker writes no status for (ADR 0484 clause 8): hq builds no module and its -// check is a script no build module can run, so it is judged by its merge-check.sh until issue 1296 settles how. -const HQRepository = "novox/hq" - -// HQNote is what the read tools say of an hq pull request (ADR 0484 clause 8). -const HQNote = "hq: judged by merge-check.sh until issue 1296" - // Pull is one pull request the checker judges: its head, the trunk head of its repository it is merged onto and the // modules the indexer says it touches, with the fingerprints the indexer wrote for them. type Pull struct { @@ -29,8 +22,8 @@ type Pull struct { Fps map[string]delivery.CandidateModule } -// Case is one check: a candidate's pull requests that are judged (those of hq and of a repository whose trunk is not known -// are left out), the trunk head of each repository and the key of the verdict. +// Case is one check: a candidate's pull requests that are judged (those of a repository whose trunk is not known are left +// out; hq is judged as any repository since R4g, its checks Go tests: ADR 0527), the trunk head of each repository and the key of the verdict. type Case struct { // Key is the verdict key, `.`. Key string @@ -41,7 +34,7 @@ type Case struct { Pulls []Pull // Trunk is the trunk head of each repository of Pulls. Trunk map[string]string - // Left says why pull requests of the candidate are not judged (hq's, a repository with no known trunk); empty when all are. + // Left says why pull requests of the candidate are not judged (a repository with no known trunk); empty when all are. Left []string // Queued is when the checker took the case up, for the time of its queue. Queued time.Time @@ -86,8 +79,6 @@ func CaseOf(c delivery.Candidate, trunk map[string]string) *Case { cs := &Case{Key: key, Group: c.Group, HeadsHash: c.HeadsHash, Heads: GateHeads(c), Trunk: map[string]string{}, Progress: &Progress{}} for _, p := range c.Pulls { switch head, known := trunk[p.Repository]; { - case p.Repository == HQRepository: - cs.Left = append(cs.Left, fmt.Sprintf("%s#%d: %s", p.Repository, p.Number, HQNote)) case !known || head == "": cs.Left = append(cs.Left, fmt.Sprintf("%s#%d: the trunk head of %s is not known", p.Repository, p.Number, p.Repository)) default: diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 2b92cc3..577ed0e 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -174,7 +174,7 @@ func (ch *Checker) heads() map[string]string { func (ch *Checker) consider(ctx context.Context, key string, c delivery.Candidate) { cs := CaseOf(c, ch.heads()) if cs == nil { - return // nothing to judge: hq's pull requests, or no trunk known + return // nothing to judge: no trunk known } cs.Queued = ch.now() ch.mu.Lock() diff --git a/modules/mesh-delivery-checker/internal/check/checker_test.go b/modules/mesh-delivery-checker/internal/check/checker_test.go index d367ba6..1184a1b 100644 --- a/modules/mesh-delivery-checker/internal/check/checker_test.go +++ b/modules/mesh-delivery-checker/internal/check/checker_test.go @@ -11,6 +11,8 @@ import ( "git.novox.be/novox/mesh-sdk/go/delivery" ) +const hqRepo = "novox/hq" + const ( repoA = "novox/mesh-delivery" repoSDK = "novox/mesh-sdk" @@ -234,10 +236,9 @@ func TestNoQualifyingNodeFailsWithTheReasonAndIsJudgedAgainWhenTheNodesChange(t } } -func TestHqAndARepositoryWithNoTrunkGetNoVerdict(t *testing.T) { // N6/AC6, ADR 0484 clauses 8 and 11 +func TestARepositoryWithNoTrunkGetsNoVerdict(t *testing.T) { // N6/AC6, ADR 0484 clause 11 ctx, j := context.Background(), passJudge() r := newRig(t, j) - r.ch.Candidate(ctx, "feat/hq", candidateOf(t, "feat/hq", pullOf(HQRepository, 3, head1))) r.ch.Candidate(ctx, "feat/y", candidateOf(t, "feat/y", pullOf("novox/unknown", 4, head1))) r.ch.Wait() if j.count() != 0 || len(r.verdicts.vals) != 0 || len(r.owed.vals) != 0 { @@ -245,17 +246,24 @@ func TestHqAndARepositoryWithNoTrunkGetNoVerdict(t *testing.T) { // N6/AC6, ADR } } -func TestAGroupWithHqJudgesItsOtherMembersAndWritesNoStatusForHq(t *testing.T) { // ADR 0484 clause 8 +// 1887/A10, ADR 0527 §2: hq is judged as any repository, its checks Go tests, and its pull request gets a verdict and a +// mesh/check; in a group, hq's member is judged with the others. +func TestAnHqPullRequestGetsAVerdictAndAStatus(t *testing.T) { ctx, j := context.Background(), passJudge() r := newRig(t, j) - r.ch.Candidate(ctx, "feat/x", candidateOf(t, "feat/x", pullOf(HQRepository, 3, head1), pullOf(repoA, 7, head2))) + r.setTrunks(t, map[string]string{repoA: trunkA, repoSDK: trunkS, hqRepo: trunkA2}) + r.ch.Candidate(ctx, "feat/hq", candidateOf(t, "feat/hq", pullOf(hqRepo, 3, head1))) r.ch.Wait() - if j.count() != 1 { - t.Fatalf("judged %d times, want once", j.count()) + if j.count() != 1 || len(r.verdicts.vals) != 1 { + t.Fatalf("judged %d times, %d verdicts, want one", j.count(), len(r.verdicts.vals)) } - keys, _ := r.owed.Keys(ctx) - if len(keys) != 1 || !strings.HasPrefix(keys[0], "novox/mesh-delivery.7.") && !strings.Contains(keys[0], "mesh-delivery") { - t.Errorf("statuses %v, want one for mesh-delivery#7 only", keys) + if s := owedState(t, r, hqRepo, 3, head1); s != "success" { + t.Errorf("hq#3 owes %q, want success", s) + } + r.ch.Candidate(ctx, "feat/x", candidateOf(t, "feat/x", pullOf(hqRepo, 4, head2), pullOf(repoA, 7, head2))) + r.ch.Wait() + if s := owedState(t, r, hqRepo, 4, head2); s != "success" { + t.Errorf("hq#4 in a group owes %q, want success", s) } } diff --git a/modules/mesh-delivery-checker/internal/check/gate_test.go b/modules/mesh-delivery-checker/internal/check/gate_test.go index fcb4f62..c66168e 100644 --- a/modules/mesh-delivery-checker/internal/check/gate_test.go +++ b/modules/mesh-delivery-checker/internal/check/gate_test.go @@ -47,11 +47,11 @@ func TestTheVerdictsHeadsAreTheGatesHashOverLowercaseHeads(t *testing.T) { // ga func TestAGroupsVerdictHeadsCoverEveryMember(t *testing.T) { // gate (b): hq's member too, as the gate's ask covers it ctx := context.Background() r := newRig(t, passJudge()) - cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1), pullOf(repoSDK, 12, head2), pullOf(HQRepository, 3, head2)) + cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1), pullOf(repoSDK, 12, head2), pullOf(hqRepo, 3, head2)) r.ch.Candidate(ctx, "feat%2Fx", cand) r.ch.Wait() want, _ := delivery.HeadsHash([]delivery.Head{{Repository: repoA, Number: 7, Head: head1}, {Repository: repoSDK, Number: 12, Head: head2}, - {Repository: HQRepository, Number: 3, Head: head2}}) + {Repository: hqRepo, Number: 3, Head: head2}}) if v := r.verdict(t, keyOf(t, "feat/x", cand)); v.Heads != want { t.Errorf("heads = %q, want %q", v.Heads, want) } diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index e87de2e..75db589 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -122,8 +122,8 @@ func (r Reader) Verdicts(group string) ([]VerdictRead, error) { return out, nil } -// Pull answers for one pull request: the verdict of the candidate that holds it, with its report; for an hq pull request, -// that hq is judged by merge-check.sh until issue 1296 (HQNote); for one the indexer holds in no candidate, that none does. +// Pull answers for one pull request: the verdict of the candidate that holds it, with its report; for one the indexer holds +// in no candidate, that none does. func (r Reader) Pull(repository string, number int) (VerdictRead, error) { if r.CandidateState.Keys == nil || r.CandidateState.Get == nil { return VerdictRead{}, fmt.Errorf("the candidates state cannot be read here") @@ -146,9 +146,6 @@ func (r Reader) Pull(repository string, number int) (VerdictRead, error) { if p.Repository != repository || p.Number != number { continue } - if repository == HQRepository { - return VerdictRead{Key: k, Note: HQNote}, nil - } vkey, err := delivery.CheckerVerdictKey(c.Group, c.HeadsHash) if err != nil { return VerdictRead{}, err @@ -360,9 +357,6 @@ func (r Reader) Status() (Status, error) { if err != nil { continue } - if !checksAny(c) { - continue // hq's pull requests alone: merge-check.sh judges them, the checker never does - } raw, err = r.VerdictState.Get(vkey) if err != nil { continue @@ -419,13 +413,3 @@ func (r Reader) leftWhy(module string, step delivery.Step, now time.Time, bound } return "" } - -// checksAny says whether a candidate has a pull request the checker judges: one outside hq. -func checksAny(c delivery.Candidate) bool { - for _, p := range c.Pulls { - if p.Repository != HQRepository { - return true - } - } - return false -} diff --git a/modules/mesh-delivery-checker/internal/check/read_test.go b/modules/mesh-delivery-checker/internal/check/read_test.go index 313b218..ecd4109 100644 --- a/modules/mesh-delivery-checker/internal/check/read_test.go +++ b/modules/mesh-delivery-checker/internal/check/read_test.go @@ -34,7 +34,7 @@ func (f fakeState) read() StateRead { func readerOf(t *testing.T) (Reader, string) { t.Helper() r := newRig(t, passJudge()) - cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1), pullOf(HQRepository, 3, head2)) + cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1), pullOf(hqRepo, 3, head2)) r.ch.Candidate(context.Background(), "feat%2Fx", cand) r.ch.Wait() ckey, _ := delivery.CandidatesKey("feat/x") @@ -66,10 +66,12 @@ func TestTheVerdictToolsAnswerTheStoredVerdictWithTheFullReportOfEachPullRequest } } -func TestAnHqPullRequestIsSaidToBeJudgedByMergeCheckUntilIssue1296(t *testing.T) { // ADR 0484 clause 8 +// 1887/A10: an hq pull request is answered as any other, its report its verdict's (ADR 0527 §2), never a note that a script +// judges it. +func TestAnHqPullRequestIsAnsweredAsAnyOther(t *testing.T) { rd, _ := readerOf(t) - hq, err := rd.Pull(HQRepository, 3) - if err != nil || hq.Note != "hq: judged by merge-check.sh until issue 1296" || hq.Verdict != nil { + hq, err := rd.Pull(hqRepo, 3) + if err != nil || hq.Note != "" || hq.Verdict == nil { t.Errorf("hq pull = %+v, %v", hq, err) } p, err := rd.Pull(repoA, 7) @@ -219,16 +221,15 @@ func TestTheStatusCountsOnlyStepsThatCanStillRunAndSaysWhyTheOthersAreLeft(t *te } } -// 1890: a group whose pull requests are all hq's is never judged by the checker (merge-check.sh judges hq), so it is not -// a candidate held back. -func TestTheStatusDoesNotCountAnHQOnlyGroupAsUnjudged(t *testing.T) { // 1890/AC4 - cand := candidateOf(t, "pb-x", pullOf(HQRepository, 3, head2)) +// 1887/A10: a group of hq's pull requests alone is judged by the checker since R4g, so without a verdict it is unjudged. +func TestTheStatusCountsAnHQOnlyGroupAsUnjudged(t *testing.T) { + cand := candidateOf(t, "pb-x", pullOf(hqRepo, 3, head2)) ckey, _ := delivery.CandidatesKey("pb-x") st, err := Reader{CandidateState: fakeState{ckey: cand}.read(), VerdictState: fakeState{}.read()}.Status() if err != nil { t.Fatal(err) } - if len(st.Unjudged) != 0 { - t.Errorf("unjudged = %v", st.Unjudged) + if len(st.Unjudged) != 1 || st.Unjudged[0] != "pb-x" { + t.Errorf("unjudged = %v, want pb-x", st.Unjudged) } } diff --git a/modules/mesh-delivery-checker/internal/check/try.go b/modules/mesh-delivery-checker/internal/check/try.go index 41024bc..55bcaab 100644 --- a/modules/mesh-delivery-checker/internal/check/try.go +++ b/modules/mesh-delivery-checker/internal/check/try.go @@ -108,15 +108,15 @@ type TryStore interface { Create(key string, value []byte) error } -// NotChecked is the error of a try of a repository the checker judges no level of: hq, judged by merge-check.sh (ADR 0484 -// clause 8), and a repository whose trunk the gate's source does not name. +// NotChecked is the error of a try of a repository the checker judges no level of: one whose trunk the gate's source does +// not name. func NotChecked(repository string) error { return fmt.Errorf("no level of %s is checked by the checker", repository) } // AskTry writes a new try, queued, and answers its id: a try of the same head that waits or runs is the answer to a second // ask, and at most MaxTries wait or run at once. trunks are the gate's trunk heads of every repository (source.main): a -// repository with none, and hq, are no repository the checker checks. A trunk asked by name is its trunk head now. +// repository with none is no repository the checker checks. A trunk asked by name is its trunk head now. func AskTry(store TryStore, trunks map[string]string, a TryAsk, now time.Time) (string, error) { repo := strings.ToLower(strings.TrimSuffix(strings.TrimSpace(a.Repository), ".git")) head := strings.ToLower(strings.TrimSpace(a.Head)) @@ -136,7 +136,7 @@ func AskTry(store TryStore, trunks map[string]string, a TryAsk, now time.Time) ( trunkHead = h } } - if repo == HQRepository || trunkHead == "" { + if trunkHead == "" { return "", NotChecked(repo) } t := Try{Repository: repo, Head: head, Tests: a.Tests, Level: a.Level, State: TryQueued, Asked: now.UTC()} diff --git a/modules/mesh-delivery-checker/internal/check/try_test.go b/modules/mesh-delivery-checker/internal/check/try_test.go index 07105ca..ce777d6 100644 --- a/modules/mesh-delivery-checker/internal/check/try_test.go +++ b/modules/mesh-delivery-checker/internal/check/try_test.go @@ -23,7 +23,7 @@ func (m memTries) Create(key string, value []byte) error { return err } -var tryTrunks = map[string]string{repoA: trunkA, repoSDK: trunkS, HQRepository: trunkA} +var tryTrunks = map[string]string{repoA: trunkA, repoSDK: trunkS, hqRepo: trunkA} func readTry(t testing.TB, kv *memKV, id string) Try { t.Helper() @@ -67,7 +67,7 @@ func TestATryIsAskedQueuedOncePerHeadAndAtMostFourAtOnce(t *testing.T) { // ADR func TestATryOfARepositoryWithNoCheckedLevelIsRefused(t *testing.T) { // the agreed error of the try store := memTries{newMemKV()} - for _, repo := range []string{HQRepository, "novox/mesh-lab"} { + for _, repo := range []string{"novox/no-trunk", "novox/mesh-lab"} { // hq is checked since R4g (ADR 0527): see tryTrunks _, err := AskTry(store, tryTrunks, TryAsk{Repository: repo, Head: head1, Trunk: "main"}, time.Now()) if err == nil || err.Error() != "no level of "+repo+" is checked by the checker" { t.Errorf("%s: %v", repo, err)