diff --git a/go/buildstep/askrun.go b/go/buildstep/askrun.go index f0cef51..6c5ebf8 100644 --- a/go/buildstep/askrun.go +++ b/go/buildstep/askrun.go @@ -8,6 +8,7 @@ import ( "slices" "sort" "strings" + "sync" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -170,16 +171,36 @@ func (s AskSteps) check(ctx context.Context, job delivery.CheckJob) (any, error) if job.Runner != "" && (job.RunnerWhen == "" || changesUnder(f.Changed, job.RunnerWhen)) { runner = job.Runner } - for _, level := range job.Levels { - la := delivery.LevelAnswer{Level: level} - passed, err := s.level(ctx, job, level, f, runner, &la) - answer.Levels = append(answer.Levels, la) - if err != nil { - return nil, err - } - if !passed { - return answer, nil // the first level that did not pass ends the check (ADR 0484 clause 13) + // The levels run side by side (issue 1921, try speed S10; the operator's yes to D1, ADR 0541): the integration level + // does not wait for the unit level. They are answered in their order, so the first level that did not pass is still + // the first failure the asker reads (ADR 0484 clause 13); the levels after it are kept in the answer for the log. + type ran struct { + la delivery.LevelAnswer + passed bool + err error + } + runs := make([]ran, len(job.Levels)) + var wg sync.WaitGroup + for i, level := range job.Levels { + wg.Add(1) + go func() { + defer wg.Done() + runs[i].la = delivery.LevelAnswer{Level: level} + runs[i].passed, runs[i].err = s.level(ctx, job, level, f, runner, &runs[i].la) + }() + } + wg.Wait() + failed := false + for _, r := range runs { + if r.err != nil { + if failed { + continue // a level after the first failure that could not run: the failure is the answer + } + answer.Levels = append(answer.Levels, r.la) + return nil, r.err } + answer.Levels = append(answer.Levels, r.la) + failed = failed || !r.passed } return answer, nil } diff --git a/go/buildstep/askrun_levels_test.go b/go/buildstep/askrun_levels_test.go new file mode 100644 index 0000000..41e1f03 --- /dev/null +++ b/go/buildstep/askrun_levels_test.go @@ -0,0 +1,60 @@ +package buildstep + +import ( + "context" + "encoding/json" + "sync" + "testing" + "time" + + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// 1921/S10 (the operator's yes to D1, ADR 0541): a check's levels run side by side: the integration level has started +// before the unit level ends. The answer holds the levels in their order, so with the unit level failing it still names +// the unit level first; the integration level's answer is kept after it for the log. +func TestACheckRunsItsLevelsSideBySideAndAnswersThemInOrder(t *testing.T) { + integrationStarted := make(chan struct{}) + var once sync.Once + var mu sync.Mutex + unitWaited := false + steps := AskSteps{ + Fetch: func(context.Context, map[string]any) (any, error) { + return map[string]any{"workspace": "/w", "modules": []string{"."}}, nil + }, + Test: func(ctx context.Context, args map[string]any) (any, error) { + switch args["level"] { + case delivery.LevelUnit: + select { + case <-integrationStarted: + mu.Lock() + unitWaited = true + mu.Unlock() + case <-time.After(2 * time.Second): + } + return map[string]any{"passed": false, "level": "unit"}, nil + default: + once.Do(func() { close(integrationStarted) }) + return map[string]any{"passed": true, "level": "integration"}, nil + } + }, + } + out, err := RunAsk(context.Background(), steps, ask(delivery.CheckJob{Levels: []string{delivery.LevelUnit, delivery.LevelIntegration}, + Fetch: map[string]any{"repository": "novox/x", "commit": runCommit}})) + if err != nil { + t.Fatal(err) + } + mu.Lock() + defer mu.Unlock() + if !unitWaited { + t.Error("the integration level did not start while the unit level ran") + } + a := out.(delivery.CheckAnswer) + if len(a.Levels) != 2 || a.Levels[0].Level != delivery.LevelUnit || a.Levels[1].Level != delivery.LevelIntegration { + t.Fatalf("levels %+v, want unit then integration", a.Levels) + } + var first struct{ Passed bool } + if err := json.Unmarshal(a.Levels[0].Tests[0], &first); err != nil || first.Passed { + t.Errorf("the first level's result %s, want the unit level's failure", a.Levels[0].Tests[0]) + } +} diff --git a/go/buildstep/askrun_test.go b/go/buildstep/askrun_test.go index 8ceb678..ca725db 100644 --- a/go/buildstep/askrun_test.go +++ b/go/buildstep/askrun_test.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "strings" + "sync" "testing" "time" @@ -16,6 +17,7 @@ const runCommit = "0123456789abcdef0123456789abcdef01234567" // recorder is a build module's step tools that record each call. type recorder struct { + mu sync.Mutex // a check's levels run side by side (issue 1921, S10) calls []string fetch map[string]any tests map[string]bool // package or module directory -> passed @@ -26,7 +28,9 @@ func (r *recorder) steps() AskSteps { call := func(name string, answer func(args map[string]any) any) StepFunc { return func(ctx context.Context, args map[string]any) (any, error) { b, _ := json.Marshal(args) + r.mu.Lock() r.calls = append(r.calls, name+" "+string(b)) + r.mu.Unlock() return answer(args), nil } } @@ -110,8 +114,8 @@ func TestRunAskRefusesAnotherToolchain(t *testing.T) { } // 1887/A3, A5a: a check ask runs the levels of one check after one fetch: it digests the unit level, takes the kept -// verdicts, tests the packages whose tests changed first, then the rest also after a failure (issue 1921), and runs no -// level after one that did not pass (ADR 0484 clause 13). +// verdicts, tests the packages whose tests changed first, then the rest also after a failure (issue 1921); the levels run +// side by side and are answered in order, the first that did not pass first (ADR 0484 clause 13; issue 1921 S10). func TestRunAskChecksTheLevelsOfOneCheck(t *testing.T) { r := &recorder{ fetch: map[string]any{"workspace": "/w", "changed": []string{"pkg/b/b_test.go"}}, @@ -123,7 +127,13 @@ func TestRunAskChecksTheLevelsOfOneCheck(t *testing.T) { if err != nil { t.Fatal(err) } - got := strings.Join(r.calls, "\n") + var unit []string + for _, c := range r.calls { + if !strings.Contains(c, `"level":"integration"`) { + unit = append(unit, c) + } + } + got := strings.Join(unit, "\n") want := `fetch {"base":"` + runCommit + `","commit":"` + runCommit + `","repository":"novox/x"} digest {"level":"unit","workspace":"/w"} test {"level":"unit","packages":["x/pkg/b"],"workspace":"/w"} @@ -132,8 +142,8 @@ test {"level":"unit","packages":["x/pkg/a"],"workspace":"/w"}` t.Fatalf("calls:\n%s\nwant:\n%s", got, want) } a := out.(delivery.CheckAnswer) - if len(a.Levels) != 1 { - t.Fatalf("levels %+v, want the unit level alone", a.Levels) + if len(a.Levels) != 2 || a.Levels[1].Level != delivery.LevelIntegration { + t.Fatalf("levels %+v, want the unit level first, the integration level after it", a.Levels) } u := a.Levels[0] if u.Level != delivery.LevelUnit || len(u.Kept) != 1 || u.Kept[0].Package != "x/pkg/k" || u.Kept[0].Tests != 7 || len(u.Tests) != 2 || u.Digest == nil { @@ -220,8 +230,8 @@ func TestRunAskRunsEveryPackageAfterAFailure(t *testing.T) { t.Fatal(err) } a := out.(delivery.CheckAnswer) - if len(a.Levels) != 1 || len(a.Levels[0].Tests) != 2 { - t.Fatalf("levels %+v, want the one level with both packages' results", a.Levels) + if len(a.Levels) != 2 || len(a.Levels[0].Tests) != 2 || a.Levels[0].Level != delivery.LevelIntegration { + t.Fatalf("levels %+v, want the first level with both packages' results, in the order asked", a.Levels) } for i, raw := range a.Levels[0].Tests { var res struct{ Passed bool } @@ -239,7 +249,7 @@ func TestRunAskRunsEveryPackageAfterAFailure(t *testing.T) { if err != nil { t.Fatal(err) } - if a := out.(delivery.CheckAnswer); len(r.calls) != 3 || len(a.Levels) != 1 || len(a.Levels[0].Tests) != 2 { - t.Fatalf("calls %v, levels %+v: want both directories of the unit level and no integration level", r.calls, a.Levels) + if a := out.(delivery.CheckAnswer); len(r.calls) != 5 || len(a.Levels) != 2 || len(a.Levels[0].Tests) != 2 || a.Levels[0].Level != delivery.LevelUnit { + t.Fatalf("calls %v, levels %+v: want both directories of each level, the unit level first", r.calls, a.Levels) } }