From 662db5e5116274206e2ee7b78f0aa6f361285ee3 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 17 Jun 2026 21:03:14 +0000 Subject: [PATCH] Report batch notifications in CLI output --- internal/cli/root.go | 22 ++++++ internal/cli/root_test.go | 148 +++++++++++++++++++++++++++++++------- 2 files changed, 144 insertions(+), 26 deletions(-) diff --git a/internal/cli/root.go b/internal/cli/root.go index f3852fc..ee9f038 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -378,6 +378,28 @@ func writeRunLogs(stderr io.Writer, result *app.BatchResult) { } _, _ = 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) } diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 4418225..4905fb9 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -211,26 +211,33 @@ func TestRunMorningReportsPartialFailureAndContinues(t *testing.T) { _ = oneArtifact(t, fixture.workspaceRoot, "data-packages", "tomorrow", "2026-05-30", "*.data_package.yaml") } -func TestBatchOutputIncludesNotificationDetails(t *testing.T) { +func TestBatchOutputIncludesTopLevelNotificationDetails(t *testing.T) { result := &app.BatchResult{ Batch: app.BatchMorning, Total: 2, - Succeeded: 1, - Failed: 1, + Succeeded: 2, + Failed: 0, + Notification: &app.BatchNotificationResult{ + Status: "succeeded", + RunID: "batch-distributor-run", + PipelineID: "weatherreporter", + BundleID: "weatherreporter.home.morning", + IdempotencyKey: "weatherreporter.home.morning.20260529T120000.000000000Z_morning", + Path: "/tmp/batch.distributor.json", + IncludedReports: []app.BatchNotificationReport{ + {ReportID: "daily", RunID: "daily-run", SourcePath: "/tmp/daily.md", BundlePaths: []string{"daily.md"}}, + }, + }, Reports: []app.BatchReportResult{ { - ReportID: "daily", - Status: "succeeded", - OutputPath: "/tmp/daily.md", - NotificationStatus: "accepted", - NotificationRunID: "distributor-run-1", + ReportID: "daily", + Status: "succeeded", + OutputPath: "/tmp/daily.md", }, { - ReportID: "three_day", - Status: "failed", - Error: "notify report three_day: upload failed", - NotificationStatus: "failed", - NotificationError: "notify report three_day: upload failed", + ReportID: "tomorrow", + Status: "succeeded", + OutputPath: "/tmp/tomorrow.md", }, }, } @@ -246,17 +253,22 @@ func TestBatchOutputIncludesNotificationDetails(t *testing.T) { if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil { t.Fatalf("decode batch JSON: %v\n%s", err, stdout.String()) } - if decoded.Reports[0].NotificationStatus != "accepted" || decoded.Reports[0].NotificationRunID != "distributor-run-1" { - t.Fatalf("success notification fields = %#v", decoded.Reports[0]) + if decoded.Notification == nil || decoded.Notification.Status != "succeeded" || decoded.Notification.RunID != "batch-distributor-run" || decoded.Notification.PipelineID != "weatherreporter" || len(decoded.Notification.IncludedReports) != 1 { + t.Fatalf("top-level notification = %#v, want succeeded batch notification", decoded.Notification) } - if decoded.Reports[1].NotificationStatus != "failed" || !strings.Contains(decoded.Reports[1].NotificationError, "upload failed") { - t.Fatalf("failure notification fields = %#v", decoded.Reports[1]) + for _, report := range decoded.Reports { + if report.NotificationStatus != "" || report.NotificationRunID != "" || report.NotificationError != "" { + t.Fatalf("report notification fields = %#v, want empty", report) + } } - if !strings.Contains(stderr.String(), `notificationStatus="accepted"`) || !strings.Contains(stderr.String(), `notificationRunId="distributor-run-1"`) { - t.Fatalf("stderr missing success notification fields:\n%s", stderr.String()) + if count := strings.Count(stderr.String(), "batchNotification "); count != 1 { + t.Fatalf("stderr batch notification lines = %d, want one:\n%s", count, stderr.String()) } - if !strings.Contains(stderr.String(), `notificationStatus="failed"`) || !strings.Contains(stderr.String(), `notificationError="notify report three_day: upload failed"`) { - t.Fatalf("stderr missing failure notification fields:\n%s", stderr.String()) + if !strings.Contains(stderr.String(), `batchNotification status="succeeded"`) || !strings.Contains(stderr.String(), `runId="batch-distributor-run"`) || !strings.Contains(stderr.String(), `pipelineId="weatherreporter"`) { + t.Fatalf("stderr missing batch notification details:\n%s", stderr.String()) + } + if strings.Contains(stderr.String(), "notificationStatus") || strings.Contains(stderr.String(), "notificationRunId") { + t.Fatalf("stderr includes per-report notification fields:\n%s", stderr.String()) } } @@ -265,13 +277,14 @@ func TestBatchOutputDoesNotExposeSecretLikeNotificationErrors(t *testing.T) { Batch: app.BatchMorning, Total: 1, Failed: 1, + Notification: &app.BatchNotificationResult{ + Status: "failed", + Error: "notify batch morning: upload failed: [redacted]", + }, Reports: []app.BatchReportResult{ { - ReportID: "daily", - Status: "failed", - Error: "notify report daily: upload failed: [redacted]", - NotificationStatus: "failed", - NotificationError: "notify report daily: upload failed: [redacted]", + ReportID: "daily", + Status: "succeeded", }, }, } @@ -293,6 +306,74 @@ func TestBatchOutputDoesNotExposeSecretLikeNotificationErrors(t *testing.T) { } } +func TestRunLogsIncludeSkippedBatchNotification(t *testing.T) { + result := &app.BatchResult{ + Batch: app.BatchMorning, + Total: 2, + Succeeded: 1, + Failed: 1, + Notification: &app.BatchNotificationResult{ + Status: "skipped", + Reason: "one or more reports failed", + }, + Reports: []app.BatchReportResult{ + {ReportID: "today", Status: "succeeded", OutputPath: "/tmp/today.md"}, + {ReportID: "tomorrow", Status: "failed", Error: "render failed"}, + }, + } + var stderr bytes.Buffer + var stdout bytes.Buffer + + if err := writeJSON(&stdout, result); err != nil { + t.Fatalf("writeJSON() error = %v", err) + } + writeRunLogs(&stderr, result) + + var decoded app.BatchResult + if err := json.Unmarshal(stdout.Bytes(), &decoded); err != nil { + t.Fatalf("decode batch JSON: %v\n%s", err, stdout.String()) + } + if decoded.Notification == nil || decoded.Notification.Status != "skipped" || decoded.Notification.Reason != "one or more reports failed" { + t.Fatalf("top-level notification = %#v, want skipped notification", decoded.Notification) + } + if count := strings.Count(stderr.String(), "batchNotification "); count != 1 { + t.Fatalf("stderr batch notification lines = %d, want one:\n%s", count, stderr.String()) + } + if !strings.Contains(stderr.String(), `batchNotification status="skipped" reason="one or more reports failed"`) { + t.Fatalf("stderr missing skipped batch notification:\n%s", stderr.String()) + } +} + +func TestRunLogsDoNotRepeatBatchNotificationErrorPerReport(t *testing.T) { + result := &app.BatchResult{ + Batch: app.BatchEvening, + Total: 1, + Succeeded: 1, + Failed: 1, + Notification: &app.BatchNotificationResult{ + Status: "failed", + Error: "notify batch evening: upload failed", + }, + Reports: []app.BatchReportResult{ + {ReportID: "tomorrow", Status: "succeeded", OutputPath: "/tmp/tomorrow.md"}, + }, + } + var stderr bytes.Buffer + + writeRunLogs(&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()) + } + if count := strings.Count(stderr.String(), "notify batch evening: upload failed"); count != 1 { + t.Fatalf("stderr batch notification error occurrences = %d, want one:\n%s", count, stderr.String()) + } + reportLine := firstLineWithPrefix(stderr.String(), "report=tomorrow ") + if strings.Contains(reportLine, "notify batch evening") || strings.Contains(reportLine, "notificationError") { + t.Fatalf("report line repeats batch notification error:\n%s", reportLine) + } +} + func TestRunEveningUsesOutputDirectoryAndSummary(t *testing.T) { fixture := newCLIFixture(t, writeFakeScriptorium) outputDir := fixture.path("copies") @@ -369,6 +450,12 @@ func TestRunEveningReportsOmitsPerReportNotification(t *testing.T) { if summary.Notification == nil || summary.Notification.Status != "succeeded" || summary.Notification.RunID != "batch-distributor-run" { t.Fatalf("batch notification = %#v, want succeeded batch notification", summary.Notification) } + if count := strings.Count(stderr.String(), "batchNotification "); count != 1 { + t.Fatalf("stderr batch notification lines = %d, want one:\n%s", count, stderr.String()) + } + if !strings.Contains(stderr.String(), `batchNotification status="succeeded"`) || !strings.Contains(stderr.String(), `runId="batch-distributor-run"`) { + t.Fatalf("stderr missing batch notification success:\n%s", stderr.String()) + } if summary.Reports[0].NotificationStatus != "" || summary.Reports[0].NotificationRunID != "" || summary.Reports[0].NotificationPipelineID != "" || summary.Reports[0].NotificationError != "" || summary.Reports[0].NotificationPath != "" { t.Fatalf("notification fields = %#v, want empty per-report notification fields", summary.Reports[0]) } @@ -1199,6 +1286,15 @@ func noArtifacts(t *testing.T, root string, parts ...string) { } } +func firstLineWithPrefix(text string, prefix string) string { + for _, line := range strings.Split(text, "\n") { + if strings.HasPrefix(line, prefix) { + return line + } + } + return "" +} + func assertFileContains(t *testing.T, path string, want string) { t.Helper() data, err := os.ReadFile(path)