Centralize CLI output helpers

internal/cli/output.go

@@ -0,0 +1,75 @@
+package cli
+
+import (
+	"encoding/json"
+	"fmt"
+	"io"
+
+	"gitea.maximumdirect.net/eric/weatherreporter/internal/app"
+)
+
+type outputOptions struct {
+	Quiet bool
+}
+
+func writeActionResult(stdout, stderr io.Writer, value any, opts outputOptions, writeStatus func(io.Writer)) error {
+	if opts.Quiet {
+		return nil
+	}
+	if writeStatus != nil && stderr != nil {
+		writeStatus(stderr)
+	}
+	return writeJSON(stdout, value)
+}
+
+func writeJSON(stdout io.Writer, value any) error {
+	encoder := json.NewEncoder(stdout)
+	encoder.SetIndent("", "  ")
+	return encoder.Encode(value)
+}
+
+func writeBatchStatus(stderr io.Writer, result *app.BatchResult) {
+	if stderr == nil || result == nil {
+		return
+	}
+	for _, item := range result.Reports {
+		notificationFields := ""
+		if item.NotificationStatus != "" {
+			notificationFields += fmt.Sprintf(" notificationStatus=%q", item.NotificationStatus)
+		}
+		if item.NotificationRunID != "" {
+			notificationFields += fmt.Sprintf(" notificationRunId=%q", item.NotificationRunID)
+		}
+		if item.NotificationError != "" {
+			notificationFields += fmt.Sprintf(" notificationError=%q", item.NotificationError)
+		}
+		if item.Status == "failed" {
+			_, _ = fmt.Fprintf(stderr, "report=%s status=failed error=%q%s\n", item.ReportID, item.Error, notificationFields)
+			continue
+		}
+		_, _ = fmt.Fprintf(stderr, "report=%s status=succeeded output=%q%s\n", item.ReportID, item.OutputPath, notificationFields)
+	}
+	if result.Notification != nil {
+		_, _ = fmt.Fprintf(stderr, "batchNotification status=%q", result.Notification.Status)
+		if result.Notification.Reason != "" {
+			_, _ = fmt.Fprintf(stderr, " reason=%q", result.Notification.Reason)
+		}
+		if result.Notification.RunID != "" {
+			_, _ = fmt.Fprintf(stderr, " runId=%q", result.Notification.RunID)
+		}
+		if result.Notification.PipelineID != "" {
+			_, _ = fmt.Fprintf(stderr, " pipelineId=%q", result.Notification.PipelineID)
+		}
+		if result.Notification.BundleID != "" {
+			_, _ = fmt.Fprintf(stderr, " bundleId=%q", result.Notification.BundleID)
+		}
+		if result.Notification.Path != "" {
+			_, _ = fmt.Fprintf(stderr, " path=%q", result.Notification.Path)
+		}
+		if result.Notification.Error != "" {
+			_, _ = fmt.Fprintf(stderr, " error=%q", result.Notification.Error)
+		}
+		_, _ = fmt.Fprintln(stderr)
+	}
+	_, _ = fmt.Fprintf(stderr, "batch=%s total=%d succeeded=%d failed=%d\n", result.Batch, result.Total, result.Succeeded, result.Failed)
+}

internal/cli/output_test.go

