diff --git a/cli/cmd/disruption.go b/cli/cmd/disruption.go index 3ae6f2e7..77a0ac2c 100644 --- a/cli/cmd/disruption.go +++ b/cli/cmd/disruption.go @@ -69,8 +69,23 @@ var disruptionRunCmd = &cobra.Command{ return cmdErr("Disruption test failed: " + err.Error()) } + // Written ahead of the -o json branch, which returns: a CI job asking + // for both got no JUnit file. Its note goes to stderr there, so stdout + // stays the JSON alone. + junitNote := "" + if outputJUnit != "" { + if err := writeJUnitXML(&result.Report, outputJUnit); err != nil { + junitNote = " ⚠ JUnit XML write failed: " + err.Error() + } else { + junitNote = " ✓ JUnit XML written to " + outputJUnit + } + } + if outputMode == "json" { output.JSON(result) + if junitNote != "" { + fmt.Fprintln(output.Err, junitNote) + } return checkSlaExit(&result.Report) } @@ -112,12 +127,8 @@ var disruptionRunCmd = &cobra.Command{ renderStepReport(step) } - if outputJUnit != "" { - if err := writeJUnitXML(&result.Report, outputJUnit); err != nil { - fmt.Println(" ⚠ JUnit XML write failed: " + err.Error()) - } else { - fmt.Println(" ✓ JUnit XML written to " + outputJUnit) - } + if junitNote != "" { + fmt.Println(junitNote) } return checkSlaExit(&result.Report) @@ -230,6 +241,9 @@ func renderSlaVerdict(verdict *client.SlaVerdict) { } } +// checkSlaExit fails the command when --fail-on-sla-breach is set and the +// report breached its SLA. The verdict is already printed, so the error is +// silent; it used to os.Exit(1) in the middle of RunE, past every defer. func checkSlaExit(report *client.DisruptionReport) error { if !failOnSlaBreach { return nil @@ -237,11 +251,8 @@ func checkSlaExit(report *client.DisruptionReport) error { if report == nil { return nil } - if report.SlaVerdict != nil && report.SlaVerdict.Violated { - os.Exit(1) - } - if report.Summary != nil && report.Summary.SlaViolated { - os.Exit(1) + if (report.SlaVerdict != nil && report.SlaVerdict.Violated) || (report.Summary != nil && report.Summary.SlaViolated) { + return &silentErr{msg: "the disruption plan breached its SLA"} } return nil } diff --git a/cli/cmd/disruption_playbook_test.go b/cli/cmd/disruption_playbook_test.go index 8902af85..0b81abdd 100644 --- a/cli/cmd/disruption_playbook_test.go +++ b/cli/cmd/disruption_playbook_test.go @@ -9,6 +9,7 @@ import ( "net/http" "net/http/httptest" "os" + "path/filepath" "strings" "sync" "testing" @@ -83,6 +84,7 @@ type playbookBackend struct { plans map[string]string runs bool dryRunResult string + report string mu sync.Mutex // the handler runs on the server's goroutines dryRuns [][]byte @@ -111,7 +113,8 @@ func (b *playbookBackend) seen() (requests []string, dryRuns [][]byte) { // The options apply before the server starts: the handler reads them. func newPlaybookBackend(t *testing.T, plans map[string]string, opts ...func(*playbookBackend)) *playbookBackend { t.Helper() - b := &playbookBackend{t: t, plans: plans, dryRunResult: leaderCascadeDryRunJSON} + b := &playbookBackend{t: t, plans: plans, dryRunResult: leaderCascadeDryRunJSON, + report: `{"planName":"playbook:leader-cascade","status":"COMPLETED"}`} for _, opt := range opts { opt(b) } @@ -124,6 +127,7 @@ func newPlaybookBackend(t *testing.T, plans map[string]string, opts ...func(*pla outputMode = "table" playbookRunDryRun = false disruptionFile, dryRunMode = "", false + outputJUnit, failOnSlaBreach = "", false } reset() t.Cleanup(reset) @@ -157,7 +161,7 @@ func (b *playbookBackend) serve(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusAccepted) _, _ = io.WriteString(w, `{"id":"d-42","status":"RUNNING","planName":"playbook:leader-cascade"}`) case b.runs && r.Method == http.MethodGet && r.URL.Path == "/api/disruptions/d-42": - _, _ = io.WriteString(w, `{"planName":"playbook:leader-cascade","status":"COMPLETED"}`) + _, _ = io.WriteString(w, b.report) default: b.t.Errorf("unexpected request %s %s", r.Method, r.URL) w.WriteHeader(http.StatusTeapot) @@ -606,6 +610,55 @@ func TestDisruptionRun_JSONIsTheResultAlone(t *testing.T) { } } +// -o json returned before the JUnit report was written, so a CI job that asked +// for both got no file. Its note goes to stderr, keeping stdout the JSON. +func TestDisruptionRun_JSONWritesJUnit(t *testing.T) { + newPlaybookBackend(t, map[string]string{}, allowRuns) + dir := t.TempDir() + planFile, junitFile := filepath.Join(dir, "plan.json"), filepath.Join(dir, "results.xml") + if err := os.WriteFile(planFile, []byte(leaderCascadePlanJSON), 0o600); err != nil { + t.Fatal(err) + } + + stdout, stderr, err := commandOutput(t, func() error { + return runCommandLine(t, "disruption", "run", "--config", planFile, "-o", "json", "--output-junit", junitFile) + }) + if err != nil { + t.Fatalf("unexpected error: %v\n%s%s", err, stdout, stderr) + } + if err := json.Unmarshal(stdout, &client.DisruptionRunResponse{}); err != nil { + t.Errorf("-o json output is not the result alone: %v\n%s", err, stdout) + } + if xml, err := os.ReadFile(junitFile); err != nil || !strings.Contains(string(xml), "