eric/weatherreporter
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

Suppress per-report notifications during batch runs

Browse Source
This commit is contained in:
Eric Rakestraw
2026-06-17 20:52:27 +00:00
parent 2b3bcdd4f1
commit 1d2f176977
3 changed files with 44 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
internal/app/app.go
View File

@@ -91,6 +91,7 @@ type ReportRequest struct {
Renderer Renderer
Store state.Store
Notifier Notifier
noNotify bool
}
type ReportResult struct {
@@ -331,6 +332,7 @@ func RunBatchDetailed(ctx context.Context, req BatchRequest) (*BatchResult, erro
Renderer: req.Renderer,
Store: store,
Notifier: req.Notifier,
noNotify: true,
})
if err != nil {
item.Status = "failed"
@@ -619,6 +621,7 @@ func GenerateReport(ctx context.Context, req ReportRequest) (*ReportResult, erro
OutputPath: req.OutputPath,
Notifier: req.Notifier,
GenerationErr: runErr,
noNotify: req.noNotify,
})
if err != nil {
return nil, err
@@ -741,6 +744,7 @@ func generateTextTemplateReport(ctx context.Context, req generatedReportRequest)
ManagedReportPath: reportPath,
OutputPath: req.OutputPath,
Notifier: req.Notifier,
noNotify: req.noNotify,
})
if err != nil {
return nil, err
@@ -778,6 +782,7 @@ type finalizeRenderedReportRequest struct {
OutputPath string
Notifier Notifier
GenerationErr error
noNotify bool
}
type finalizeRenderedReportResult struct {
@@ -820,6 +825,9 @@ func finalizeRenderedReport(ctx context.Context, req finalizeRenderedReportReque
if req.GenerationErr != nil {
return result, req.GenerationErr
}
if req.noNotify {
return result, nil
}
notification, notificationPath, err := notifyReport(ctx, req.Config, req.Resolved, req.ManagedReportPath, metadata, req.Notifier, req.Store)
if notificationPath != "" {

98
internal/app/app_test.go
View File

@@ -2497,92 +2497,54 @@ func TestRunBatchContinuesAfterReportFailure(t *testing.T) {
}
}
func TestRunBatchContinuesAfterNotificationFailure(t *testing.T) {
func TestRunBatchSuppressesPerReportNotification(t *testing.T) {
server := dailyBundleServer(t)
cfg := dailyNotificationConfig(t, server)
collection := collectionWithFutureDailyForTest(t, cfg, "2026-05-31")
cfg.WeatherAPI.BaseURL = ""
collector := &recordingCollector{result: &collection}
notifier := &recordingNotifier{
errByReport: map[report.ID]error{
report.Tomorrow: errors.New("distributor unavailable"),
},
}
store := recordingFilesystemStore(t, cfg)
notifier := &recordingNotifier{err: errors.New("distributor unavailable")}
result, err := RunBatchDetailed(context.Background(), BatchRequest{
Config: cfg,
Batch: BatchMorning,
Now: mustParse("2026-05-29T05:00:00-05:00"),
Batch: BatchEvening,
Now: mustParse("2026-05-29T18:00:00-05:00"),
Collector: collector,
Renderer: &selectiveRenderer{runBody: "# Batch Report\n"},
Store: store,
Notifier: notifier,
})
if err != nil {
t.Fatalf("RunBatchDetailed() error = %v", err)
}
if result.Total != 3 || result.Succeeded != 2 || result.Failed != 1 {
t.Fatalf("summary total/succeeded/failed = %d/%d/%d, want 3/2/1", result.Total, result.Succeeded, result.Failed)
if result.Total != 2 || result.Succeeded != 2 || result.Failed != 0 {
t.Fatalf("summary total/succeeded/failed = %d/%d/%d, want 2/2/0", result.Total, result.Succeeded, result.Failed)
}
if len(collector.requests) != 1 {
t.Fatalf("collector requests = %d, want one collection for batch", len(collector.requests))
}
if len(notifier.requests) != 3 {
t.Fatalf("notification requests = %d, want one per generated report", len(notifier.requests))
if len(notifier.requests) != 0 {
t.Fatalf("notification requests = %#v, want none for batch-generated reports", notifier.requests)
}
var failedTomorrow bool
for _, item := range result.Reports {
if item.ReportID == report.Tomorrow {
if item.Status == "failed" && strings.Contains(item.Error, "notify report") && strings.Contains(item.Error, "distributor unavailable") {
failedTomorrow = true
}
if item.NotificationStatus != "failed" {
t.Fatalf("Tomorrow notification status = %q, want failed", item.NotificationStatus)
}
if item.NotificationPipelineID != "weatherreporter.tomorrow" {
t.Fatalf("Tomorrow notification pipeline = %q, want weatherreporter.tomorrow", item.NotificationPipelineID)
}
if !strings.Contains(item.NotificationError, "distributor unavailable") {
t.Fatalf("Tomorrow notification error = %q, want distributor unavailable", item.NotificationError)
}
continue
}
if item.Status != "succeeded" {
t.Fatalf("report %s status = %s, want succeeded", item.ReportID, item.Status)
}
if item.NotificationStatus != "accepted" {
t.Fatalf("report %s notification status = %q, want accepted", item.ReportID, item.NotificationStatus)
if item.NotificationStatus != "" || item.NotificationRunID != "" || item.NotificationPipelineID != "" || item.NotificationError != "" || item.NotificationPath != "" {
t.Fatalf("report %s notification fields = %#v, want empty per-report notification fields", item.ReportID, item)
}
if item.NotificationPipelineID == "" {
t.Fatalf("report %s notification pipeline is empty", item.ReportID)
metadata := readMetadataForTest(t, item.MetadataPath)
if metadata.NotificationPath != "" {
t.Fatalf("report %s metadata NotificationPath = %q, want empty", item.ReportID, metadata.NotificationPath)
}
}
if !failedTomorrow {
t.Fatalf("reports = %#v, want notification failure on Tomorrow item", result.Reports)
}
cfg.Workspace.Root = t.TempDir()
collector = &recordingCollector{result: &collection}
err = RunBatch(context.Background(), BatchRequest{
Config: cfg,
Batch: BatchMorning,
Now: mustParse("2026-05-29T05:00:00-05:00"),
Collector: collector,
Renderer: &selectiveRenderer{
runBody: "# Batch Report\n",
},
Notifier: &recordingNotifier{
errByReport: map[report.ID]error{
report.Tomorrow: errors.New("distributor unavailable"),
},
},
})
var batchErr BatchError
if !errors.As(err, &batchErr) {
t.Fatalf("RunBatch() error = %T %v, want BatchError", err, err)
}
if batchErr.Result == nil || batchErr.Result.Failed != 1 || batchErr.Result.Succeeded != 2 {
t.Fatalf("RunBatch() result = %#v, want notification failure aggregate", batchErr.Result)
notificationDir := filepath.Join(cfg.Workspace.Root, cfg.Workspace.NotificationsDir)
if _, err := os.Stat(notificationDir); err == nil {
t.Fatalf("notification directory %q exists, want no per-report notification artifacts", notificationDir)
} else if !os.IsNotExist(err) {
t.Fatalf("stat notification directory %q: %v", notificationDir, err)
}
}
@@ -2664,8 +2626,8 @@ func TestRunBatchDynamicDailyReportsHaveDistinctIdentity(t *testing.T) {
if result.Failed != 0 || len(result.Reports) != 3 {
t.Fatalf("summary = %#v, want three successful reports", result)
}
if len(notifier.requests) != 3 {
t.Fatalf("notification requests = %d, want one per report", len(notifier.requests))
if len(notifier.requests) != 0 {
t.Fatalf("notification requests = %#v, want none for batch-generated reports", notifier.requests)
}
dailyByDate := map[string]BatchReportResult{}
@@ -2713,22 +2675,6 @@ func TestRunBatchDynamicDailyReportsHaveDistinctIdentity(t *testing.T) {
}
}
dailyKeys := map[string]struct{}{}
for _, req := range notifier.requests {
if req.ReportID != report.Daily {
continue
}
if req.IdempotencyKey != req.BundleID+"."+req.RunID {
t.Fatalf("Daily idempotency key = %q, want bundle id plus run id", req.IdempotencyKey)
}
if _, ok := dailyKeys[req.IdempotencyKey]; ok {
t.Fatalf("duplicate Daily idempotency key: %q", req.IdempotencyKey)
}
dailyKeys[req.IdempotencyKey] = struct{}{}
}
if len(dailyKeys) != 2 {
t.Fatalf("Daily notification keys = %#v, want two distinct keys", dailyKeys)
}
}
func TestRunBatchMorningUsesTodayOutputName(t *testing.T) {

47
internal/cli/root_test.go
View File

@@ -321,20 +321,10 @@ func TestRunEveningUsesOutputDirectoryAndSummary(t *testing.T) {
}
}
func TestRunEveningReportsNotificationSuccess(t *testing.T) {
func TestRunEveningReportsOmitsPerReportNotification(t *testing.T) {
server := dailyServer(t)
distributorServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/runs/distributor-run-1" {
w.Header().Set("Content-Type", "application/json")
_, _ = w.Write([]byte(`{"run_id":"distributor-run-1","pipeline_id":"weatherreporter.tomorrow","status":"succeeded","report":{"actions":[{"action":"replace_older"}]}}`))
return
}
if r.URL.Path != "/v1/pipelines/weatherreporter.tomorrow/upload" {
http.NotFound(w, r)
return
}
w.WriteHeader(http.StatusAccepted)
_, _ = w.Write([]byte(`{"run_id":"distributor-run-1","status":"accepted"}`))
t.Fatalf("unexpected distributor request %s", r.URL.Path)
}))
t.Cleanup(distributorServer.Close)
tempDir := t.TempDir()
@@ -361,29 +351,27 @@ func TestRunEveningReportsNotificationSuccess(t *testing.T) {
if len(summary.Reports) != 1 {
t.Fatalf("reports = %#v, want one report", summary.Reports)
}
if summary.Reports[0].NotificationStatus != "succeeded" || summary.Reports[0].NotificationRunID != "distributor-run-1" || summary.Reports[0].NotificationPipelineID != "weatherreporter.tomorrow" {
t.Fatalf("notification fields = %#v", summary.Reports[0])
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])
}
if !strings.Contains(stderr.String(), `notificationStatus="succeeded"`) || !strings.Contains(stderr.String(), `notificationRunId="distributor-run-1"`) {
t.Fatalf("stderr missing notification fields:\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())
}
if strings.Contains(stdout.String(), "cli-secret-token") || strings.Contains(stderr.String(), "cli-secret-token") {
t.Fatalf("output contains token value\nstdout=%s\nstderr=%s", stdout.String(), stderr.String())
}
}
func TestRunEveningReportsNotificationFailureWithoutToken(t *testing.T) {
func TestRunEveningReportsDoesNotRequirePerReportDistributorToken(t *testing.T) {
server := dailyServer(t)
distributorServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusBadRequest)
_, _ = w.Write([]byte(`{"error":"rejected cli-secret-token","retryable":false}`))
t.Fatalf("unexpected distributor request %s", r.URL.Path)
}))
t.Cleanup(distributorServer.Close)
tempDir := t.TempDir()
scriptoriumPath := writeFakeScriptorium(t, tempDir)
workspaceRoot := filepath.Join(tempDir, "workspace")
configPath := writeTestConfigWithDistributor(t, server, scriptoriumPath, workspaceRoot, distributorServer.URL)
t.Setenv("CLI_DISTRIBUTOR_TOKEN", "cli-secret-token")
var stdout bytes.Buffer
var stderr bytes.Buffer
runner := Runner{Clock: fixedClock()}
@@ -392,26 +380,19 @@ func TestRunEveningReportsNotificationFailureWithoutToken(t *testing.T) {
"run", "evening",
"--config", configPath,
}, &stdout, &stderr)
if err == nil {
t.Fatal("Run() error = nil, want notification failure")
if err != nil {
t.Fatalf("Run() error = %v", err)
}
var summary app.BatchResult
if decodeErr := json.Unmarshal(stdout.Bytes(), &summary); decodeErr != nil {
t.Fatalf("decode summary: %v\n%s", decodeErr, stdout.String())
}
if len(summary.Reports) != 1 || summary.Reports[0].NotificationStatus != "failed" {
t.Fatalf("summary reports = %#v, want failed notification", summary.Reports)
if len(summary.Reports) != 1 {
t.Fatalf("summary reports = %#v, want one report", summary.Reports)
}
for _, output := range []string{stdout.String(), stderr.String(), err.Error()} {
if strings.Contains(output, "cli-secret-token") {
t.Fatalf("output contains token value:\n%s", output)
}
}
for _, output := range []string{stdout.String(), stderr.String()} {
if !strings.Contains(output, "[redacted]") {
t.Fatalf("output missing redaction marker:\n%s", output)
}
if summary.Reports[0].NotificationStatus != "" || summary.Reports[0].NotificationError != "" {
t.Fatalf("notification fields = %#v, want empty per-report notification fields", summary.Reports[0])
}
}