diff --git a/AGENTS.md b/AGENTS.md index e03b907..9d0185d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,6 +48,13 @@ when one is authored, add it here. - `cd candy/plugin-task && go test ./...` — the dependency closure, CLI parsing, validation, and incremental-semantics tests. - `cd candy/plugin-task && gofmt -l .` — formatting (empty output passes). +- `cd candy/plugin-task && go vet ./...` — the vet pass. +- `cd candy/plugin-task && GOTOOLCHAIN=go$(go list -m -f '{{.GoVersion}}') golangci-lint run` + — the linter, under the toolchain this module's `go.mod` declares: golangci-lint + cannot read the export data of a newer GOROOT, so the toolchain must be the + module's own, and the linter's version is the one CI pins in + `.github/workflows/ci.yml` (one source of truth — read it there). The local gate + runs what CI's `go` job runs, so a finding CI would raise is raised locally first. - `charly box validate` at the repo root — the structural check (the candy + `plugin:` block, CUE schema). - `scripts/gate-task.sh` at the repo root — the live gate: it clones charly at diff --git a/candy/plugin-task/capture_test.go b/candy/plugin-task/capture_test.go new file mode 100644 index 0000000..144ff39 --- /dev/null +++ b/candy/plugin-task/capture_test.go @@ -0,0 +1,51 @@ +package task + +import ( + "io" + "os" + "testing" +) + +// captureStreams runs fn with os.Stdout and os.Stderr pointed at pipes and returns +// what each stream received. +// +// ONE shape for the whole package: a test that asserts on what the reporter wrote +// must not re-implement the capture, because a capture written twice is a capture +// written once correctly and once wrongly. This one is written correctly: +// +// - BOTH ends of BOTH pipes are kept. os.Pipe returns (read, write); a pipe whose +// write end is dropped can never be drained, so the read end would block a later +// writer forever and the descriptor would leak. +// - The write ends are closed BEFORE the drain. io.ReadAll returns EOF only when +// every write end is closed, and a pipe left undrained blocks the next writer. +// - The globals go back on every exit path, including a panic in fn — which the +// testing package recovers and then runs the NEXT test with. Without the +// restore, that test would write its output into a pipe nobody reads. +// - The restore happens before the caller asserts, so a failure message is never +// written to a closed pipe (and lost). +func captureStreams(t *testing.T, fn func()) (stdout, stderr string) { + t.Helper() + outR, outW, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + errR, errW, err := os.Pipe() + if err != nil { + _ = outR.Close() + _ = outW.Close() + t.Fatal(err) + } + oldOut, oldErr := os.Stdout, os.Stderr + os.Stdout, os.Stderr = outW, errW + defer func() { + os.Stdout, os.Stderr = oldOut, oldErr + }() + fn() + _ = outW.Close() + _ = errW.Close() + out, _ := io.ReadAll(outR) + _ = outR.Close() + errOut, _ := io.ReadAll(errR) + _ = errR.Close() + return string(out), string(errOut) +} diff --git a/candy/plugin-task/cli_test.go b/candy/plugin-task/cli_test.go index 3b7c4c3..b3666a0 100644 --- a/candy/plugin-task/cli_test.go +++ b/candy/plugin-task/cli_test.go @@ -2,8 +2,6 @@ package task import ( "context" - "io" - "os" "strings" "testing" @@ -44,29 +42,17 @@ func TestRunTaskCLI_OutputMode(t *testing.T) { } defer func() { entityCache = oldCache }() - oldOut, oldErr := os.Stdout, os.Stderr - outR, outW, err := os.Pipe() - if err != nil { - t.Fatal(err) - } - errR, errW, err := os.Pipe() - if err != nil { - t.Fatal(err) - } - os.Stdout, os.Stderr = outW, errW - runErr := runTaskCLI(context.Background(), nil, []string{"capture", "--output"}) - _ = outW.Close() - _ = errW.Close() - os.Stdout, os.Stderr = oldOut, oldErr + var runErr error + stdout, stderr := captureStreams(t, func() { + runErr = runTaskCLI(context.Background(), nil, []string{"capture", "--output"}) + }) if runErr != nil { t.Fatalf("runTaskCLI --output: %v", runErr) } - stdout, _ := io.ReadAll(outR) - stderr, _ := io.ReadAll(errR) - if string(stdout) != "CAPTURED-RAW-VALUE\n" { + if stdout != "CAPTURED-RAW-VALUE\n" { t.Fatalf("stdout must carry ONLY the raw captured value + newline, got %q", stdout) } - if !strings.Contains(string(stderr), "task capture:") { + if !strings.Contains(stderr, "task capture:") { t.Fatalf("the human report must be on stderr, got %q", stderr) } } diff --git a/candy/plugin-task/plugin_test.go b/candy/plugin-task/plugin_test.go index 2dac3f8..3962117 100644 --- a/candy/plugin-task/plugin_test.go +++ b/candy/plugin-task/plugin_test.go @@ -3,7 +3,6 @@ package task import ( "context" "encoding/json" - "io" "os" "path/filepath" "strings" @@ -396,23 +395,16 @@ func TestFormatStepLine_RendersStepMessage(t *testing.T) { // asserts the step message reaches it — the end-to-end arm the pure renderer test // cannot cover (a reporter that stopped calling the renderer would pass that one). func TestPrintResultText_EmitsStepMessage(t *testing.T) { - old := os.Stdout - r, w, err := os.Pipe() - if err != nil { - t.Fatal(err) - } - os.Stdout = w - printResultText(outWriter(), &runResult{ - Name: "map", Status: "ran", - Steps: []spec.StepResult{{ - Keyword: "run", Text: "print the submodule map", - Result: spec.CheckResult{Status: spec.StatusPass, Message: "MAP-TABLE-MARKER"}, - }}, + out, _ := captureStreams(t, func() { + printResultText(outWriter(), &runResult{ + Name: "map", Status: "ran", + Steps: []spec.StepResult{{ + Keyword: "run", Text: "print the submodule map", + Result: spec.CheckResult{Status: spec.StatusPass, Message: "MAP-TABLE-MARKER"}, + }}, + }) }) - _ = w.Close() - os.Stdout = old - out, _ := io.ReadAll(r) - if !strings.Contains(string(out), "MAP-TABLE-MARKER") { + if !strings.Contains(out, "MAP-TABLE-MARKER") { t.Fatalf("printResultText must emit the step message, got:\n%s", out) } } diff --git a/candy/plugin-task/skipped_test.go b/candy/plugin-task/skipped_test.go index f9e8819..f10e72b 100644 --- a/candy/plugin-task/skipped_test.go +++ b/candy/plugin-task/skipped_test.go @@ -4,8 +4,6 @@ import ( "bytes" "context" "encoding/json" - "io" - "os" "strings" "testing" @@ -153,31 +151,22 @@ func TestPrintResultsJSON_CarriesTheSkippedCount(t *testing.T) { Skipped: 1, Steps: []spec.StepResult{{Keyword: "run", Text: "the step that ran nothing", Result: spec.CheckResult{Status: spec.StatusSkip}}}, }} - oldOut := os.Stdout - r, w, err := os.Pipe() - if err != nil { - t.Fatal(err) - } - os.Stdout = w - jerr := printResultsJSON(results) - _ = w.Close() - os.Stdout = oldOut + var jerr error + out, _ := captureStreams(t, func() { + jerr = printResultsJSON(results) + }) if jerr != nil { t.Fatalf("printResultsJSON: %v", jerr) } - var buf bytes.Buffer - if _, err := buf.ReadFrom(r); err != nil { - t.Fatal(err) - } var decoded []struct { Failed int `json:"failed"` Skipped int `json:"skipped"` } - if err := json.Unmarshal(buf.Bytes(), &decoded); err != nil { - t.Fatalf("the JSON report must decode: %v (%s)", err, buf.String()) + if err := json.Unmarshal([]byte(out), &decoded); err != nil { + t.Fatalf("the JSON report must decode: %v (%s)", err, out) } if len(decoded) != 1 || decoded[0].Skipped != 1 { - t.Fatalf("the JSON report must carry the skipped count, got %s", buf.String()) + t.Fatalf("the JSON report must carry the skipped count, got %s", out) } } @@ -200,25 +189,17 @@ func TestRunTaskCLI_SkippedStepExitsZero(t *testing.T) { } defer func() { entityCache = oldCache }() - oldOut, oldErr := os.Stdout, os.Stderr - outR, outW, err := os.Pipe() - if err != nil { - t.Fatal(err) - } - errW, _, err := os.Pipe() - if err != nil { - t.Fatal(err) - } - os.Stdout, os.Stderr = outW, errW - runErr := runTaskCLI(context.Background(), nil, []string{"mixed"}) - _ = outW.Close() - _ = errW.Close() - os.Stdout, os.Stderr = oldOut, oldErr + var runErr error + report, stderrReport := captureStreams(t, func() { + runErr = runTaskCLI(context.Background(), nil, []string{"mixed"}) + }) if runErr != nil { t.Fatalf("a task whose only step ran nothing must exit 0, got %v", runErr) } - report, _ := io.ReadAll(outR) - if !strings.Contains(string(report), "0 failed, 1 skipped") { + if !strings.Contains(report, "0 failed, 1 skipped") { t.Fatalf("the report must name the skipped count, got:\n%s", report) } + if stderrReport != "" { + t.Fatalf("the report belongs on stdout, not stderr, got:\n%s", stderrReport) + } }