@@ -0,0 +1,59 @@
+package cli
+
+import (
+	"bytes"
+	"io"
+	"strings"
+	"testing"
+)
+
+func TestWriteActionResultWritesStatusBeforeJSON(t *testing.T) {
+	var output bytes.Buffer
+
+	err := writeActionResult(&output, &output, map[string]string{"status": "succeeded"}, outputOptions{}, func(w io.Writer) {
+		_, _ = w.Write([]byte("status line\n"))
+	})
+	if err != nil {
+		t.Fatalf("writeActionResult() error = %v", err)
+	}
+	if !strings.HasPrefix(output.String(), "status line\n") {
+		t.Fatalf("output = %q, want status before JSON", output.String())
+	}
+	if !strings.Contains(output.String(), `"status": "succeeded"`) {
+		t.Fatalf("output missing JSON result:\n%s", output.String())
+	}
+}
+
+func TestWriteActionResultQuietSuppressesOutput(t *testing.T) {
+	var stdout bytes.Buffer
+	var stderr bytes.Buffer
+
+	err := writeActionResult(&stdout, &stderr, map[string]string{"status": "succeeded"}, outputOptions{Quiet: true}, func(w io.Writer) {
+		_, _ = w.Write([]byte("status line\n"))
+	})
+	if err != nil {
+		t.Fatalf("writeActionResult() error = %v", err)
+	}
+	if stdout.Len() != 0 || stderr.Len() != 0 {
+		t.Fatalf("stdout/stderr = %q/%q, want no output", stdout.String(), stderr.String())
+	}
+}
+
+func TestWriteActionResultToleratesNilStderr(t *testing.T) {
+	var stdout bytes.Buffer
+	statusCalled := false
+
+	err := writeActionResult(&stdout, nil, map[string]string{"status": "succeeded"}, outputOptions{}, func(w io.Writer) {
+		statusCalled = true
+		_, _ = w.Write([]byte("status line\n"))
+	})
+	if err != nil {
+		t.Fatalf("writeActionResult() error = %v", err)
+	}
+	if statusCalled {
+		t.Fatal("status writer was called with nil stderr")
+	}
+	if !strings.Contains(stdout.String(), `"status": "succeeded"`) {
+		t.Fatalf("stdout missing JSON result:\n%s", stdout.String())
+	}
+}

internal/cli/root.go

