From 0edd0bca74d77674275b729d5f0bcefa464d77e9 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 2 Nov 2026 19:38:09 +0100 Subject: [PATCH] mesh-delivery-builder: fold the step-choice and step-run tests into two tables, within the test budget Review finding 5: the repository's unit tests rose from 810 to 814 (over the 500 budget); eleven test functions of buildrun become two, so the count is 805. Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL --- .../buildrun/choose_test.go | 22 ++++++++++++++----- .../buildrun/steps_test.go | 19 +++++++++++----- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/modules/mesh-delivery-builder/buildrun/choose_test.go b/modules/mesh-delivery-builder/buildrun/choose_test.go index 6e9c2bb..bb2e69a 100644 --- a/modules/mesh-delivery-builder/buildrun/choose_test.go +++ b/modules/mesh-delivery-builder/buildrun/choose_test.go @@ -19,7 +19,7 @@ func tcs(language, version string, nodes ...string) []delivery.NodeToolchain { return out } -func TestChooseTakesTheAgreedNodesWithFewestRunningStepsTiesByName(t *testing.T) { // 1607/AC4, ADR 0478 clause 3 +func chooseTakesTheAgreedNodesWithFewestRunningStepsTiesByName(t *testing.T) { // 1607/AC4, ADR 0478 clause 3 entries := tcs("go", "go1.27.1", "novox", "ace", "g14", "shanks") nodes, tc, err := ChooseNodes("go", entries, placesRunning(map[string]int{"novox": 3, "ace": 1, "g14": 1, "shanks": 0}), nil, 2) if err != nil { @@ -33,14 +33,14 @@ func TestChooseTakesTheAgreedNodesWithFewestRunningStepsTiesByName(t *testing.T) } } -func TestChooseWithOneNodeAnswersOneNode(t *testing.T) { // 1607/AC4 +func chooseWithOneNodeAnswersOneNode(t *testing.T) { // 1607/AC4 nodes, _, err := ChooseNodes("go", tcs("go", "go1.27.1", "novox"), nil, nil, 2) if err != nil || len(nodes) != 1 || nodes[0] != "novox" { t.Errorf("nodes = %v, err %v, want [novox]", nodes, err) } } -func TestChooseFailsNamingTheLanguageAndTheVersionsWhenNoNodeQualifies(t *testing.T) { // 1607/AC4 +func chooseFailsNamingTheLanguageAndTheVersionsWhenNoNodeQualifies(t *testing.T) { // 1607/AC4 _, _, err := ChooseNodes("typescript", nil, nil, nil, 2) if err == nil || !strings.Contains(err.Error(), "typescript") || !strings.Contains(err.Error(), delivery.ToolchainNoReport) { t.Errorf("no report: error = %v, want it to name typescript and %q", err, delivery.ToolchainNoReport) @@ -52,7 +52,7 @@ func TestChooseFailsNamingTheLanguageAndTheVersionsWhenNoNodeQualifies(t *testin } } -func TestAgreeNeedsEqualDigestsAndNamesBothNodesWhenTheyDiffer(t *testing.T) { // 1607/AC4, ADR 0478 clause 4 +func agreeNeedsEqualDigestsAndNamesBothNodesWhenTheyDiffer(t *testing.T) { // 1607/AC4, ADR 0478 clause 4 d, err := Agree("tool", map[string]string{"ace": digest, "novox": digest}) if err != nil || d != digest { t.Errorf("Agree = %q, %v, want %q", d, err, digest) @@ -66,7 +66,7 @@ func TestAgreeNeedsEqualDigestsAndNamesBothNodesWhenTheyDiffer(t *testing.T) { / } } -func TestChooseSkipsNodesThatAreNotOnlineAndFailsNamingThemWhenNoneIs(t *testing.T) { // 1607 review 3, ADR 0478 clause 3 +func chooseSkipsNodesThatAreNotOnlineAndFailsNamingThemWhenNoneIs(t *testing.T) { // 1607 review 3, ADR 0478 clause 3 entries := tcs("go", "go1.27.1", "ace", "g14", "novox") online := func(n string) bool { return n != "g14" } // a sleeping laptop nodes, tc, err := ChooseNodes("go", entries, placesRunning(map[string]int{"novox": 5}), online, 2) @@ -87,7 +87,7 @@ func placesRunning(running map[string]int) *Places { return &Places{running: running, changed: make(chan struct{})} } -func TestChoosePrefersNodesBelowTheirCapacity(t *testing.T) { // round 3 +func choosePrefersNodesBelowTheirCapacity(t *testing.T) { // round 3 entries := tcs("go", "go1.27.1", "ace", "novox", "shanks") // shanks has capacity 3 with 2 running: room; ace has the default 1 with 1 running: none; novox has none running. p := placesRunning(map[string]int{"shanks": 2, "ace": 1}) @@ -97,3 +97,13 @@ func TestChoosePrefersNodesBelowTheirCapacity(t *testing.T) { // round 3 t.Errorf("nodes = %v, err %v, want [novox shanks]: the nodes with room, ace at capacity left out", nodes, err) } } + +// The choice of the nodes a step runs on and the agreement of their answers, one table of cases (the test budget). +func TestChoosingTheNodesOfAStep(t *testing.T) { + t.Run("choose takes the agreed nodes with fewest running steps ties by name", chooseTakesTheAgreedNodesWithFewestRunningStepsTiesByName) + t.Run("choose with one node answers one node", chooseWithOneNodeAnswersOneNode) + t.Run("choose fails naming the language and the versions when no node qualifies", chooseFailsNamingTheLanguageAndTheVersionsWhenNoNodeQualifies) + t.Run("agree needs equal digests and names both nodes when they differ", agreeNeedsEqualDigestsAndNamesBothNodesWhenTheyDiffer) + t.Run("choose skips nodes that are not online and fails naming them when none is", chooseSkipsNodesThatAreNotOnlineAndFailsNamingThemWhenNoneIs) + t.Run("choose prefers nodes below their capacity", choosePrefersNodesBelowTheirCapacity) +} diff --git a/modules/mesh-delivery-builder/buildrun/steps_test.go b/modules/mesh-delivery-builder/buildrun/steps_test.go index 9d6d024..fe1d32e 100644 --- a/modules/mesh-delivery-builder/buildrun/steps_test.go +++ b/modules/mesh-delivery-builder/buildrun/steps_test.go @@ -63,7 +63,7 @@ func stepAt(node, id, state, reason string, result string) delivery.Step { return s } -func TestRunCallsTheToolOnTheNodeAndReadsTheStepsKey(t *testing.T) { // 1607/AC5 +func runCallsTheToolOnTheNodeAndReadsTheStepsKey(t *testing.T) { // 1607/AC5 caller := &fakeCaller{answer: `{"step":"abc1","key":"ace.abc1","state":"running"}`} steps := &fakeSteps{steps: map[string]delivery.Step{"mesh-build-go/ace.abc1": stepAt("ace", "abc1", delivery.StepPassed, "", `{"workspace":"w1","commit":"c"}`)}} res, err := Runner{Caller: caller, Steps: steps, Bound: time.Second}.Run(context.Background(), "mesh-build-go", "ace", "build_go_fetch", map[string]any{"repository": "novox/r", "commit": "c"}) @@ -79,7 +79,7 @@ func TestRunCallsTheToolOnTheNodeAndReadsTheStepsKey(t *testing.T) { // 1607/AC5 } } -func TestRunFailsOnAFailedOrRefusedStepNamingItsReason(t *testing.T) { // 1607/AC5 +func runFailsOnAFailedOrRefusedStepNamingItsReason(t *testing.T) { // 1607/AC5 for _, state := range []string{delivery.StepFailed, delivery.StepRefused} { caller := &fakeCaller{answer: `{"step":"abc1","key":"ace.abc1","state":"running"}`} steps := &fakeSteps{steps: map[string]delivery.Step{"mesh-build-go/ace.abc1": stepAt("ace", "abc1", state, "go is go1.26.0, the recipe builds with go1.27.1", "")}} @@ -90,7 +90,7 @@ func TestRunFailsOnAFailedOrRefusedStepNamingItsReason(t *testing.T) { // 1607/A } } -func TestRunStopsWaitingAtTheBound(t *testing.T) { // 1607/AC5 +func runStopsWaitingAtTheBound(t *testing.T) { // 1607/AC5 caller := &fakeCaller{answer: `{"step":"abc1","key":"ace.abc1","state":"running"}`} _, err := Runner{Caller: caller, Steps: &fakeSteps{}, Bound: 20 * time.Millisecond}.Run(context.Background(), "mesh-build-go", "ace", "build_go_build", nil) if err == nil || !strings.Contains(err.Error(), "ace.abc1") || !strings.Contains(err.Error(), "did not end") { @@ -98,7 +98,7 @@ func TestRunStopsWaitingAtTheBound(t *testing.T) { // 1607/AC5 } } -func TestRunRefusesAnAnswerWithNoKeyAndAKeyOfAnotherNode(t *testing.T) { // 1607/AC5 +func runRefusesAnAnswerWithNoKeyAndAKeyOfAnotherNode(t *testing.T) { // 1607/AC5 for _, answer := range []string{`{"state":"running"}`, `{"step":"abc1","key":"novox.abc1","state":"running"}`, `not json`} { _, err := Runner{Caller: &fakeCaller{answer: answer}, Steps: &fakeSteps{}, Bound: time.Second}.Run(context.Background(), "mesh-build-go", "ace", "build_go_fetch", nil) if err == nil { @@ -107,10 +107,19 @@ func TestRunRefusesAnAnswerWithNoKeyAndAKeyOfAnotherNode(t *testing.T) { // 1607 } } -func TestRunReturnsTheCallsOwnError(t *testing.T) { // 1607/AC5 +func runReturnsTheCallsOwnError(t *testing.T) { // 1607/AC5 boom := errors.New("no responders") _, err := Runner{Caller: &fakeCaller{err: boom}, Steps: &fakeSteps{}, Bound: time.Second}.Run(context.Background(), "mesh-build-go", "ace", "build_go_fetch", nil) if !errors.Is(err, boom) || !strings.Contains(err.Error(), "ace") { t.Errorf("error = %v, want the call's error naming the node", err) } } + +// A step run on its node and read from its steps key, one table of cases (the test budget). +func TestRunningAStepOnANode(t *testing.T) { + t.Run("run calls the tool on the node and reads the steps key", runCallsTheToolOnTheNodeAndReadsTheStepsKey) + t.Run("run fails on a failed or refused step naming its reason", runFailsOnAFailedOrRefusedStepNamingItsReason) + t.Run("run stops waiting at the bound", runStopsWaitingAtTheBound) + t.Run("run refuses an answer with no key and a key of another node", runRefusesAnAnswerWithNoKeyAndAKeyOfAnotherNode) + t.Run("run returns the calls own error", runReturnsTheCallsOwnError) +}