Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions cmd/multi_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,7 @@ func confirmPurchaseRun(recs []common.Recommendation, isDryRun bool) bool {
func writeReportAndSummary(passed []common.Recommendation, allResults []common.PurchaseResult, isDryRun bool, cfg Config, drops *common.DropSummary) {
serviceStats := buildServiceStats(passed, allResults)
finalCSVOutput := generateCSVFilename(isDryRun, cfg)
if err := writeMultiServiceCSVReport(allResults, finalCSVOutput); err != nil {
log.Printf("Warning: Failed to write CSV output: %v", err)
} else {
AppLogger.Printf("\n馃搵 CSV report written to: %s\n", finalCSVOutput)
}
writeCSVReport(allResults, finalCSVOutput)
printDropSummary(drops)
printMultiServiceSummary(passed, allResults, serviceStats, isDryRun)
}
Expand Down Expand Up @@ -812,11 +808,7 @@ func runCSVPurchases(ctx context.Context, awsCfg aws.Config, recs []common.Recom
finalCSVOutput := generateCSVFilename(isDryRun, cfg)

// Write CSV report
if err := writeMultiServiceCSVReport(allResults, finalCSVOutput); err != nil {
log.Printf("Warning: Failed to write CSV output: %v", err)
} else {
AppLogger.Printf("\n馃搵 CSV report written to: %s\n", finalCSVOutput)
}
writeCSVReport(allResults, finalCSVOutput)

// Print final summary using the post-dedup slice so counts match what was
// actually processed, not the pre-dedup input passed into the outer loop.
Expand Down
39 changes: 33 additions & 6 deletions cmd/multi_service_csv.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,24 @@ func parseCSVFloat(record []string, colIdx map[string]int, fieldName string, tar
return nil
}

// writeMultiServiceCSVReport writes purchase results to a CSV file.
func writeMultiServiceCSVReport(results []common.PurchaseResult, filepath string) error {
// writeCSVReport writes the purchase report and tells the operator whether a
// file exists: "written to" is printed only after a complete, flushed and
// closed write. A run with no results writes no file, so it says so instead.
func writeCSVReport(results []common.PurchaseResult, path string) {
if len(results) == 0 {
AppLogger.Printf("\n馃搵 No purchase results; CSV report not written.\n")
return
}
if err := writeMultiServiceCSVReport(results, path); err != nil {
log.Printf("Warning: Failed to write CSV output: %v", err)
return
}
AppLogger.Printf("\n馃搵 CSV report written to: %s\n", path)
}

// writeMultiServiceCSVReport writes purchase results to a CSV file. It is a
// no-op returning nil when results is empty (no file is created).
func writeMultiServiceCSVReport(results []common.PurchaseResult, filepath string) (err error) {
if len(results) == 0 {
return nil
}
Expand All @@ -265,13 +281,20 @@ func writeMultiServiceCSVReport(results []common.PurchaseResult, filepath string
return fmt.Errorf("failed to create CSV file: %w", err)
}
defer func() {
if err := file.Close(); err != nil {
log.Printf("Warning: failed to close CSV file %s: %v", filepath, err)
// A failed close can mean the data never reached the disk (e.g. on
// a network mount), so it fails the write rather than a warning.
if cerr := file.Close(); cerr != nil && err == nil {
err = fmt.Errorf("failed to close CSV file: %w", cerr)
}
}()

writer := csv.NewWriter(file)
defer writer.Flush()
return writeMultiServiceCSV(file, results)
}

// writeMultiServiceCSV renders the purchase report to w, flushing before it
// returns so a failed flush is returned rather than lost in a defer.
func writeMultiServiceCSV(w io.Writer, results []common.PurchaseResult) error {
writer := csv.NewWriter(w)

// Write header. RecommendedCount shows AWS's pre-sizing count alongside
// Count (the post-sizing value); UpfrontPayment is rec.CommitmentCost,
Expand Down Expand Up @@ -364,6 +387,10 @@ func writeMultiServiceCSVReport(results []common.PurchaseResult, filepath string
}
}

writer.Flush()
if err := writer.Error(); err != nil {
return fmt.Errorf("failed to flush CSV file: %w", err)
}
return nil
}

Expand Down
57 changes: 57 additions & 0 deletions cmd/multi_service_csv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -854,3 +854,60 @@ func TestLoadRecommendationsFromCSV_FileErrors(t *testing.T) {
})
}
}

// failingWriter rejects every write, standing in for a full disk or a
// network mount that fails when the buffered CSV is flushed.
type failingWriter struct{}

func (failingWriter) Write([]byte) (int, error) { return 0, errors.New("disk full") }

// Issue #2004: csv.Writer buffers everything, so the bytes only reach the
// file at Flush. A Flush failure must surface instead of a nil return.
func TestWriteMultiServiceCSV_FlushErrorIsReturned(t *testing.T) {
results := []common.PurchaseResult{{
Recommendation: common.Recommendation{Service: common.ServiceRDS, Region: "us-east-1", ResourceType: "db.t3.small", Count: 1},
Success: true,
Timestamp: time.Now(),
}}
err := writeMultiServiceCSV(failingWriter{}, results)
require.Error(t, err)
assert.Contains(t, err.Error(), "disk full")
}

// Issue #2004: a run with no purchase results writes no file, so it must
// not claim "CSV report written to".
func TestWriteReportAndSummary_NoResultsDoesNotClaimCSVWritten(t *testing.T) {
var logs strings.Builder
previous := AppLogger.Writer()
AppLogger.SetOutput(&logs)
t.Cleanup(func() { AppLogger.SetOutput(previous) })

path := filepath.Join(t.TempDir(), "report.csv")
writeReportAndSummary(nil, nil, true, Config{CSVOutput: path}, nil)

_, statErr := os.Stat(path)
require.ErrorIs(t, statErr, os.ErrNotExist)
assert.NotContains(t, logs.String(), "CSV report written to")
assert.Contains(t, logs.String(), "CSV report not written")
}

// Issue #2004: a failed write must not be reported as written either.
func TestWriteCSVReport_FailureDoesNotClaimCSVWritten(t *testing.T) {
var logs strings.Builder
previous := AppLogger.Writer()
AppLogger.SetOutput(&logs)
t.Cleanup(func() { AppLogger.SetOutput(previous) })

results := []common.PurchaseResult{{
Recommendation: common.Recommendation{Service: common.ServiceRDS, Region: "us-east-1", ResourceType: "db.t3.small", Count: 1},
Success: true,
Timestamp: time.Now(),
}}
writeCSVReport(results, filepath.Join(t.TempDir(), "missing-dir", "report.csv"))
assert.NotContains(t, logs.String(), "CSV report written to")

logs.Reset()
path := filepath.Join(t.TempDir(), "report.csv")
writeCSVReport(results, path)
assert.Contains(t, logs.String(), "CSV report written to: "+path)
}
Loading