@@ -2,7 +2,6 @@ package cli
 
 import (
 	"context"
-	"encoding/json"
 	"flag"
 	"fmt"
 	"io"
@@ -74,8 +73,9 @@ func (r Runner) Run(ctx context.Context, args []string, stdout io.Writer, stderr
 		}
 		result, err := app.RunBatchDetailed(ctx, req)
 		if result != nil {
-			writeRunLogs(stderr, result)
-			if encodeErr := writeJSON(stdout, result); encodeErr != nil {
+			if encodeErr := writeActionResult(stdout, stderr, result, outputOptions{}, func(w io.Writer) {
+				writeBatchStatus(w, result)
+			}); encodeErr != nil {
 				return encodeErr
 			}
 			if result.Failed > 0 {
@@ -351,58 +351,6 @@ func parseInspectRunFlags(command string, args []string) (inspectOptions, error)
 	return opts, nil
 }
 
-func writeJSON(stdout io.Writer, value any) error {
-	encoder := json.NewEncoder(stdout)
-	encoder.SetIndent("", "  ")
-	return encoder.Encode(value)
-}
-
-func writeRunLogs(stderr io.Writer, result *app.BatchResult) {
-	if stderr == nil || result == nil {
-		return
-	}
-	for _, item := range result.Reports {
-		notificationFields := ""
-		if item.NotificationStatus != "" {
-			notificationFields += fmt.Sprintf(" notificationStatus=%q", item.NotificationStatus)
-		}
-		if item.NotificationRunID != "" {
-			notificationFields += fmt.Sprintf(" notificationRunId=%q", item.NotificationRunID)
-		}
-		if item.NotificationError != "" {
-			notificationFields += fmt.Sprintf(" notificationError=%q", item.NotificationError)
-		}
-		if item.Status == "failed" {
-			_, _ = fmt.Fprintf(stderr, "report=%s status=failed error=%q%s\n", item.ReportID, item.Error, notificationFields)
-			continue
-		}
-		_, _ = fmt.Fprintf(stderr, "report=%s status=succeeded output=%q%s\n", item.ReportID, item.OutputPath, notificationFields)
-	}
-	if result.Notification != nil {
-		_, _ = fmt.Fprintf(stderr, "batchNotification status=%q", result.Notification.Status)
-		if result.Notification.Reason != "" {
-			_, _ = fmt.Fprintf(stderr, " reason=%q", result.Notification.Reason)
-		}
-		if result.Notification.RunID != "" {
-			_, _ = fmt.Fprintf(stderr, " runId=%q", result.Notification.RunID)
-		}
-		if result.Notification.PipelineID != "" {
-			_, _ = fmt.Fprintf(stderr, " pipelineId=%q", result.Notification.PipelineID)
-		}
-		if result.Notification.BundleID != "" {
-			_, _ = fmt.Fprintf(stderr, " bundleId=%q", result.Notification.BundleID)
-		}
-		if result.Notification.Path != "" {
-			_, _ = fmt.Fprintf(stderr, " path=%q", result.Notification.Path)
-		}
-		if result.Notification.Error != "" {
-			_, _ = fmt.Fprintf(stderr, " error=%q", result.Notification.Error)
-		}
-		_, _ = fmt.Fprintln(stderr)
-	}
-	_, _ = fmt.Fprintf(stderr, "batch=%s total=%d succeeded=%d failed=%d\n", result.Batch, result.Total, result.Succeeded, result.Failed)
-}
-
 func addCommonFlags(fs *flag.FlagSet, opts *commonOptions, includeOutput bool) {
 	fs.StringVar(&opts.ConfigPath, "config", "", "configuration file path")
 	fs.StringVar(&opts.Units, "units", "", "weather API units")

internal/cli/root_test.go

@@ -247,7 +247,7 @@ func TestBatchOutputIncludesTopLevelNotificationDetails(t *testing.T) {
 	if err := writeJSON(&stdout, result); err != nil {
 		t.Fatalf("writeJSON() error = %v", err)
 	}
-	writeRunLogs(&stderr, result)
+	writeBatchStatus(&stderr, result)
 
 	var decoded app.BatchResult
 	if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil {
@@ -294,7 +294,7 @@ func TestBatchOutputDoesNotExposeSecretLikeNotificationErrors(t *testing.T) {
 	if err := writeJSON(&stdout, result); err != nil {
 		t.Fatalf("writeJSON() error = %v", err)
 	}
-	writeRunLogs(&stderr, result)
+	writeBatchStatus(&stderr, result)
 
 	for _, output := range []string{stdout.String(), stderr.String()} {
 		if strings.Contains(output, "DISTRIBUTOR_SECRET_TOKEN") {
@@ -306,7 +306,7 @@ func TestBatchOutputDoesNotExposeSecretLikeNotificationErrors(t *testing.T) {
 	}
 }
 
-func TestRunLogsIncludeSkippedBatchNotification(t *testing.T) {
+func TestBatchStatusIncludesSkippedBatchNotification(t *testing.T) {
 	result := &app.BatchResult{
 		Batch:     app.BatchMorning,
 		Total:     2,
@@ -327,7 +327,7 @@ func TestRunLogsIncludeSkippedBatchNotification(t *testing.T) {
 	if err := writeJSON(&stdout, result); err != nil {
 		t.Fatalf("writeJSON() error = %v", err)
 	}
-	writeRunLogs(&stderr, result)
+	writeBatchStatus(&stderr, result)
 
 	var decoded app.BatchResult
 	if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil {
@@ -344,7 +344,7 @@ func TestRunLogsIncludeSkippedBatchNotification(t *testing.T) {
 	}
 }
 
-func TestRunLogsDoNotRepeatBatchNotificationErrorPerReport(t *testing.T) {
+func TestBatchStatusDoesNotRepeatBatchNotificationErrorPerReport(t *testing.T) {
 	result := &app.BatchResult{
 		Batch:     app.BatchEvening,
 		Total:     1,
@@ -360,7 +360,7 @@ func TestRunLogsDoNotRepeatBatchNotificationErrorPerReport(t *testing.T) {
 	}
 	var stderr bytes.Buffer
 
-	writeRunLogs(&stderr, result)
+	writeBatchStatus(&stderr, result)
 
 	if count := strings.Count(stderr.String(), "batchNotification "); count != 1 {
 		t.Fatalf("stderr batch notification lines = %d, want one:\n%s", count, stderr.String())