diff --git a/.cursor/rules.md b/.cursor/rules.md new file mode 100644 index 00000000..43181880 --- /dev/null +++ b/.cursor/rules.md @@ -0,0 +1,52 @@ +# Project Rules + +## Before making code changes + +- Discuss the design with the user +- Show example changes to make sure we are in the right direction +- If the user request is not clear, ask for more details +- List possible alternatives to make the change + +## Making code changes + +- Do the minimal change needed, no extra code that is not needed right now +- Avoid unrelated changes (spelling, whitespace, etc.) +- Keep changes small to make human review easy and avoid mistakes + +## After making code changes + +- Run `make fmt` to format code with golangci-lint formatters +- Run `make test` to verify all tests pass +- Running specific package tests (e.g., `go test ./pkg/test/...`) is fine for quick local verification + +## File organization + +- Keep files focused - separate files for different concerns (e.g., `html.go`, `yaml.go`, `summary.go`) +- All files need SPDX license headers - check existing files for the format + +## Error handling + +- Check existing code for error formatting conventions + +## Testing + +- Use `helpers.FakeTime(t)` for time-dependent tests to ensure reproducibility + +## Commit messages + +When the user wants to commit changes, suggest a commit message. + +The main purpose is to explain why the change was made - what are we trying to do. + +Content guidelines: +- Explain how the change affects the user - what is the new or modified behavior +- If the change affects performance, include measurements and description of how we measured +- If the change modifies the output, include example output with and without the change +- If the change introduces new logs, show example logs including the changed or new logs +- If several alternatives were considered, explain why we chose the particular solution +- Discuss the negative effects of the change if any +- If the change includes new APIs, describe the new APIs and how they are used +- Avoid describing details that are best seen in the diff + +Footer: +- Include `Assisted-by: Cursor/{model-name}` footer diff --git a/pkg/command/command.go b/pkg/command/command.go index a027f8dd..2529b607 100644 --- a/pkg/command/command.go +++ b/pkg/command/command.go @@ -15,9 +15,9 @@ import ( e2eenv "github.com/ramendr/ramen/e2e/env" "github.com/ramendr/ramen/e2e/types" "go.uber.org/zap" - "sigs.k8s.io/yaml" "github.com/ramendr/ramenctl/pkg/console" + "github.com/ramendr/ramenctl/pkg/report" ) // Command is a ramenctl generic command used by all ramenctl commands. Note that the config is not @@ -134,14 +134,33 @@ func (c *Command) Close() { c.closeLog() } -// WriteReport writes report in yaml format to the command output directory. -func (c *Command) WriteReport(report any) error { - data, err := yaml.Marshal(report) +// OpenReport opens a report file for writing in the specified format. +func (c *Command) OpenReport(format string) (*os.File, error) { + path := filepath.Join(c.outputDir, c.name+"."+format) + file, err := os.Create(path) if err != nil { - return fmt.Errorf("failed to marshal report: %w", err) + return nil, fmt.Errorf("failed to create report file %s: %w", path, err) + } + return file, nil +} + +// WriteYAMLReport writes any report as YAML to the command output directory. +func (c *Command) WriteYAMLReport(r any) { + file, err := c.OpenReport("yaml") + if err != nil { + console.Error("failed to open report file: %s", err) + return + } + defer file.Close() + + if err := report.WriteYAML(file, r); err != nil { + console.Error("failed to write report: %s", err) + return + } + + if err := file.Close(); err != nil { + console.Error("failed to close report file: %s", err) } - path := filepath.Join(c.outputDir, c.name+".yaml") - return os.WriteFile(path, data, 0o640) } func logName(commandName string) string { diff --git a/pkg/gather/command.go b/pkg/gather/command.go index 5ec642b6..097c3d31 100644 --- a/pkg/gather/command.go +++ b/pkg/gather/command.go @@ -280,16 +280,12 @@ func (c Command) withTimeout(d stdtime.Duration) (*Command, context.CancelFunc) } func (c *Command) failed() error { - if err := c.command.WriteReport(c.report); err != nil { - console.Error("failed to write report: %s", err) - } + c.command.WriteYAMLReport(c.report) return fmt.Errorf("Gather %s", c.report.Status) } func (c *Command) passed() { - if err := c.command.WriteReport(c.report); err != nil { - console.Error("failed to write report: %s", err) - } + c.command.WriteYAMLReport(c.report) console.Completed("Gather completed") } diff --git a/pkg/report/report.go b/pkg/report/report.go index e9b4b938..a0b6fa1e 100644 --- a/pkg/report/report.go +++ b/pkg/report/report.go @@ -51,6 +51,9 @@ type Base struct { Status Status `json:"status,omitempty"` Duration float64 `json:"duration,omitempty"` Steps []*Step `json:"steps"` + + // Summary is set by `validate` and `test` commands. + Summary *Summary `json:"summary,omitempty"` } // Application is application info. @@ -128,9 +131,15 @@ func (r *Base) Equal(o *Base) bool { if r.Duration != o.Duration { return false } - return slices.EqualFunc(r.Steps, o.Steps, func(a *Step, b *Step) bool { + if !slices.EqualFunc(r.Steps, o.Steps, func(a *Step, b *Step) bool { return a.Equal(b) - }) + }) { + return false + } + if !r.Summary.Equal(o.Summary) { + return false + } + return true } // AddStep adds a step to the report. diff --git a/pkg/report/report_test.go b/pkg/report/report_test.go index 0b28b397..c9c9ba88 100644 --- a/pkg/report/report_test.go +++ b/pkg/report/report_test.go @@ -162,6 +162,23 @@ func TestBaseNotEqual(t *testing.T) { t.Error("reports with different step should not be equal") } }) + t.Run("summary", func(t *testing.T) { + r1 := report.NewBase("name") + r1.Summary = &report.Summary{report.SummaryKey("ok"): 5} + r2 := report.NewBase("name") + r2.Summary = &report.Summary{report.SummaryKey("ok"): 3} + if r1.Equal(r2) { + t.Error("reports with different summary should not be equal") + } + }) + t.Run("nil vs non-nil summary", func(t *testing.T) { + r1 := report.NewBase("name") + r2 := report.NewBase("name") + r2.Summary = &report.Summary{} + if r1.Equal(r2) { + t.Error("reports with nil vs non-nil summary should not be equal") + } + }) } func TestBaseDuration(t *testing.T) { diff --git a/pkg/report/summary.go b/pkg/report/summary.go new file mode 100644 index 00000000..d3cc7ce9 --- /dev/null +++ b/pkg/report/summary.go @@ -0,0 +1,35 @@ +// SPDX-FileCopyrightText: The RamenDR authors +// SPDX-License-Identifier: Apache-2.0 + +package report + +import "maps" + +// SummaryKey is a typed key for Summary counters. +// Each package defines its own keys (e.g., validate.OK, test.Passed). +type SummaryKey string + +// Summary is a counter for report summaries. +type Summary map[SummaryKey]int + +// Add increments the counter for the given key. +func (s Summary) Add(key SummaryKey) { + s[key]++ +} + +// Get returns the count for the given key. +func (s Summary) Get(key SummaryKey) int { + return s[key] +} + +// Equal returns true if both summaries are equal. +// Handles nil pointers: two nil summaries are equal, nil and non-nil are not. +func (s *Summary) Equal(o *Summary) bool { + if s == o { + return true + } + if s == nil || o == nil { + return false + } + return maps.Equal(*s, *o) +} diff --git a/pkg/report/summary_test.go b/pkg/report/summary_test.go new file mode 100644 index 00000000..6466c11f --- /dev/null +++ b/pkg/report/summary_test.go @@ -0,0 +1,58 @@ +// SPDX-FileCopyrightText: The RamenDR authors +// SPDX-License-Identifier: Apache-2.0 + +package report_test + +import ( + "maps" + "testing" + + "sigs.k8s.io/yaml" + + "github.com/ramendr/ramenctl/pkg/report" +) + +func TestTestSummaryYAMLRoundtrip(t *testing.T) { + s1 := report.Summary{ + report.SummaryKey("passed"): 5, + report.SummaryKey("failed"): 2, + report.SummaryKey("skipped"): 1, + report.SummaryKey("canceled"): 0, + } + + data, err := yaml.Marshal(s1) + if err != nil { + t.Fatalf("failed to marshal summary: %v", err) + } + + var s2 report.Summary + if err := yaml.Unmarshal(data, &s2); err != nil { + t.Fatalf("failed to unmarshal summary: %v", err) + } + + if !maps.Equal(s1, s2) { + t.Errorf("expected %v, got %v", s1, s2) + } +} + +func TestValidationSummaryYAMLRoundtrip(t *testing.T) { + s1 := report.Summary{ + report.SummaryKey("ok"): 10, + report.SummaryKey("stale"): 1, + report.SummaryKey("problem"): 2, + } + + data, err := yaml.Marshal(s1) + if err != nil { + t.Fatalf("failed to marshal summary: %v", err) + } + + var s2 report.Summary + if err := yaml.Unmarshal(data, &s2); err != nil { + t.Fatalf("failed to unmarshal summary: %v", err) + } + + if !maps.Equal(s1, s2) { + t.Errorf("expected %v, got %v", s1, s2) + } +} diff --git a/pkg/report/yaml.go b/pkg/report/yaml.go new file mode 100644 index 00000000..f715fd1e --- /dev/null +++ b/pkg/report/yaml.go @@ -0,0 +1,21 @@ +// SPDX-FileCopyrightText: The RamenDR authors +// SPDX-License-Identifier: Apache-2.0 + +package report + +import ( + "fmt" + "io" + + "sigs.k8s.io/yaml" +) + +// WriteYAML writes YAML for any report to the writer. +func WriteYAML(w io.Writer, report any) error { + data, err := yaml.Marshal(report) + if err != nil { + return fmt.Errorf("failed to marshal report: %w", err) + } + _, err = w.Write(data) + return err +} diff --git a/pkg/test/command.go b/pkg/test/command.go index 546f501d..023beadd 100644 --- a/pkg/test/command.go +++ b/pkg/test/command.go @@ -274,17 +274,13 @@ func (c *Command) gatherS3Data() { } func (c *Command) failed() error { - if err := c.command.WriteReport(c.report); err != nil { - console.Error("failed to write report: %s", err) - } - return fmt.Errorf("%s (%s)", c.report.Status, c.report.Summary) + c.command.WriteYAMLReport(c.report) + return fmt.Errorf("%s (%s)", c.report.Status, summaryString(c.report.Summary)) } func (c *Command) passed() { - if err := c.command.WriteReport(c.report); err != nil { - console.Error("failed to write report: %s", err) - } - console.Completed("%s (%s)", c.report.Status, c.report.Summary) + c.command.WriteYAMLReport(c.report) + console.Completed("%s (%s)", c.report.Status, summaryString(c.report.Summary)) } func (c *Command) startStep(name string) { diff --git a/pkg/test/command_test.go b/pkg/test/command_test.go index 1c9fa06b..d30f492c 100644 --- a/pkg/test/command_test.go +++ b/pkg/test/command_test.go @@ -130,7 +130,12 @@ func TestRunPassed(t *testing.T) { t.Fatal(err) } - checkReport(t, test.report, report.Passed, Summary{Passed: len(testConfig.Tests)}) + checkReport( + t, + test.report, + report.Passed, + report.Summary{Passed: len(testConfig.Tests)}, + ) if len(test.report.Steps) != 3 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -153,7 +158,7 @@ func TestRunValidateFailed(t *testing.T) { t.Fatal("command did not fail") } - checkReport(t, test.report, report.Failed, Summary{}) + checkReport(t, test.report, report.Failed, report.Summary{}) if len(test.report.Steps) != 1 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -168,7 +173,7 @@ func TestRunValidateCanceled(t *testing.T) { t.Fatal("command did not fail") } - checkReport(t, test.report, report.Canceled, Summary{}) + checkReport(t, test.report, report.Canceled, report.Summary{}) if len(test.report.Steps) != 1 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -183,7 +188,7 @@ func TestRunSetupFailed(t *testing.T) { t.Fatal("command did not fail") } - checkReport(t, test.report, report.Failed, Summary{}) + checkReport(t, test.report, report.Failed, report.Summary{}) if len(test.report.Steps) != 2 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -200,7 +205,7 @@ func TestRunSetupCanceled(t *testing.T) { t.Fatal("command did not fail") } - checkReport(t, test.report, report.Canceled, Summary{}) + checkReport(t, test.report, report.Canceled, report.Summary{}) if len(test.report.Steps) != 2 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -217,7 +222,12 @@ func TestRunTestsFailed(t *testing.T) { t.Fatal("command did not fail") } - checkReport(t, test.report, report.Failed, Summary{Failed: len(testConfig.Tests)}) + checkReport( + t, + test.report, + report.Failed, + report.Summary{Failed: len(testConfig.Tests)}, + ) if len(test.report.Steps) != 3 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -240,7 +250,12 @@ func TestRunDisappFailed(t *testing.T) { t.Fatal("command did not fail") } - checkReport(t, test.report, report.Failed, Summary{Passed: 4, Failed: 2}) + checkReport( + t, + test.report, + report.Failed, + report.Summary{Passed: 4, Failed: 2}, + ) if len(test.report.Steps) != 3 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -267,7 +282,12 @@ func TestRunTestsCanceled(t *testing.T) { t.Fatal("command did not fail") } - checkReport(t, test.report, report.Canceled, Summary{Canceled: len(testConfig.Tests)}) + checkReport( + t, + test.report, + report.Canceled, + report.Summary{Canceled: len(testConfig.Tests)}, + ) if len(test.report.Steps) != 3 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -290,7 +310,7 @@ func TestCleanPassed(t *testing.T) { t.Fatal(err) } - checkReport(t, test.report, report.Passed, Summary{Passed: 6}) + checkReport(t, test.report, report.Passed, report.Summary{Passed: 6}) if len(test.report.Steps) != 3 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -313,7 +333,7 @@ func TestCleanValidateFailed(t *testing.T) { t.Fatal("command did not fail") } - checkReport(t, test.report, report.Failed, Summary{}) + checkReport(t, test.report, report.Failed, report.Summary{}) if len(test.report.Steps) != 1 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -328,7 +348,7 @@ func TestCleanValidateCanceled(t *testing.T) { t.Fatal("command did not fail") } - checkReport(t, test.report, report.Canceled, Summary{}) + checkReport(t, test.report, report.Canceled, report.Summary{}) if len(test.report.Steps) != 1 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -343,7 +363,12 @@ func TestCleanPurgeFailed(t *testing.T) { t.Fatal("command did not fail") } - checkReport(t, test.report, report.Failed, Summary{Failed: len(testConfig.Tests)}) + checkReport( + t, + test.report, + report.Failed, + report.Summary{Failed: len(testConfig.Tests)}, + ) if len(test.report.Steps) != 2 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -364,7 +389,12 @@ func TestCleanPurgeCanceled(t *testing.T) { t.Fatal("command did not fail") } - checkReport(t, test.report, report.Canceled, Summary{Canceled: len(testConfig.Tests)}) + checkReport( + t, + test.report, + report.Canceled, + report.Summary{Canceled: len(testConfig.Tests)}, + ) if len(test.report.Steps) != 2 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -385,7 +415,12 @@ func TestCleanCleanupFailed(t *testing.T) { t.Fatal("command did not fail") } - checkReport(t, test.report, report.Failed, Summary{Passed: len(testConfig.Tests)}) + checkReport( + t, + test.report, + report.Failed, + report.Summary{Passed: len(testConfig.Tests)}, + ) if len(test.report.Steps) != 3 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -408,7 +443,12 @@ func TestCleanCleanupCanceled(t *testing.T) { t.Fatal("command did not fail") } - checkReport(t, test.report, report.Canceled, Summary{Passed: len(testConfig.Tests)}) + checkReport( + t, + test.report, + report.Canceled, + report.Summary{Passed: len(testConfig.Tests)}, + ) if len(test.report.Steps) != 3 { t.Fatalf("unexpected steps %+v", test.report.Steps) } @@ -437,16 +477,16 @@ func testCommand(t *testing.T, name string, backend rtesting.Testing) *Command { return newCommand(cmd, testConfig, backend) } -func checkReport(t *testing.T, report *Report, status report.Status, summary Summary) { - if report.Status != status { - t.Errorf("expected status %q, got %q", status, report.Status) +func checkReport(t *testing.T, r *Report, status report.Status, summary report.Summary) { + if r.Status != status { + t.Errorf("expected status %q, got %q", status, r.Status) } - if report.Summary != summary { - t.Errorf("expected summary %+v, got %+v", summary, report.Summary) + if !r.Summary.Equal(&summary) { + t.Errorf("expected summary %+v, got %+v", summary, *r.Summary) } - duration := totalDuration(report.Steps) - if report.Duration != duration { - t.Fatalf("expected duration %v, got %v", duration, report.Duration) + duration := totalDuration(r.Steps) + if r.Duration != duration { + t.Fatalf("expected duration %v, got %v", duration, r.Duration) } } diff --git a/pkg/test/report.go b/pkg/test/report.go index 2caf94b5..8d164a6a 100644 --- a/pkg/test/report.go +++ b/pkg/test/report.go @@ -18,27 +18,28 @@ const ( CleanupStep = "cleanup" ) -// Summary summaries a test run or clean. -type Summary struct { - Passed int `json:"passed"` - Failed int `json:"failed"` - Skipped int `json:"skipped"` - Canceled int `json:"canceled"` -} +// Summary keys for test reports. +const ( + Passed = report.SummaryKey("passed") + Failed = report.SummaryKey("failed") + Skipped = report.SummaryKey("skipped") + Canceled = report.SummaryKey("canceled") +) // Report created by test sub commands. type Report struct { *report.Base - Config *e2econfig.Config `json:"config"` - Summary Summary `json:"summary"` + Config *e2econfig.Config `json:"config"` } func newReport(commandName string, config *e2econfig.Config) *Report { if config == nil { panic("config must not be nil") } + base := report.NewBase(commandName) + base.Summary = &report.Summary{} return &Report{ - Base: report.NewBase(commandName), + Base: base, Config: config, } } @@ -50,7 +51,7 @@ func (r *Report) AddStep(step *report.Step) { // Handle the special "tests" step. if step.Name == TestsStep { for _, t := range step.Items { - r.Summary.AddTest(t) + addTest(r.Summary, t) } } } @@ -73,26 +74,24 @@ func (r *Report) Equal(o *Report) bool { } else if r.Config != o.Config { return false } - if r.Summary != o.Summary { - return false - } return true } -func (s *Summary) AddTest(t *report.Step) { +func addTest(s *report.Summary, t *report.Step) { switch t.Status { case report.Passed: - s.Passed++ + s.Add(Passed) case report.Failed: - s.Failed++ + s.Add(Failed) case report.Skipped: - s.Skipped++ + s.Add(Skipped) case report.Canceled: - s.Canceled++ + s.Add(Canceled) } } -func (s Summary) String() string { +func summaryString(s *report.Summary) string { return fmt.Sprintf("%d passed, %d failed, %d skipped, %d canceled", - s.Passed, s.Failed, s.Skipped, s.Canceled) + s.Get(Passed), s.Get(Failed), + s.Get(Skipped), s.Get(Canceled)) } diff --git a/pkg/test/report_test.go b/pkg/test/report_test.go index 30dcbe11..4899b50c 100644 --- a/pkg/test/report_test.go +++ b/pkg/test/report_test.go @@ -59,8 +59,13 @@ func TestReportSummary(t *testing.T) { } r.AddStep(testsStep) - expectedSummary := Summary{Passed: 1, Failed: 1, Skipped: 1, Canceled: 1} - if r.Summary != expectedSummary { + expectedSummary := &report.Summary{ + Passed: 1, + Failed: 1, + Skipped: 1, + Canceled: 1, + } + if !r.Summary.Equal(expectedSummary) { t.Errorf("expected summary %+v, got %+v", expectedSummary, r.Summary) } } @@ -70,7 +75,7 @@ func TestReportEqual(t *testing.T) { // Helper function to create a standard report createReport := func() *Report { r := newReport("test-command", reportConfig) - r.Summary = Summary{Passed: 2} + r.Summary = &report.Summary{Passed: 2} return r } @@ -116,13 +121,6 @@ func TestReportEqual(t *testing.T) { } }) - t.Run("different summary", func(t *testing.T) { - r2 := createReport() - r2.Summary = Summary{Passed: 1, Failed: 1} - if r1.Equal(r2) { - t.Error("reports with different summary should not be equal") - } - }) } func TestReportMarshaling(t *testing.T) { @@ -146,52 +144,43 @@ func TestReportMarshaling(t *testing.T) { Duration: 1.0, }, } - r.Summary = Summary{Passed: 2, Failed: 1} + r.Summary = &report.Summary{Passed: 2, Failed: 1} // Test roundtrip marshaling/unmarshaling checkRoundtrip(t, r) } func TestSummaryString(t *testing.T) { - summary := Summary{Passed: 5, Failed: 2, Skipped: 3, Canceled: 1} - expectedString := "5 passed, 2 failed, 3 skipped, 1 canceled" - if summary.String() != expectedString { - t.Errorf("expected summary string %s, got %s", expectedString, summary.String()) - } -} - -func TestSummaryMarshal(t *testing.T) { - summary := Summary{Passed: 4, Failed: 3, Skipped: 2, Canceled: 1} - - bytes, err := yaml.Marshal(summary) - if err != nil { - t.Fatalf("failed to marshal summary: %v", err) - } - - var unmarshaledSummary Summary - err = yaml.Unmarshal(bytes, &unmarshaledSummary) - if err != nil { - t.Fatalf("failed to unmarshal summary: %v", err) + summary := &report.Summary{ + Passed: 5, + Failed: 2, + Skipped: 3, + Canceled: 1, } - if unmarshaledSummary != summary { - t.Errorf("unmarshaled summary %+v does not match original summary %+v", - unmarshaledSummary, summary) + expectedString := "5 passed, 2 failed, 3 skipped, 1 canceled" + if summaryString(summary) != expectedString { + t.Errorf("expected summary string %s, got %s", expectedString, summaryString(summary)) } } func TestSummaryCount(t *testing.T) { - summary := Summary{} + summary := &report.Summary{} // Add multiple tests of different status - summary.AddTest(&report.Step{Status: report.Passed}) - summary.AddTest(&report.Step{Status: report.Passed}) - summary.AddTest(&report.Step{Status: report.Failed}) - summary.AddTest(&report.Step{Status: report.Skipped}) - summary.AddTest(&report.Step{Status: report.Canceled}) - summary.AddTest(&report.Step{Status: report.Passed}) - - expectedSummary := Summary{Passed: 3, Failed: 1, Skipped: 1, Canceled: 1} - if summary != expectedSummary { + addTest(summary, &report.Step{Status: report.Passed}) + addTest(summary, &report.Step{Status: report.Passed}) + addTest(summary, &report.Step{Status: report.Failed}) + addTest(summary, &report.Step{Status: report.Skipped}) + addTest(summary, &report.Step{Status: report.Canceled}) + addTest(summary, &report.Step{Status: report.Passed}) + + expectedSummary := &report.Summary{ + Passed: 3, + Failed: 1, + Skipped: 1, + Canceled: 1, + } + if !summary.Equal(expectedSummary) { t.Errorf("expected summary %+v, got %+v", expectedSummary, summary) } } diff --git a/pkg/validate/application.go b/pkg/validate/application.go index 1be7800c..67ba57b9 100644 --- a/pkg/validate/application.go +++ b/pkg/validate/application.go @@ -258,11 +258,11 @@ func (c *Command) validateGatheredApplicationData(drpcName, drpcNamespace string c.validateApplicationS3Status(&s.S3) - if c.report.Summary.HasIssues() { + if hasIssues(c.report.Summary) { step.Status = report.Failed msg := "Issues found during validation" console.Error(msg) - log.Errorf("%s: %s", msg, c.report.Summary) + log.Errorf("%s: %s", msg, summaryString(c.report.Summary)) return false } @@ -330,7 +330,7 @@ func (c *Command) validatedApplicationS3ProfileStatus( s.Description = "S3 data not available" } - c.report.Summary.Add(s) + addValidation(c.report.Summary, s) } func (c *Command) validatedApplicationS3Profile( @@ -357,7 +357,7 @@ func (c *Command) validatedApplicationS3Profile( } } - c.report.Summary.Add(&profileStatus.Gathered) + addValidation(c.report.Summary, &profileStatus.Gathered) return profileStatus } @@ -431,7 +431,7 @@ func (c *Command) validatedDRPCPhase(drpc *ramenapi.DRPlacementControl) report.V } } - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) return validated } @@ -452,7 +452,7 @@ func (c *Command) validatedDRPCProgression( validated.State = report.OK } - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) return validated } @@ -471,7 +471,7 @@ func (c *Command) validatedVRGState( validated.State = report.OK } - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) return validated } @@ -488,7 +488,7 @@ func (c *Command) validatedProtectedPVCPhase( validated.State = report.OK } - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) return validated } @@ -500,7 +500,7 @@ func (c *Command) validatedDRPCAction(action string) report.ValidatedString { validated.State = report.Problem validated.Description = fmt.Sprintf("Unknown action %q", action) } - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) return validated } @@ -565,7 +565,7 @@ func (c *Command) validatedVRGConditions( continue } validated := validatedCondition(vrg, condition, metav1.ConditionTrue) - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) conditions = append(conditions, validated) } return conditions @@ -609,7 +609,7 @@ func (c *Command) validatedProtectedPVCConditions( } validated := validatedCondition(vrg, condition, metav1.ConditionTrue) - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) conditions = append(conditions, validated) } return conditions diff --git a/pkg/validate/clusters.go b/pkg/validate/clusters.go index c4bd801b..c622ac63 100644 --- a/pkg/validate/clusters.go +++ b/pkg/validate/clusters.go @@ -140,11 +140,11 @@ func (c *Command) validateGatheredClustersData() bool { c.validateClustersS3Status(&s.S3) - if c.report.Summary.HasIssues() { + if hasIssues(c.report.Summary) { step.Status = report.Failed msg := "Issues found during validation" console.Error(msg) - log.Errorf("%s: %s", msg, c.report.Summary) + log.Errorf("%s: %s", msg, summaryString(c.report.Summary)) return false } @@ -206,7 +206,7 @@ func (c *Command) validateClustersDRPolicies( drPoliciesList.State = report.OK } - c.report.Summary.Add(drPoliciesList) + addValidation(c.report.Summary, drPoliciesList) return nil } @@ -231,7 +231,7 @@ func (c *Command) validatedPeerClasses( } else { peerClassesList.State = report.OK } - c.report.Summary.Add(&peerClassesList) + addValidation(c.report.Summary, &peerClassesList) return peerClassesList } @@ -270,7 +270,7 @@ func (c *Command) validateClustersDRClusters( drClustersList.State = report.OK } - c.report.Summary.Add(drClustersList) + addValidation(c.report.Summary, drClustersList) return nil } @@ -291,7 +291,7 @@ func (c *Command) validatedDRClusterConditions( validated = validatedCondition(drCluster, condition, metav1.ConditionTrue) } - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) conditions = append(conditions, validated) } @@ -409,7 +409,7 @@ func (c *Command) validatedRamenControllerType( validated.State = report.OK } - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) return validated } @@ -441,7 +441,7 @@ func (c *Command) validatedS3Profiles( } else { s.State = report.OK } - c.report.Summary.Add(s) + addValidation(c.report.Summary, s) return nil } @@ -480,7 +480,7 @@ func (c *Command) validatedSecretRef( validated.State = report.OK } - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) return validated, nil } @@ -535,7 +535,7 @@ func (c *Command) validatedDeploymentReplicas( validated.State = report.OK } - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) return validated } @@ -562,7 +562,7 @@ func (c *Command) validatedDeploymentConditions( } validated := validatedDeploymentCondition(condition, expectedStatus) - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) conditions = append(conditions, validated) } @@ -626,7 +626,7 @@ func (c *Command) validatedClustersS3ProfileStatus(s *report.ValidatedClustersS3 s.Description = "No s3 profiles found" } - c.report.Summary.Add(s) + addValidation(c.report.Summary, s) } func (c *Command) validatedClustersS3Profile(result s3.Result) report.ClustersS3ProfileStatus { @@ -651,6 +651,6 @@ func (c *Command) validatedClustersS3Profile(result s3.Result) report.ClustersS3 } } - c.report.Summary.Add(&profileStatus.Accessible) + addValidation(c.report.Summary, &profileStatus.Accessible) return profileStatus } diff --git a/pkg/validate/command.go b/pkg/validate/command.go index 8bb99e78..f429fe97 100644 --- a/pkg/validate/command.go +++ b/pkg/validate/command.go @@ -41,7 +41,7 @@ type Command struct { context context.Context // report describes the command execution. - report *Report + report *report.Report // current validation step. current *report.Step @@ -55,12 +55,14 @@ type Command struct { var _ validation.Context = &Command{} func newCommand(cmd *command.Command, cfg *config.Config, backend validation.Validation) *Command { + r := report.NewReport(cmd.Name(), cfg) + r.Summary = &report.Summary{} return &Command{ command: cmd, config: cfg, backend: backend, context: cmd.Context(), - report: &Report{Report: report.NewReport(cmd.Name(), cfg)}, + report: r, } } @@ -123,7 +125,7 @@ func (c *Command) validatedDeleted(obj client.Object) report.ValidatedBool { validated.State = report.OK } } - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) return validated } @@ -135,7 +137,7 @@ func (c *Command) validatedConditions( for i := range conditions { condition := &conditions[i] validated := validatedCondition(obj, condition, metav1.ConditionTrue) - c.report.Summary.Add(&validated) + addValidation(c.report.Summary, &validated) validatedConditions = append(validatedConditions, validated) } return validatedConditions @@ -182,17 +184,13 @@ func (c *Command) dataDir() string { // Completing commands. func (c *Command) failed() error { - if err := c.command.WriteReport(c.report); err != nil { - console.Error("failed to write report: %s", err) - } - return fmt.Errorf("validation %s (%s)", c.report.Status, c.report.Summary) + c.command.WriteYAMLReport(c.report) + return fmt.Errorf("validation %s (%s)", c.report.Status, summaryString(c.report.Summary)) } func (c *Command) passed() { - if err := c.command.WriteReport(c.report); err != nil { - console.Error("failed to write report: %s", err) - } - console.Completed("Validation completed (%s)", c.report.Summary) + c.command.WriteYAMLReport(c.report) + console.Completed("Validation completed (%s)", summaryString(c.report.Summary)) } // Managing steps. diff --git a/pkg/validate/command_test.go b/pkg/validate/command_test.go index 6084c378..f9cb4497 100644 --- a/pkg/validate/command_test.go +++ b/pkg/validate/command_test.go @@ -265,9 +265,9 @@ func TestValidatedDeleted(t *testing.T) { }) t.Run("update summary", func(t *testing.T) { - expected := Summary{OK: 1, Problem: 2} - if cmd.report.Summary != expected { - t.Fatalf("expected summary %q, got %q", expected, cmd.report.Summary) + expected := report.Summary{OK: 1, Problem: 2} + if !cmd.report.Summary.Equal(&expected) { + t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary) } }) } @@ -315,9 +315,9 @@ func TestValidatedDRPCAction(t *testing.T) { }) t.Run("update summary", func(t *testing.T) { - expected := Summary{OK: 3, Problem: 1} - if cmd.report.Summary != expected { - t.Fatalf("expected summary %q, got %q", expected, cmd.report.Summary) + expected := report.Summary{OK: 3, Problem: 1} + if !cmd.report.Summary.Equal(&expected) { + t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary) } }) } @@ -396,13 +396,13 @@ func TestValidatedDRPCPhaseError(t *testing.T) { } } - var errors uint + var errors int for _, group := range unstable { - errors += uint(len(group.cases)) + errors += len(group.cases) } - expected := Summary{Problem: errors} - if cmd.report.Summary != expected { - t.Fatalf("expected summary %q, got %q", expected, cmd.report.Summary) + expected := report.Summary{Problem: errors} + if !cmd.report.Summary.Equal(&expected) { + t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary) } } @@ -442,9 +442,9 @@ func TestValidatedDRPCPhaseOK(t *testing.T) { }) } - expected := Summary{OK: uint(len(cases))} - if cmd.report.Summary != expected { - t.Fatalf("expected summary %q, got %q", expected, cmd.report.Summary) + expected := report.Summary{OK: len(cases)} + if !cmd.report.Summary.Equal(&expected) { + t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary) } } @@ -470,9 +470,9 @@ func TestValidatedDRPCProgressionOK(t *testing.T) { } }) - expected := Summary{OK: 1} - if cmd.report.Summary != expected { - t.Fatalf("expected summary %q, got %q", expected, cmd.report.Summary) + expected := report.Summary{OK: 1} + if !cmd.report.Summary.Equal(&expected) { + t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary) } } @@ -527,9 +527,9 @@ func TestValidatedDRPCProgressionError(t *testing.T) { }) } - expected := Summary{Problem: uint(len(progressions))} - if cmd.report.Summary != expected { - t.Fatalf("expected summary %q, got %q", expected, cmd.report.Summary) + expected := report.Summary{Problem: len(progressions)} + if !cmd.report.Summary.Equal(&expected) { + t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary) } } @@ -564,9 +564,9 @@ func TestValidatedVRGSTateOK(t *testing.T) { }) } - expected := Summary{OK: uint(len(cases))} - if cmd.report.Summary != expected { - t.Fatalf("expected summary %q, got %q", expected, cmd.report.Summary) + expected := report.Summary{OK: len(cases)} + if !cmd.report.Summary.Equal(&expected) { + t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary) } } @@ -607,9 +607,9 @@ func TestValidatedVRGSTateError(t *testing.T) { }) } - expected := Summary{Problem: uint(len(cases))} - if cmd.report.Summary != expected { - t.Fatalf("expected summary %q, got %q", expected, cmd.report.Summary) + expected := report.Summary{Problem: len(cases)} + if !cmd.report.Summary.Equal(&expected) { + t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary) } } @@ -634,9 +634,9 @@ func TestValidatedProtectedPVCOK(t *testing.T) { } }) - expected := Summary{OK: 1} - if cmd.report.Summary != expected { - t.Fatalf("expected summary %q, got %q", expected, cmd.report.Summary) + expected := report.Summary{OK: 1} + if !cmd.report.Summary.Equal(&expected) { + t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary) } } @@ -674,9 +674,9 @@ func TestValidatedProtectedPVCError(t *testing.T) { }) } - expected := Summary{Problem: uint(len(cases))} - if cmd.report.Summary != expected { - t.Fatalf("expected summary %q, got %q", expected, cmd.report.Summary) + expected := report.Summary{Problem: len(cases)} + if !cmd.report.Summary.Equal(&expected) { + t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary) } } @@ -1100,7 +1100,7 @@ func TestValidateClustersK8s(t *testing.T) { } checkClusterStatus(t, validate.report, expected) - checkSummary(t, validate.report, Summary{OK: 42}) + checkSummary(t, validate.report, report.Summary{OK: 42}) } func TestValidateClustersOcp(t *testing.T) { @@ -1499,7 +1499,7 @@ func TestValidateClustersOcp(t *testing.T) { } checkClusterStatus(t, validate.report, expected) - checkSummary(t, validate.report, Summary{OK: 40}) + checkSummary(t, validate.report, report.Summary{OK: 40}) } func TestValidateClustersValidateFailed(t *testing.T) { @@ -1517,7 +1517,7 @@ func TestValidateClustersValidateFailed(t *testing.T) { checkStep(t, validate.report.Steps[0], "validate config", report.Failed) checkApplicationStatus(t, validate.report, nil) checkClusterStatus(t, validate.report, nil) - checkSummary(t, validate.report, Summary{}) + checkSummary(t, validate.report, report.Summary{}) } func TestValidateClustersValidateCanceled(t *testing.T) { @@ -1535,7 +1535,7 @@ func TestValidateClustersValidateCanceled(t *testing.T) { checkStep(t, validate.report.Steps[0], "validate config", report.Canceled) checkApplicationStatus(t, validate.report, nil) checkClusterStatus(t, validate.report, nil) - checkSummary(t, validate.report, Summary{}) + checkSummary(t, validate.report, report.Summary{}) } func TestValidateClusterGatherClusterFailed(t *testing.T) { @@ -1562,7 +1562,7 @@ func TestValidateClusterGatherClusterFailed(t *testing.T) { checkItems(t, validate.report.Steps[1], items) checkApplicationStatus(t, validate.report, nil) checkClusterStatus(t, validate.report, nil) - checkSummary(t, validate.report, Summary{}) + checkSummary(t, validate.report, report.Summary{}) } func TestValidateClustersInspectS3ProfilesFailed(t *testing.T) { @@ -1593,7 +1593,7 @@ func TestValidateClustersInspectS3ProfilesFailed(t *testing.T) { if validate.report.ClustersStatus == nil { t.Fatal("clusters status is nil") } - checkSummary(t, validate.report, Summary{Problem: 9}) + checkSummary(t, validate.report, report.Summary{Problem: 9}) } func TestValidateClustersCheckS3Failed(t *testing.T) { @@ -1627,7 +1627,11 @@ func TestValidateClustersCheckS3Failed(t *testing.T) { if validate.report.ClustersStatus == nil { t.Fatal("clusters status is nil") } - checkSummary(t, validate.report, Summary{OK: 41, Problem: 1}) + checkSummary( + t, + validate.report, + report.Summary{OK: 41, Problem: 1}, + ) } func TestValidateClustersCheckS3Canceled(t *testing.T) { @@ -1657,7 +1661,7 @@ func TestValidateClustersCheckS3Canceled(t *testing.T) { } checkItems(t, validate.report.Steps[1], items) checkClusterStatus(t, validate.report, nil) - checkSummary(t, validate.report, Summary{}) + checkSummary(t, validate.report, report.Summary{}) } // Validate application tests. @@ -1879,7 +1883,7 @@ func TestValidateApplicationPassed(t *testing.T) { } checkApplicationStatus(t, validate.report, expectedStatus) - checkSummary(t, validate.report, Summary{OK: 24}) + checkSummary(t, validate.report, report.Summary{OK: 24}) } func TestValidateApplicationValidateFailed(t *testing.T) { @@ -1896,7 +1900,7 @@ func TestValidateApplicationValidateFailed(t *testing.T) { } checkStep(t, validate.report.Steps[0], "validate config", report.Failed) checkApplicationStatus(t, validate.report, nil) - checkSummary(t, validate.report, Summary{}) + checkSummary(t, validate.report, report.Summary{}) } func TestValidateApplicationValidateCanceled(t *testing.T) { @@ -1913,7 +1917,7 @@ func TestValidateApplicationValidateCanceled(t *testing.T) { } checkStep(t, validate.report.Steps[0], "validate config", report.Canceled) checkApplicationStatus(t, validate.report, nil) - checkSummary(t, validate.report, Summary{}) + checkSummary(t, validate.report, report.Summary{}) } func TestValidateApplicationInspectApplicationFailed(t *testing.T) { @@ -1937,7 +1941,7 @@ func TestValidateApplicationInspectApplicationFailed(t *testing.T) { } checkItems(t, validate.report.Steps[1], items) checkApplicationStatus(t, validate.report, nil) - checkSummary(t, validate.report, Summary{}) + checkSummary(t, validate.report, report.Summary{}) } func TestValidateApplicationInspectApplicationCanceled(t *testing.T) { @@ -1961,7 +1965,7 @@ func TestValidateApplicationInspectApplicationCanceled(t *testing.T) { } checkItems(t, validate.report.Steps[1], items) checkApplicationStatus(t, validate.report, nil) - checkSummary(t, validate.report, Summary{}) + checkSummary(t, validate.report, report.Summary{}) } func TestValidateApplicationGatherClusterFailed(t *testing.T) { @@ -1988,7 +1992,7 @@ func TestValidateApplicationGatherClusterFailed(t *testing.T) { } checkItems(t, validate.report.Steps[1], items) checkApplicationStatus(t, validate.report, nil) - checkSummary(t, validate.report, Summary{}) + checkSummary(t, validate.report, report.Summary{}) } func TestValidateApplicationInspectS3ProfilesFailed(t *testing.T) { @@ -2018,7 +2022,7 @@ func TestValidateApplicationInspectS3ProfilesFailed(t *testing.T) { {Name: "validate data", Status: report.Failed}, } checkItems(t, validate.report.Steps[1], items) - checkSummary(t, validate.report, Summary{}) + checkSummary(t, validate.report, report.Summary{}) } func TestValidateApplicationGatherS3Failed(t *testing.T) { @@ -2050,7 +2054,11 @@ func TestValidateApplicationGatherS3Failed(t *testing.T) { {Name: "validate data", Status: report.Failed}, } checkItems(t, validate.report.Steps[1], items) - checkSummary(t, validate.report, Summary{OK: 23, Problem: 1}) + checkSummary( + t, + validate.report, + report.Summary{OK: 23, Problem: 1}, + ) } func TestValidateApplicationGatherS3Canceled(t *testing.T) { @@ -2080,7 +2088,7 @@ func TestValidateApplicationGatherS3Canceled(t *testing.T) { {Name: "gather S3 profile \"minio-on-dr2\"", Status: report.Canceled}, } checkItems(t, validate.report.Steps[1], items) - checkSummary(t, validate.report, Summary{}) + checkSummary(t, validate.report, report.Summary{}) } // TODO: Test gather cancellation when kubectl-gahter supports it: @@ -2117,16 +2125,16 @@ func checkReport(t *testing.T, cmd *Command, status report.Status) { } } -func checkApplication(t *testing.T, report *Report, expected *report.Application) { - if !reflect.DeepEqual(expected, report.Application) { - diff := helpers.UnifiedDiff(t, expected, report.Application) +func checkApplication(t *testing.T, r *report.Report, expected *report.Application) { + if !reflect.DeepEqual(expected, r.Application) { + diff := helpers.UnifiedDiff(t, expected, r.Application) t.Fatalf("applications not equal\n%s", diff) } } -func checkNamespaces(t *testing.T, report *Report, expected []string) { - if !slices.Equal(report.Namespaces, expected) { - t.Fatalf("expected namespaces %q, got %q", expected, report.Namespaces) +func checkNamespaces(t *testing.T, r *report.Report, expected []string) { + if !slices.Equal(r.Namespaces, expected) { + t.Fatalf("expected namespaces %q, got %q", expected, r.Namespaces) } } @@ -2151,39 +2159,39 @@ func checkItems(t *testing.T, step *report.Step, expected []*report.Step) { func checkApplicationStatus( t *testing.T, - report *Report, + r *report.Report, expected *report.ApplicationStatus, ) { if expected != nil { - if !expected.Equal(report.ApplicationStatus) { - diff := helpers.UnifiedDiff(t, expected, report.ApplicationStatus) + if !expected.Equal(r.ApplicationStatus) { + diff := helpers.UnifiedDiff(t, expected, r.ApplicationStatus) t.Fatalf("application statuses not equal\n%s", diff) } - } else if report.ApplicationStatus != nil { + } else if r.ApplicationStatus != nil { t.Fatalf("application status not nil\n%s", - helpers.MarshalYAML(t, report.ApplicationStatus)) + helpers.MarshalYAML(t, r.ApplicationStatus)) } } func checkClusterStatus( t *testing.T, - report *Report, + r *report.Report, expected *report.ClustersStatus, ) { if expected != nil { - if !expected.Equal(report.ClustersStatus) { - diff := helpers.UnifiedDiff(t, expected, report.ClustersStatus) + if !expected.Equal(r.ClustersStatus) { + diff := helpers.UnifiedDiff(t, expected, r.ClustersStatus) t.Fatalf("clusters statuses not equal\n%s", diff) } - } else if report.ClustersStatus != nil { + } else if r.ClustersStatus != nil { t.Fatalf("clusters status not nil\n%s", - helpers.MarshalYAML(t, report.ClustersStatus)) + helpers.MarshalYAML(t, r.ClustersStatus)) } } -func checkSummary(t *testing.T, report *Report, expected Summary) { - if report.Summary != expected { - t.Fatalf("expected summary %q, got %q", expected, report.Summary) +func checkSummary(t *testing.T, r *report.Report, expected report.Summary) { + if !r.Summary.Equal(&expected) { + t.Fatalf("expected summary %v, got %v", expected, *r.Summary) } } diff --git a/pkg/validate/report.go b/pkg/validate/report.go index 7444293e..74623b10 100644 --- a/pkg/validate/report.go +++ b/pkg/validate/report.go @@ -9,52 +9,32 @@ import ( "github.com/ramendr/ramenctl/pkg/report" ) -type Summary struct { - Problem uint `json:"error"` - Stale uint `json:"stale"` - OK uint `json:"ok"` -} - -// Report created by validate sub commands. -type Report struct { - *report.Report - Summary Summary `json:"summary"` -} - -func (r *Report) Equal(o *Report) bool { - if r == o { - return true - } - if o == nil { - return false - } - if !r.Report.Equal(o.Report) { - return false - } - if r.Summary != o.Summary { - return false - } - return true -} +// Summary keys for validation reports. +const ( + OK = report.SummaryKey("ok") + Stale = report.SummaryKey("stale") + Problem = report.SummaryKey("problem") +) -// Add a validation to the summary. -func (s *Summary) Add(v report.Validation) { +// addValidation adds a validation to the summary. +func addValidation(s *report.Summary, v report.Validation) { switch v.GetState() { case report.OK: - s.OK++ + s.Add(OK) case report.Stale: - s.Stale++ + s.Add(Stale) case report.Problem: - s.Problem++ + s.Add(Problem) } } -// HasIssues returns true if there are any problems or stale results. -func (s *Summary) HasIssues() bool { - return s.Stale > 0 || s.Problem > 0 +// hasIssues returns true if there are any problems or stale results. +func hasIssues(s *report.Summary) bool { + return s.Get(Stale) > 0 || s.Get(Problem) > 0 } -// String returns a string representation. -func (s Summary) String() string { - return fmt.Sprintf("%d ok, %d stale, %d problem", s.OK, s.Stale, s.Problem) +// summaryString returns a string representation. +func summaryString(s *report.Summary) string { + return fmt.Sprintf("%d ok, %d stale, %d problem", + s.Get(OK), s.Get(Stale), s.Get(Problem)) } diff --git a/pkg/validate/report_test.go b/pkg/validate/report_test.go index 9611cbc2..93bb57e0 100644 --- a/pkg/validate/report_test.go +++ b/pkg/validate/report_test.go @@ -14,51 +14,63 @@ import ( ) func TestSummaryAdd(t *testing.T) { - s := Summary{} + s := &report.Summary{} - s.Add(&report.Validated{State: report.OK}) - s.Add(&report.Validated{State: report.OK}) - s.Add(&report.Validated{State: report.Stale}) - s.Add(&report.Validated{State: report.OK}) - s.Add(&report.Validated{State: report.Stale}) - s.Add(&report.Validated{State: report.Problem}) + addValidation(s, &report.Validated{State: report.OK}) + addValidation(s, &report.Validated{State: report.OK}) + addValidation(s, &report.Validated{State: report.Stale}) + addValidation(s, &report.Validated{State: report.OK}) + addValidation(s, &report.Validated{State: report.Stale}) + addValidation(s, &report.Validated{State: report.Problem}) - expected := Summary{OK: 3, Stale: 2, Problem: 1} - if s != expected { - t.Fatalf("expected %+v, got %+v", expected, s) + expected := report.Summary{ + OK: 3, + Stale: 2, + Problem: 1, + } + if !s.Equal(&expected) { + t.Fatalf("expected %+v, got %+v", expected, *s) } } func TestSummaryHasProblems(t *testing.T) { cases := []struct { name string - summary Summary + summary *report.Summary expected bool }{ - {"empty", Summary{}, false}, - {"ok", Summary{OK: 5}, false}, - {"only stale", Summary{Stale: 2}, true}, - {"only problem", Summary{Problem: 4}, true}, - {"problem and stale", Summary{Stale: 2, Problem: 3}, true}, + {"empty", &report.Summary{}, false}, + {"ok", &report.Summary{OK: 5}, false}, + {"only stale", &report.Summary{Stale: 2}, true}, + {"only problem", &report.Summary{Problem: 4}, true}, + { + "problem and stale", + &report.Summary{Stale: 2, Problem: 3}, + true, + }, } for _, tc := range cases { - if got := tc.summary.HasIssues(); got != tc.expected { + if got := hasIssues(tc.summary); got != tc.expected { t.Errorf("%s: expected %v, got %v", tc.name, tc.expected, got) } } } func TestSummaryString(t *testing.T) { - s := Summary{OK: 1, Stale: 0, Problem: 2} + s := &report.Summary{ + OK: 1, + Problem: 2, + } expected := "1 ok, 0 stale, 2 problem" - if s.String() != expected { - t.Fatalf("expected %q, got %q", expected, s.String()) + if summaryString(s) != expected { + t.Fatalf("expected %q, got %q", expected, summaryString(s)) } } func TestReportEqual(t *testing.T) { helpers.FakeTime(t) - r1 := &Report{Report: report.NewReport("name", &config.Config{})} + r1 := report.NewReport("name", &config.Config{}) + r1.Summary = &report.Summary{} t.Run("equal to self", func(t *testing.T) { r2 := r1 if !r1.Equal(r2) { @@ -67,7 +79,8 @@ func TestReportEqual(t *testing.T) { } }) t.Run("equal reports", func(t *testing.T) { - r2 := &Report{Report: report.NewReport("name", &config.Config{})} + r2 := report.NewReport("name", &config.Config{}) + r2.Summary = &report.Summary{} if !r1.Equal(r2) { diff := helpers.UnifiedDiff(t, r1, r2) t.Fatalf("reports not equal\n%s", diff) @@ -77,39 +90,34 @@ func TestReportEqual(t *testing.T) { func TestReportNotEqual(t *testing.T) { helpers.FakeTime(t) - r1 := &Report{Report: report.NewReport("name", &config.Config{})} + r1 := report.NewReport("name", &config.Config{}) + r1.Summary = &report.Summary{} t.Run("nil", func(t *testing.T) { if r1.Equal(nil) { t.Fatal("report should not be equal to nil") } }) t.Run("report", func(t *testing.T) { - r2 := &Report{Report: report.NewReport("other", &config.Config{})} + r2 := report.NewReport("other", &config.Config{}) + r2.Summary = &report.Summary{} if r1.Equal(r2) { t.Fatal("reports with different report should not be equal") } }) - t.Run("summary", func(t *testing.T) { - r1 := &Report{Report: report.NewReport("name", &config.Config{})} - r1.Summary = Summary{OK: 5} - r2 := &Report{Report: report.NewReport("name", &config.Config{})} - r2.Summary = Summary{OK: 3} - if r1.Equal(r2) { - t.Fatal("reports with different summary should not be equal") - } - }) } func TestReportRoundtrip(t *testing.T) { - r1 := &Report{ - Report: report.NewReport("name", &config.Config{}), - Summary: Summary{OK: 3, Stale: 2, Problem: 1}, + r1 := report.NewReport("name", &config.Config{}) + r1.Summary = &report.Summary{ + OK: 3, + Stale: 2, + Problem: 1, } b, err := yaml.Marshal(r1) if err != nil { t.Fatalf("failed to marshal yaml: %s", err) } - r2 := &Report{} + r2 := &report.Report{} if err := yaml.Unmarshal(b, r2); err != nil { t.Fatalf("failed to unmarshal yaml: %s", err) }