diff --git a/internal/command/updater.go b/internal/command/updater.go index d46b56d1a..6dd049459 100644 --- a/internal/command/updater.go +++ b/internal/command/updater.go @@ -16,6 +16,34 @@ import ( "github.com/CustomResourceDefinition/catalog/internal/timing" ) +type teeWriter struct { + Writers []io.Writer +} + +func (t teeWriter) Write(p []byte) (n int, err error) { + for _, w := range t.Writers { + if _, err := w.Write(p); err != nil { + return n, err + } + n = len(p) + } + return n, nil +} + +func (cmd Updater) createSummaryWriter() (io.Writer, func(), error) { + summaryPath := os.Getenv("GITHUB_STEP_SUMMARY") + if summaryPath == "" { + return cmd.Logger, func() {}, nil + } + + f, err := os.OpenFile(summaryPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + return nil, nil, err + } + + return teeWriter{Writers: []io.Writer{cmd.Logger, f}}, func() { f.Close() }, nil +} + type Updater struct { Configuration, Schema, Definitions string Logger io.Writer @@ -105,7 +133,12 @@ func (cmd Updater) Run() error { } } - totalStats.PrintSummary(cmd.Logger) + writer, closer, err := cmd.createSummaryWriter() + if err != nil { + return err + } + defer closer() + totalStats.PrintSummary(writer) return merge(tmpDir, cmd.Schema) } diff --git a/internal/command/updater_test.go b/internal/command/updater_test.go index cccbedda1..e99903ea4 100644 --- a/internal/command/updater_test.go +++ b/internal/command/updater_test.go @@ -2,6 +2,8 @@ package command import ( "bytes" + "fmt" + "io" "net/http" "net/http/httptest" "os" @@ -328,9 +330,9 @@ func TestRunAggregatesStats(t *testing.T) { outStr := output.String() assert.Contains(t, outStr, "Update Statistics") - assert.Contains(t, outStr, "Overall:") + assert.Contains(t, outStr, "**Overall:**") assert.Contains(t, outStr, "operations") - assert.Contains(t, outStr, "Http:") + assert.Contains(t, outStr, "#### Http") assert.Contains(t, outStr, "api_fetch") } @@ -417,6 +419,107 @@ func TestRunWithPerformanceLog(t *testing.T) { perfContent, err := os.ReadFile(logPath) assert.Nil(t, err) assert.NotEmpty(t, string(perfContent)) - assert.Contains(t, string(perfContent), "http") - assert.Contains(t, string(perfContent), "api_fetch") +} + +func TestTeeWriter(t *testing.T) { + var buf1, buf2 bytes.Buffer + tw := teeWriter{Writers: []io.Writer{&buf1, &buf2}} + + n, err := tw.Write([]byte("hello world")) + assert.Nil(t, err) + assert.Equal(t, 11, n) + + assert.Equal(t, "hello world", buf1.String()) + assert.Equal(t, "hello world", buf2.String()) +} + +func TestTeeWriterPartialError(t *testing.T) { + var buf1 bytes.Buffer + errorWriter := errorWriter{err: io.EOF} + tw := teeWriter{Writers: []io.Writer{&buf1, errorWriter}} + + _, err := tw.Write([]byte("hello")) + assert.Error(t, err) + assert.Equal(t, io.EOF, err) +} + +type errorWriter struct { + err error +} + +func (e errorWriter) Write(p []byte) (n int, err error) { + return 0, e.err +} + +func TestCreateSummaryWriterNoEnv(t *testing.T) { + original := os.Getenv("GITHUB_STEP_SUMMARY") + os.Unsetenv("GITHUB_STEP_SUMMARY") + defer func() { + if original != "" { + os.Setenv("GITHUB_STEP_SUMMARY", original) + } + }() + + buf := bytes.NewBuffer([]byte{}) + updater := Updater{Logger: buf} + + writer, closer, err := updater.createSummaryWriter() + assert.Nil(t, err) + assert.Equal(t, buf, writer) + closer() +} + +func TestCreateSummaryWriterWithEnv(t *testing.T) { + original := os.Getenv("GITHUB_STEP_SUMMARY") + tmpDir := t.TempDir() + summaryPath := path.Join(tmpDir, "summary.md") + os.Setenv("GITHUB_STEP_SUMMARY", summaryPath) + defer func() { + os.Unsetenv("GITHUB_STEP_SUMMARY") + if original != "" { + os.Setenv("GITHUB_STEP_SUMMARY", original) + } + }() + + buf := bytes.NewBuffer([]byte{}) + updater := Updater{Logger: buf} + + writer, closer, err := updater.createSummaryWriter() + assert.Nil(t, err) + closer() + + _, ok := writer.(teeWriter) + assert.True(t, ok) + + content, err := os.ReadFile(summaryPath) + assert.Nil(t, err) + assert.Empty(t, string(content)) +} + +func TestCreateSummaryWriterAppendsToFile(t *testing.T) { + original := os.Getenv("GITHUB_STEP_SUMMARY") + tmpDir := t.TempDir() + summaryPath := path.Join(tmpDir, "summary.md") + os.WriteFile(summaryPath, []byte("Existing content\n"), 0644) + os.Setenv("GITHUB_STEP_SUMMARY", summaryPath) + defer func() { + os.Unsetenv("GITHUB_STEP_SUMMARY") + if original != "" { + os.Setenv("GITHUB_STEP_SUMMARY", original) + } + }() + + buf := bytes.NewBuffer([]byte{}) + updater := Updater{Logger: buf} + + writer, closer, err := updater.createSummaryWriter() + assert.Nil(t, err) + + fmt.Fprintf(writer, "New content") + closer() + + content, err := os.ReadFile(summaryPath) + assert.Nil(t, err) + assert.Contains(t, string(content), "Existing content") + assert.Contains(t, string(content), "New content") } diff --git a/internal/timing/stats.go b/internal/timing/stats.go index 18097222d..7524469dc 100644 --- a/internal/timing/stats.go +++ b/internal/timing/stats.go @@ -235,8 +235,8 @@ func (s *Stats) PrintSummary(writer io.Writer) { s.mu.RLock() defer s.mu.RUnlock() - fmt.Fprintf(writer, "\n=== Update Statistics ===\n\n") - fmt.Fprintf(writer, "Overall: %s (%d operations)\n\n", formatDuration(s.totalTime), s.totalOps) + fmt.Fprintf(writer, "\n### Update Statistics\n\n") + fmt.Fprintf(writer, "**Overall:** %s (%d operations)\n\n", formatDuration(s.totalTime), s.totalOps) categoryOrder := []Category{CategoryHTTP, CategoryGit, CategoryHelm, CategoryOCI, CategoryGeneration, CategoryMisc} @@ -252,7 +252,9 @@ func (s *Stats) PrintSummary(writer io.Writer) { typeStats[op.Type] = append(typeStats[op.Type], op.Duration) } - fmt.Fprintf(writer, "%s:\n", strings.ToUpper(string(cat)[:1])+string(cat)[1:]) + fmt.Fprintf(writer, "#### %s\n\n", strings.ToUpper(string(cat)[:1])+string(cat)[1:]) + fmt.Fprintf(writer, "| Operation | Count | Total | p75 | p90 | p95 |\n") + fmt.Fprintf(writer, "|-----------|-------|-------|-----|-----|-----|\n") for opType, durations := range typeStats { if len(durations) == 0 { @@ -266,23 +268,19 @@ func (s *Stats) PrintSummary(writer io.Writer) { durationsSecs[i] = d.Seconds() } - percs := calculatePercentiles(durationsSecs, []float64{0.75, 0.90, 0.95}) + percentiles := calculatePercentiles(durationsSecs, []float64{0.75, 0.90, 0.95}) typeLabel := string(opType) - fmt.Fprintf(writer, " %-10s %4d operations total: %s", - typeLabel+":", + p75 := formatDuration(percentiles[0.75]) + p90 := formatDuration(percentiles[0.90]) + p95 := formatDuration(percentiles[0.95]) + + fmt.Fprintf(writer, "| %s | %d | %s | %s | %s | %s |\n", + typeLabel, len(durations), formatDuration(total), + p75, p90, p95, ) - - if len(percs) > 0 { - fmt.Fprintf(writer, " p75: %s p90: %s p95: %s", - formatDuration(percs[0.75]), - formatDuration(percs[0.90]), - formatDuration(percs[0.95]), - ) - } - fmt.Fprintln(writer) } fmt.Fprintln(writer) } diff --git a/internal/timing/stats_test.go b/internal/timing/stats_test.go index 898ed07db..8ba7dc121 100644 --- a/internal/timing/stats_test.go +++ b/internal/timing/stats_test.go @@ -254,6 +254,6 @@ func TestPrintSummaryWriter(t *testing.T) { s.PrintSummary(&buf) assert.Contains(t, buf.String(), "Update Statistics") - assert.Contains(t, buf.String(), "Overall:") - assert.Contains(t, buf.String(), "Http:") + assert.Contains(t, buf.String(), "**Overall:**") + assert.Contains(t, buf.String(), "#### Http") }