diff --git a/internal/app/app.go b/internal/app/app.go index 243e1ab..2b51663 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -154,13 +154,14 @@ func (e BatchError) Error() string { if e.Result == nil { return "batch failed" } - if batchNotificationFailed(e.Result) && batchReportFailures(e.Result) == 0 { + failedReports := batchReportFailures(e.Result) + if batchNotificationFailed(e.Result) && failedReports == 0 { if e.Result.Notification.Error != "" { return fmt.Sprintf("batch %s notification failed: %s", e.Result.Batch, e.Result.Notification.Error) } return fmt.Sprintf("batch %s notification failed", e.Result.Batch) } - return fmt.Sprintf("batch %s failed: %d of %d reports failed", e.Result.Batch, e.Result.Failed, e.Result.Total) + return fmt.Sprintf("batch %s failed: %d of %d reports failed", e.Result.Batch, failedReports, len(e.Result.Reports)) } func batchNotificationFailed(result *BatchResult) bool { @@ -291,7 +292,7 @@ func RunBatch(ctx context.Context, req BatchRequest) error { if err != nil { return err } - if result.Failed > 0 { + if result.Failed > 0 || batchNotificationFailed(result) { return BatchError{Result: result} } return nil @@ -370,13 +371,10 @@ func RunBatchDetailed(ctx context.Context, req BatchRequest) (*BatchResult, erro result.Reports = append(result.Reports, item) } result.Total = len(result.Reports) - batchNotification, err := notifyBatch(ctx, req.Config, req.Batch, batchRunID(startedAt, req.Batch), startedAt, result, plannedReports, req.Notifier) + batchNotification, _ := notifyBatch(ctx, req.Config, req.Batch, batchRunID(startedAt, req.Batch), startedAt, result, plannedReports, req.Notifier) if batchNotification != nil { result.Notification = batchNotification } - if err != nil { - result.Failed++ - } result.FinishedAt = time.Now() return result, nil } diff --git a/internal/app/batch_generation_test.go b/internal/app/batch_generation_test.go index f2be9e7..b478f5e 100644 --- a/internal/app/batch_generation_test.go +++ b/internal/app/batch_generation_test.go @@ -2,8 +2,10 @@ package app import ( "context" + "errors" "os" "path/filepath" + "strings" "testing" "gitea.maximumdirect.net/eric/weatherreporter/internal/config" @@ -95,6 +97,43 @@ func TestRunBatchDetailedPreflightsAllOutputPaths(t *testing.T) { } } +func TestRunBatchDetailedRetainsReportCountsWhenNotificationFails(t *testing.T) { + bundle := generationBundle(t) + bundle.Hourly.Periods = bundle.Hourly.Periods[:1] + outputDir := t.TempDir() + notifier := &generationNotifier{batchErr: errors.New("distributor unavailable")} + result, err := RunBatchDetailed(context.Background(), BatchRequest{ + Config: generationDistributorConfig(), Batch: BatchMorning, + Now: generationTime("2026-05-29T08:30:00-05:00"), WorkingDir: t.TempDir(), OutputDir: outputDir, + Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{}, Notifier: notifier, + }) + if err != nil || result == nil || result.Total != len(result.Reports) || result.Succeeded != len(result.Reports) || result.Failed != 0 || result.Notification == nil || result.Notification.Status != "failed" { + t.Fatalf("RunBatchDetailed() result/error = %#v/%v", result, err) + } + for _, item := range result.Reports { + if item.Status != "succeeded" || item.OutputPath == "" { + t.Fatalf("report result = %#v", item) + } + if _, statErr := os.Stat(item.OutputPath); statErr != nil { + t.Fatalf("published output %q: %v", item.OutputPath, statErr) + } + } +} + +func TestRunBatchReturnsNotificationFailureWithoutReportFailureWording(t *testing.T) { + bundle := generationBundle(t) + bundle.Hourly.Periods = bundle.Hourly.Periods[:1] + err := RunBatch(context.Background(), BatchRequest{ + Config: generationDistributorConfig(), Batch: BatchMorning, + Now: generationTime("2026-05-29T08:30:00-05:00"), WorkingDir: t.TempDir(), OutputDir: t.TempDir(), + Collector: &generationCollector{bundle: &bundle}, Executor: &generationExecutor{}, Notifier: &generationNotifier{batchErr: errors.New("distributor unavailable")}, + }) + var batchErr BatchError + if !errors.As(err, &batchErr) || batchErr.Result == nil || batchErr.Result.Failed != 0 || batchErr.Result.Notification == nil || batchErr.Result.Notification.Status != "failed" || !strings.Contains(err.Error(), "notification failed") || strings.Contains(err.Error(), "reports failed") { + t.Fatalf("RunBatch() error/result = %v/%#v", err, batchErr.Result) + } +} + func generationDistributorConfig() config.Config { cfg := generationConfig() cfg.Notify.Distributor.Enabled = true diff --git a/internal/app/generation_test.go b/internal/app/generation_test.go index 4702c56..c2aea11 100644 --- a/internal/app/generation_test.go +++ b/internal/app/generation_test.go @@ -243,6 +243,7 @@ func generationBundlePointer(t *testing.T) *weatherdata.Bundle { type generationNotifier struct { err error + batchErr error request NotificationRequest batchRequest batchNotificationRequest batchCalls int @@ -261,7 +262,7 @@ func (n *generationNotifier) NotifyBatch(_ context.Context, request batchNotific return nil, err } } - return &NotificationResult{Status: "succeeded", PipelineID: request.PipelineID, BundleID: request.BundleID}, nil + return &NotificationResult{Status: "succeeded", PipelineID: request.PipelineID, BundleID: request.BundleID}, n.batchErr } func generationBundle(t *testing.T) weatherdata.Bundle { diff --git a/internal/cli/root.go b/internal/cli/root.go index 3a25565..b02b274 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -40,10 +40,11 @@ Options: ` type Runner struct { - Clock timeutil.Clock - ExecutorFactory ExecutorFactory - Version string - WorkingDir string + Clock timeutil.Clock + ExecutorFactory ExecutorFactory + Version string + WorkingDir string + runBatchDetailed func(context.Context, app.BatchRequest) (*app.BatchResult, error) } func Run(ctx context.Context, args []string, stdout io.Writer, stderr io.Writer) error { @@ -89,7 +90,11 @@ func (r Runner) Run(ctx context.Context, args []string, stdout io.Writer, stderr if err != nil { return err } - result, err := app.RunBatchDetailed(ctx, req) + runBatchDetailed := r.runBatchDetailed + if runBatchDetailed == nil { + runBatchDetailed = app.RunBatchDetailed + } + result, err := runBatchDetailed(ctx, req) if result != nil { summary := newBatchSummary(result) if encodeErr := writeActionResult(stdout, stderr, summary, outputOptions{Quiet: opts.Quiet}, func(w io.Writer) { diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index 5bc977c..48f9510 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -3,12 +3,15 @@ package cli import ( "bytes" "context" + "encoding/json" + "errors" "os" "path/filepath" "strings" "testing" "time" + "gitea.maximumdirect.net/eric/weatherreporter/internal/app" "gitea.maximumdirect.net/eric/weatherreporter/internal/promptexec" "gitea.maximumdirect.net/eric/weatherreporter/internal/timeutil" ) @@ -80,6 +83,43 @@ func TestResolveGenerateActionUsesInjectedWorkingDirectoryForOutputOverrides(t * } } +func TestRunActionReturnsFailureForBatchNotificationFailure(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "config.yml") + if err := os.WriteFile(configPath, []byte("weather_api:\n base_url: https://weather.api.example.com/\n"), 0o600); err != nil { + t.Fatal(err) + } + result := &app.BatchResult{ + Batch: app.BatchMorning, Total: 2, Succeeded: 2, + Reports: []app.BatchReportResult{ + {ReportID: "today", Status: "succeeded", OutputPath: "/reports/today.md"}, + {ReportID: "tomorrow", Status: "succeeded", OutputPath: "/reports/tomorrow.md"}, + }, + Notification: &app.BatchNotificationResult{Status: "failed", Error: "distributor unavailable"}, + } + var stdout, stderr bytes.Buffer + runner := Runner{ + Clock: timeutil.FixedClock{Time: time.Date(2026, 5, 29, 8, 0, 0, 0, time.UTC)}, + ExecutorFactory: func(PromptExecutorConfig) (promptexec.Executor, error) { + return &factoryExecutor{}, nil + }, + runBatchDetailed: func(context.Context, app.BatchRequest) (*app.BatchResult, error) { + return result, nil + }, + } + err := runner.Run(context.Background(), []string{"run", "morning", "--config", configPath}, &stdout, &stderr) + var batchErr app.BatchError + if !errors.As(err, &batchErr) || !strings.Contains(err.Error(), "notification failed") { + t.Fatalf("Run() error = %v", err) + } + var summary batchSummary + if err := json.Unmarshal(stdout.Bytes(), &summary); err != nil { + t.Fatalf("decode summary: %v", err) + } + if summary.Status != summaryStatusFailed || summary.Total != 2 || summary.Succeeded != 2 || summary.Failed != 0 || summary.Notification == nil || summary.Notification.Status != "failed" { + t.Fatalf("summary = %#v", summary) + } +} + func TestInspectCommandIsUnknownAndAbsentFromHelp(t *testing.T) { var stdout, stderr bytes.Buffer err := (Runner{}).Run(context.Background(), []string{"inspect", "reports"}, &stdout, &stderr)