From b80ccba29d77b6eb560deb2ea042c5c750afd14c Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Wed, 14 Jan 2026 21:23:30 +0200
Subject: [PATCH 1/8] Add .cursor/rules.md for AI assistant guidelines

AI coding assistants tend to make changes without discussing design,
add unnecessary code, and create commit messages that repeat the diff.

This file establishes conventions to ensure AI assistants collaborate
effectively with human reviewers, keeping changes minimal and focused.

Assisted-by: Cursor/Claude Opus 4.5
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 .cursor/rules.md | 52 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 .cursor/rules.md

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

From 54491d91fe223100573982dfc500cc7f767188c1 Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Wed, 14 Jan 2026 19:40:27 +0200
Subject: [PATCH 2/8] report: Refactor YAML report writing

Restructure report writing architecture to prepare for HTML report
generation while keeping YAML serialization generic.

Key changes:
- Add report.WriteYAML(io.Writer, any) standalone function
- Add command.OpenReport(format) for file management
- Add command.WriteYAMLReport(any) centralized method
- Update gather, validate, test commands to use centralized method

Architecture rationale:
YAML serialization is generic - yaml.Marshal() works with any struct
via reflection, so WriteYAML belongs in the report package and accepts
any report type.

HTML generation is specific - templates need knowledge of specific
report fields (ApplicationStatus, Summary, etc.), so each command
package will implement its own WriteHTML() for its report type.

This asymmetry reflects the fundamental difference between:
- Serialization (generic, reflection-based)
- Presentation (domain-specific, template-based)

The command package now provides:
- OpenReport(format) - file management
- WriteYAMLReport(any) - generic YAML writing

Each command package will later add:
- writeHTMLReport() - package-specific HTML generation

Assisted-by: Cursor/Claude Opus 4.5
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 pkg/command/command.go  | 33 ++++++++++++++++++++++++++-------
 pkg/gather/command.go   |  8 ++------
 pkg/report/yaml.go      | 21 +++++++++++++++++++++
 pkg/test/command.go     |  8 ++------
 pkg/validate/command.go |  8 ++------
 5 files changed, 53 insertions(+), 25 deletions(-)
 create mode 100644 pkg/report/yaml.go

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/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..ba00db7d 100644
--- a/pkg/test/command.go
+++ b/pkg/test/command.go
@@ -274,16 +274,12 @@ 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)
-	}
+	c.command.WriteYAMLReport(c.report)
 	return fmt.Errorf("%s (%s)", c.report.Status, c.report.Summary)
 }
 
 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("%s (%s)", c.report.Status, c.report.Summary)
 }
 
diff --git a/pkg/validate/command.go b/pkg/validate/command.go
index 8bb99e78..8bba1e3b 100644
--- a/pkg/validate/command.go
+++ b/pkg/validate/command.go
@@ -182,16 +182,12 @@ 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)
-	}
+	c.command.WriteYAMLReport(c.report)
 	return fmt.Errorf("validation %s (%s)", c.report.Status, c.report.Summary)
 }
 
 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("Validation completed (%s)", c.report.Summary)
 }
 

From 07fd619da66983ffd3e9f71171c2ac8cd7ab94a8 Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Wed, 14 Jan 2026 20:37:47 +0200
Subject: [PATCH 3/8] report: Add generic Summary type

Add a unified Summary type that can be used by all commands (test,
validate, gather) instead of having separate summary types per package.

Key changes:
- Add SummaryKey type for type-safe map keys
- Add Summary type as map[SummaryKey]int
- Add TestXxx constants for test summaries (passed, failed, etc.)
- Add ValidationXxx constants for validation summaries (ok, problem, etc.)
- Add Add() and Get() methods for Summary
- Add YAML roundtrip tests for both test and validation summaries

The naming convention (TestPassed vs ValidationOK) makes the purpose
of each key clear and prevents mixing keys from different domains.

This enables a single Report type that all commands can use, which
is required for generic HTML report generation.

Assisted-by: Cursor/Claude Opus 4.5
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 pkg/report/summary.go      | 36 +++++++++++++++++++++++
 pkg/report/summary_test.go | 58 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100644 pkg/report/summary.go
 create mode 100644 pkg/report/summary_test.go

diff --git a/pkg/report/summary.go b/pkg/report/summary.go
new file mode 100644
index 00000000..828d6945
--- /dev/null
+++ b/pkg/report/summary.go
@@ -0,0 +1,36 @@
+// SPDX-FileCopyrightText: The RamenDR authors
+// SPDX-License-Identifier: Apache-2.0
+
+package report
+
+// SummaryKey is a typed key for Summary counters.
+type SummaryKey string
+
+// Summary key constants for tests.
+const (
+	TestPassed   = SummaryKey("passed")
+	TestFailed   = SummaryKey("failed")
+	TestSkipped  = SummaryKey("skipped")
+	TestCanceled = SummaryKey("canceled")
+)
+
+// Summary key constants for validations.
+const (
+	ValidationOK      = SummaryKey("ok")
+	ValidationStale   = SummaryKey("stale")
+	ValidationProblem = SummaryKey("problem")
+)
+
+// Summary is a counter for report summaries.
+// Use TestXxx keys for test reports, ValidationXxx keys for validation reports.
+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]
+}
diff --git a/pkg/report/summary_test.go b/pkg/report/summary_test.go
new file mode 100644
index 00000000..7fa7ad45
--- /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.TestPassed:   5,
+		report.TestFailed:   2,
+		report.TestSkipped:  1,
+		report.TestCanceled: 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.ValidationOK:      10,
+		report.ValidationStale:   1,
+		report.ValidationProblem: 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)
+	}
+}

From 97ec77c78a1c80691db737ac9e68a003728cdaaa Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Wed, 14 Jan 2026 20:48:47 +0200
Subject: [PATCH 4/8] test: Use report.Summary instead of local Summary type

Replace the test package's local Summary struct with the unified
report.Summary type from pkg/report/summary.go.

Changes:
- Remove local Summary struct from report.go
- Use report.Summary with TestPassed, TestFailed, etc. constants
- Convert AddTest method to addTest package function
- Convert String method to summaryString package function
- Use maps.Equal for Summary comparison in Equal and tests
- Update all tests to use new report.Summary syntax

This is part of the effort to unify report types across all commands
to enable generic HTML report generation.

Assisted-by: Cursor/Claude Opus 4.5
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 pkg/test/command.go      |  4 +-
 pkg/test/command_test.go | 87 +++++++++++++++++++++++++++++-----------
 pkg/test/report.go       | 35 +++++++---------
 pkg/test/report_test.go  | 63 +++++++++++++++++++----------
 4 files changed, 123 insertions(+), 66 deletions(-)

diff --git a/pkg/test/command.go b/pkg/test/command.go
index ba00db7d..023beadd 100644
--- a/pkg/test/command.go
+++ b/pkg/test/command.go
@@ -275,12 +275,12 @@ func (c *Command) gatherS3Data() {
 
 func (c *Command) failed() error {
 	c.command.WriteYAMLReport(c.report)
-	return fmt.Errorf("%s (%s)", c.report.Status, c.report.Summary)
+	return fmt.Errorf("%s (%s)", c.report.Status, summaryString(c.report.Summary))
 }
 
 func (c *Command) passed() {
 	c.command.WriteYAMLReport(c.report)
-	console.Completed("%s (%s)", c.report.Status, c.report.Summary)
+	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..4173b628 100644
--- a/pkg/test/command_test.go
+++ b/pkg/test/command_test.go
@@ -7,6 +7,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"maps"
 	"testing"
 
 	e2econfig "github.com/ramendr/ramen/e2e/config"
@@ -130,7 +131,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{report.TestPassed: len(testConfig.Tests)},
+	)
 	if len(test.report.Steps) != 3 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
 	}
@@ -153,7 +159,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 +174,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 +189,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 +206,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 +223,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{report.TestFailed: len(testConfig.Tests)},
+	)
 	if len(test.report.Steps) != 3 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
 	}
@@ -240,7 +251,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{report.TestPassed: 4, report.TestFailed: 2},
+	)
 	if len(test.report.Steps) != 3 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
 	}
@@ -267,7 +283,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{report.TestCanceled: len(testConfig.Tests)},
+	)
 	if len(test.report.Steps) != 3 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
 	}
@@ -290,7 +311,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{report.TestPassed: 6})
 	if len(test.report.Steps) != 3 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
 	}
@@ -313,7 +334,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 +349,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 +364,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{report.TestFailed: len(testConfig.Tests)},
+	)
 	if len(test.report.Steps) != 2 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
 	}
@@ -364,7 +390,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{report.TestCanceled: len(testConfig.Tests)},
+	)
 	if len(test.report.Steps) != 2 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
 	}
@@ -385,7 +416,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{report.TestPassed: len(testConfig.Tests)},
+	)
 	if len(test.report.Steps) != 3 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
 	}
@@ -408,7 +444,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{report.TestPassed: len(testConfig.Tests)},
+	)
 	if len(test.report.Steps) != 3 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
 	}
@@ -437,16 +478,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 !maps.Equal(r.Summary, 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..e58f9148 100644
--- a/pkg/test/report.go
+++ b/pkg/test/report.go
@@ -5,6 +5,7 @@ package test
 
 import (
 	"fmt"
+	"maps"
 
 	e2econfig "github.com/ramendr/ramen/e2e/config"
 
@@ -18,19 +19,11 @@ 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"`
-}
-
 // Report created by test sub commands.
 type Report struct {
 	*report.Base
 	Config  *e2econfig.Config `json:"config"`
-	Summary Summary           `json:"summary"`
+	Summary report.Summary    `json:"summary"`
 }
 
 func newReport(commandName string, config *e2econfig.Config) *Report {
@@ -38,8 +31,9 @@ func newReport(commandName string, config *e2econfig.Config) *Report {
 		panic("config must not be nil")
 	}
 	return &Report{
-		Base:   report.NewBase(commandName),
-		Config: config,
+		Base:    report.NewBase(commandName),
+		Config:  config,
+		Summary: report.Summary{},
 	}
 }
 
@@ -50,7 +44,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 +67,27 @@ func (r *Report) Equal(o *Report) bool {
 	} else if r.Config != o.Config {
 		return false
 	}
-	if r.Summary != o.Summary {
+	if !maps.Equal(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(report.TestPassed)
 	case report.Failed:
-		s.Failed++
+		s.Add(report.TestFailed)
 	case report.Skipped:
-		s.Skipped++
+		s.Add(report.TestSkipped)
 	case report.Canceled:
-		s.Canceled++
+		s.Add(report.TestCanceled)
 	}
 }
 
-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(report.TestPassed), s.Get(report.TestFailed),
+		s.Get(report.TestSkipped), s.Get(report.TestCanceled))
 }
diff --git a/pkg/test/report_test.go b/pkg/test/report_test.go
index 30dcbe11..e54c80eb 100644
--- a/pkg/test/report_test.go
+++ b/pkg/test/report_test.go
@@ -4,6 +4,7 @@
 package test
 
 import (
+	"maps"
 	"testing"
 
 	e2econfig "github.com/ramendr/ramen/e2e/config"
@@ -59,8 +60,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{
+		report.TestPassed:   1,
+		report.TestFailed:   1,
+		report.TestSkipped:  1,
+		report.TestCanceled: 1,
+	}
+	if !maps.Equal(r.Summary, expectedSummary) {
 		t.Errorf("expected summary %+v, got %+v", expectedSummary, r.Summary)
 	}
 }
@@ -70,7 +76,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{report.TestPassed: 2}
 		return r
 	}
 
@@ -118,7 +124,7 @@ func TestReportEqual(t *testing.T) {
 
 	t.Run("different summary", func(t *testing.T) {
 		r2 := createReport()
-		r2.Summary = Summary{Passed: 1, Failed: 1}
+		r2.Summary = report.Summary{report.TestPassed: 1, report.TestFailed: 1}
 		if r1.Equal(r2) {
 			t.Error("reports with different summary should not be equal")
 		}
@@ -146,52 +152,67 @@ func TestReportMarshaling(t *testing.T) {
 			Duration: 1.0,
 		},
 	}
-	r.Summary = Summary{Passed: 2, Failed: 1}
+	r.Summary = report.Summary{report.TestPassed: 2, report.TestFailed: 1}
 
 	// Test roundtrip marshaling/unmarshaling
 	checkRoundtrip(t, r)
 }
 
 func TestSummaryString(t *testing.T) {
-	summary := Summary{Passed: 5, Failed: 2, Skipped: 3, Canceled: 1}
+	summary := report.Summary{
+		report.TestPassed:   5,
+		report.TestFailed:   2,
+		report.TestSkipped:  3,
+		report.TestCanceled: 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())
+	if summaryString(summary) != expectedString {
+		t.Errorf("expected summary string %s, got %s", expectedString, summaryString(summary))
 	}
 }
 
 func TestSummaryMarshal(t *testing.T) {
-	summary := Summary{Passed: 4, Failed: 3, Skipped: 2, Canceled: 1}
+	summary := report.Summary{
+		report.TestPassed:   4,
+		report.TestFailed:   3,
+		report.TestSkipped:  2,
+		report.TestCanceled: 1,
+	}
 
 	bytes, err := yaml.Marshal(summary)
 	if err != nil {
 		t.Fatalf("failed to marshal summary: %v", err)
 	}
 
-	var unmarshaledSummary Summary
+	var unmarshaledSummary report.Summary
 	err = yaml.Unmarshal(bytes, &unmarshaledSummary)
 	if err != nil {
 		t.Fatalf("failed to unmarshal summary: %v", err)
 	}
-	if unmarshaledSummary != summary {
+	if !maps.Equal(unmarshaledSummary, summary) {
 		t.Errorf("unmarshaled summary %+v does not match original summary %+v",
 			unmarshaledSummary, 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{
+		report.TestPassed:   3,
+		report.TestFailed:   1,
+		report.TestSkipped:  1,
+		report.TestCanceled: 1,
+	}
+	if !maps.Equal(summary, expectedSummary) {
 		t.Errorf("expected summary %+v, got %+v", expectedSummary, summary)
 	}
 }

From 41738f3f000dea5fd87c42f1cc2656b1189ec51e Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Wed, 14 Jan 2026 21:46:11 +0200
Subject: [PATCH 5/8] validate: Use report.Summary

Replace the validate package's local Summary struct with the unified
report.Summary type from pkg/report/summary.go.

The unified Summary is a simple map[SummaryKey]int that works for all
commands, but cannot provide validation-specific methods. Added package
helper functions to replace the removed methods:

- addValidation(s, v): adds validation result to summary based on state
- hasIssues(s): returns true if summary has problems or stale results
- summaryString(s): formats summary as "N ok, N stale, N problem"

This is part of the effort to unify report types across all commands
to enable generic HTML report generation.

Assisted-by: Cursor/Claude Opus 4.5
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 pkg/validate/application.go  |  22 +++----
 pkg/validate/clusters.go     |  26 ++++----
 pkg/validate/command.go      |  10 ++--
 pkg/validate/command_test.go | 113 +++++++++++++++++++----------------
 pkg/validate/report.go       |  34 +++++------
 pkg/validate/report_test.go  |  81 ++++++++++++++++---------
 6 files changed, 157 insertions(+), 129 deletions(-)

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 8bba1e3b..b2c7dc47 100644
--- a/pkg/validate/command.go
+++ b/pkg/validate/command.go
@@ -60,7 +60,7 @@ func newCommand(cmd *command.Command, cfg *config.Config, backend validation.Val
 		config:  cfg,
 		backend: backend,
 		context: cmd.Context(),
-		report:  &Report{Report: report.NewReport(cmd.Name(), cfg)},
+		report:  &Report{Report: report.NewReport(cmd.Name(), cfg), Summary: report.Summary{}},
 	}
 }
 
@@ -123,7 +123,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 +135,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
@@ -183,12 +183,12 @@ func (c *Command) dataDir() string {
 
 func (c *Command) failed() error {
 	c.command.WriteYAMLReport(c.report)
-	return fmt.Errorf("validation %s (%s)", c.report.Status, c.report.Summary)
+	return fmt.Errorf("validation %s (%s)", c.report.Status, summaryString(c.report.Summary))
 }
 
 func (c *Command) passed() {
 	c.command.WriteYAMLReport(c.report)
-	console.Completed("Validation completed (%s)", c.report.Summary)
+	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..0e85883e 100644
--- a/pkg/validate/command_test.go
+++ b/pkg/validate/command_test.go
@@ -7,6 +7,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"maps"
 	"os"
 	"reflect"
 	"slices"
@@ -265,9 +266,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{report.ValidationOK: 1, report.ValidationProblem: 2}
+		if !maps.Equal(cmd.report.Summary, expected) {
+			t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
 		}
 	})
 }
@@ -315,9 +316,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{report.ValidationOK: 3, report.ValidationProblem: 1}
+		if !maps.Equal(cmd.report.Summary, expected) {
+			t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
 		}
 	})
 }
@@ -396,13 +397,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{report.ValidationProblem: errors}
+	if !maps.Equal(cmd.report.Summary, expected) {
+		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
 	}
 }
 
@@ -442,9 +443,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{report.ValidationOK: len(cases)}
+	if !maps.Equal(cmd.report.Summary, expected) {
+		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
 	}
 }
 
@@ -470,9 +471,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{report.ValidationOK: 1}
+	if !maps.Equal(cmd.report.Summary, expected) {
+		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
 	}
 }
 
@@ -527,9 +528,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{report.ValidationProblem: len(progressions)}
+	if !maps.Equal(cmd.report.Summary, expected) {
+		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
 	}
 }
 
@@ -564,9 +565,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{report.ValidationOK: len(cases)}
+	if !maps.Equal(cmd.report.Summary, expected) {
+		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
 	}
 }
 
@@ -607,9 +608,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{report.ValidationProblem: len(cases)}
+	if !maps.Equal(cmd.report.Summary, expected) {
+		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
 	}
 }
 
@@ -634,9 +635,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{report.ValidationOK: 1}
+	if !maps.Equal(cmd.report.Summary, expected) {
+		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
 	}
 }
 
@@ -674,9 +675,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{report.ValidationProblem: len(cases)}
+	if !maps.Equal(cmd.report.Summary, expected) {
+		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
 	}
 }
 
@@ -1100,7 +1101,7 @@ func TestValidateClustersK8s(t *testing.T) {
 	}
 	checkClusterStatus(t, validate.report, expected)
 
-	checkSummary(t, validate.report, Summary{OK: 42})
+	checkSummary(t, validate.report, report.Summary{report.ValidationOK: 42})
 }
 
 func TestValidateClustersOcp(t *testing.T) {
@@ -1499,7 +1500,7 @@ func TestValidateClustersOcp(t *testing.T) {
 	}
 	checkClusterStatus(t, validate.report, expected)
 
-	checkSummary(t, validate.report, Summary{OK: 40})
+	checkSummary(t, validate.report, report.Summary{report.ValidationOK: 40})
 }
 
 func TestValidateClustersValidateFailed(t *testing.T) {
@@ -1517,7 +1518,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 +1536,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 +1563,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 +1594,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{report.ValidationProblem: 9})
 }
 
 func TestValidateClustersCheckS3Failed(t *testing.T) {
@@ -1627,7 +1628,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{report.ValidationOK: 41, report.ValidationProblem: 1},
+	)
 }
 
 func TestValidateClustersCheckS3Canceled(t *testing.T) {
@@ -1657,7 +1662,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 +1884,7 @@ func TestValidateApplicationPassed(t *testing.T) {
 	}
 	checkApplicationStatus(t, validate.report, expectedStatus)
 
-	checkSummary(t, validate.report, Summary{OK: 24})
+	checkSummary(t, validate.report, report.Summary{report.ValidationOK: 24})
 }
 
 func TestValidateApplicationValidateFailed(t *testing.T) {
@@ -1896,7 +1901,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 +1918,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 +1942,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 +1966,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 +1993,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 +2023,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 +2055,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{report.ValidationOK: 23, report.ValidationProblem: 1},
+	)
 }
 
 func TestValidateApplicationGatherS3Canceled(t *testing.T) {
@@ -2080,7 +2089,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:
@@ -2181,9 +2190,9 @@ func checkClusterStatus(
 	}
 }
 
-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, expected report.Summary) {
+	if !maps.Equal(r.Summary, 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..702b7aa2 100644
--- a/pkg/validate/report.go
+++ b/pkg/validate/report.go
@@ -5,20 +5,15 @@ package validate
 
 import (
 	"fmt"
+	"maps"
 
 	"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"`
+	Summary report.Summary `json:"summary"`
 }
 
 func (r *Report) Equal(o *Report) bool {
@@ -31,30 +26,31 @@ func (r *Report) Equal(o *Report) bool {
 	if !r.Report.Equal(o.Report) {
 		return false
 	}
-	if r.Summary != o.Summary {
+	if !maps.Equal(r.Summary, o.Summary) {
 		return false
 	}
 	return true
 }
 
-// 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(report.ValidationOK)
 	case report.Stale:
-		s.Stale++
+		s.Add(report.ValidationStale)
 	case report.Problem:
-		s.Problem++
+		s.Add(report.ValidationProblem)
 	}
 }
 
-// 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(report.ValidationStale) > 0 || s.Get(report.ValidationProblem) > 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(report.ValidationOK), s.Get(report.ValidationStale), s.Get(report.ValidationProblem))
 }
diff --git a/pkg/validate/report_test.go b/pkg/validate/report_test.go
index 9611cbc2..88631f74 100644
--- a/pkg/validate/report_test.go
+++ b/pkg/validate/report_test.go
@@ -4,6 +4,7 @@
 package validate
 
 import (
+	"maps"
 	"testing"
 
 	"sigs.k8s.io/yaml"
@@ -14,17 +15,21 @@ 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 {
+	expected := report.Summary{
+		report.ValidationOK:      3,
+		report.ValidationStale:   2,
+		report.ValidationProblem: 1,
+	}
+	if !maps.Equal(s, expected) {
 		t.Fatalf("expected %+v, got %+v", expected, s)
 	}
 }
@@ -32,33 +37,40 @@ func TestSummaryAdd(t *testing.T) {
 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{report.ValidationOK: 5}, false},
+		{"only stale", report.Summary{report.ValidationStale: 2}, true},
+		{"only problem", report.Summary{report.ValidationProblem: 4}, true},
+		{
+			"problem and stale",
+			report.Summary{report.ValidationStale: 2, report.ValidationProblem: 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{
+		report.ValidationOK:      1,
+		report.ValidationProblem: 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{Report: report.NewReport("name", &config.Config{}), Summary: report.Summary{}}
 	t.Run("equal to self", func(t *testing.T) {
 		r2 := r1
 		if !r1.Equal(r2) {
@@ -67,7 +79,7 @@ func TestReportEqual(t *testing.T) {
 		}
 	})
 	t.Run("equal reports", func(t *testing.T) {
-		r2 := &Report{Report: report.NewReport("name", &config.Config{})}
+		r2 := &Report{Report: report.NewReport("name", &config.Config{}), Summary: report.Summary{}}
 		if !r1.Equal(r2) {
 			diff := helpers.UnifiedDiff(t, r1, r2)
 			t.Fatalf("reports not equal\n%s", diff)
@@ -77,23 +89,30 @@ func TestReportEqual(t *testing.T) {
 
 func TestReportNotEqual(t *testing.T) {
 	helpers.FakeTime(t)
-	r1 := &Report{Report: report.NewReport("name", &config.Config{})}
+	r1 := &Report{Report: report.NewReport("name", &config.Config{}), 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{
+			Report:  report.NewReport("other", &config.Config{}),
+			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}
+		r1 := &Report{
+			Report:  report.NewReport("name", &config.Config{}),
+			Summary: report.Summary{report.ValidationOK: 5},
+		}
+		r2 := &Report{
+			Report:  report.NewReport("name", &config.Config{}),
+			Summary: report.Summary{report.ValidationOK: 3},
+		}
 		if r1.Equal(r2) {
 			t.Fatal("reports with different summary should not be equal")
 		}
@@ -102,8 +121,12 @@ func TestReportNotEqual(t *testing.T) {
 
 func TestReportRoundtrip(t *testing.T) {
 	r1 := &Report{
-		Report:  report.NewReport("name", &config.Config{}),
-		Summary: Summary{OK: 3, Stale: 2, Problem: 1},
+		Report: report.NewReport("name", &config.Config{}),
+		Summary: report.Summary{
+			report.ValidationOK:      3,
+			report.ValidationStale:   2,
+			report.ValidationProblem: 1,
+		},
 	}
 	b, err := yaml.Marshal(r1)
 	if err != nil {

From c96f9d1e43338e373f7072fcb885aa365953a978 Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Wed, 14 Jan 2026 22:14:09 +0200
Subject: [PATCH 6/8] report: Use unified report.Summary

The validate package now uses report.Report directly. Both validate and
test packages share Summary through report.Base.

Changes:
- Move Summary *Summary to report.Base (shared by all reports)
- Add Summary.Equal() method for nil-safe pointer comparison
- Remove validate.Report struct, keep only helper functions
- Update validate package to use *report.Report directly
- Remove Summary field from test.Report (uses Base.Summary)

This simplifies the codebase and moves us closer to having a unified
report type that can be used for generic HTML generation.

Assisted-by: Cursor/Claude Opus 4.5
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 pkg/report/report.go         | 13 +++++-
 pkg/report/summary.go        | 14 +++++++
 pkg/test/command_test.go     |  5 +--
 pkg/test/report.go           | 18 ++++----
 pkg/test/report_test.go      | 25 ++++++------
 pkg/validate/command.go      |  6 ++-
 pkg/validate/command_test.go | 79 ++++++++++++++++++------------------
 pkg/validate/report.go       | 29 ++-----------
 pkg/validate/report_test.go  | 62 +++++++++++++---------------
 9 files changed, 120 insertions(+), 131 deletions(-)

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/summary.go b/pkg/report/summary.go
index 828d6945..e9dde1e1 100644
--- a/pkg/report/summary.go
+++ b/pkg/report/summary.go
@@ -3,6 +3,8 @@
 
 package report
 
+import "maps"
+
 // SummaryKey is a typed key for Summary counters.
 type SummaryKey string
 
@@ -34,3 +36,15 @@ func (s Summary) Add(key SummaryKey) {
 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/test/command_test.go b/pkg/test/command_test.go
index 4173b628..17a31af0 100644
--- a/pkg/test/command_test.go
+++ b/pkg/test/command_test.go
@@ -7,7 +7,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"maps"
 	"testing"
 
 	e2econfig "github.com/ramendr/ramen/e2e/config"
@@ -482,8 +481,8 @@ func checkReport(t *testing.T, r *Report, status report.Status, summary report.S
 	if r.Status != status {
 		t.Errorf("expected status %q, got %q", status, r.Status)
 	}
-	if !maps.Equal(r.Summary, summary) {
-		t.Errorf("expected summary %+v, got %+v", summary, r.Summary)
+	if !r.Summary.Equal(&summary) {
+		t.Errorf("expected summary %+v, got %+v", summary, *r.Summary)
 	}
 	duration := totalDuration(r.Steps)
 	if r.Duration != duration {
diff --git a/pkg/test/report.go b/pkg/test/report.go
index e58f9148..30edb3d3 100644
--- a/pkg/test/report.go
+++ b/pkg/test/report.go
@@ -5,7 +5,6 @@ package test
 
 import (
 	"fmt"
-	"maps"
 
 	e2econfig "github.com/ramendr/ramen/e2e/config"
 
@@ -22,18 +21,18 @@ const (
 // Report created by test sub commands.
 type Report struct {
 	*report.Base
-	Config  *e2econfig.Config `json:"config"`
-	Summary report.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),
-		Config:  config,
-		Summary: report.Summary{},
+		Base:   base,
+		Config: config,
 	}
 }
 
@@ -67,13 +66,10 @@ func (r *Report) Equal(o *Report) bool {
 	} else if r.Config != o.Config {
 		return false
 	}
-	if !maps.Equal(r.Summary, o.Summary) {
-		return false
-	}
 	return true
 }
 
-func addTest(s report.Summary, t *report.Step) {
+func addTest(s *report.Summary, t *report.Step) {
 	switch t.Status {
 	case report.Passed:
 		s.Add(report.TestPassed)
@@ -86,7 +82,7 @@ func addTest(s report.Summary, t *report.Step) {
 	}
 }
 
-func summaryString(s report.Summary) string {
+func summaryString(s *report.Summary) string {
 	return fmt.Sprintf("%d passed, %d failed, %d skipped, %d canceled",
 		s.Get(report.TestPassed), s.Get(report.TestFailed),
 		s.Get(report.TestSkipped), s.Get(report.TestCanceled))
diff --git a/pkg/test/report_test.go b/pkg/test/report_test.go
index e54c80eb..c17cd02c 100644
--- a/pkg/test/report_test.go
+++ b/pkg/test/report_test.go
@@ -4,7 +4,6 @@
 package test
 
 import (
-	"maps"
 	"testing"
 
 	e2econfig "github.com/ramendr/ramen/e2e/config"
@@ -60,13 +59,13 @@ func TestReportSummary(t *testing.T) {
 	}
 	r.AddStep(testsStep)
 
-	expectedSummary := report.Summary{
+	expectedSummary := &report.Summary{
 		report.TestPassed:   1,
 		report.TestFailed:   1,
 		report.TestSkipped:  1,
 		report.TestCanceled: 1,
 	}
-	if !maps.Equal(r.Summary, expectedSummary) {
+	if !r.Summary.Equal(expectedSummary) {
 		t.Errorf("expected summary %+v, got %+v", expectedSummary, r.Summary)
 	}
 }
@@ -76,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 = report.Summary{report.TestPassed: 2}
+		r.Summary = &report.Summary{report.TestPassed: 2}
 		return r
 	}
 
@@ -124,7 +123,7 @@ func TestReportEqual(t *testing.T) {
 
 	t.Run("different summary", func(t *testing.T) {
 		r2 := createReport()
-		r2.Summary = report.Summary{report.TestPassed: 1, report.TestFailed: 1}
+		r2.Summary = &report.Summary{report.TestPassed: 1, report.TestFailed: 1}
 		if r1.Equal(r2) {
 			t.Error("reports with different summary should not be equal")
 		}
@@ -152,14 +151,14 @@ func TestReportMarshaling(t *testing.T) {
 			Duration: 1.0,
 		},
 	}
-	r.Summary = report.Summary{report.TestPassed: 2, report.TestFailed: 1}
+	r.Summary = &report.Summary{report.TestPassed: 2, report.TestFailed: 1}
 
 	// Test roundtrip marshaling/unmarshaling
 	checkRoundtrip(t, r)
 }
 
 func TestSummaryString(t *testing.T) {
-	summary := report.Summary{
+	summary := &report.Summary{
 		report.TestPassed:   5,
 		report.TestFailed:   2,
 		report.TestSkipped:  3,
@@ -172,7 +171,7 @@ func TestSummaryString(t *testing.T) {
 }
 
 func TestSummaryMarshal(t *testing.T) {
-	summary := report.Summary{
+	summary := &report.Summary{
 		report.TestPassed:   4,
 		report.TestFailed:   3,
 		report.TestSkipped:  2,
@@ -189,14 +188,14 @@ func TestSummaryMarshal(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed to unmarshal summary: %v", err)
 	}
-	if !maps.Equal(unmarshaledSummary, summary) {
+	if !summary.Equal(&unmarshaledSummary) {
 		t.Errorf("unmarshaled summary %+v does not match original summary %+v",
-			unmarshaledSummary, summary)
+			unmarshaledSummary, *summary)
 	}
 }
 
 func TestSummaryCount(t *testing.T) {
-	summary := report.Summary{}
+	summary := &report.Summary{}
 
 	// Add multiple tests of different status
 	addTest(summary, &report.Step{Status: report.Passed})
@@ -206,13 +205,13 @@ func TestSummaryCount(t *testing.T) {
 	addTest(summary, &report.Step{Status: report.Canceled})
 	addTest(summary, &report.Step{Status: report.Passed})
 
-	expectedSummary := report.Summary{
+	expectedSummary := &report.Summary{
 		report.TestPassed:   3,
 		report.TestFailed:   1,
 		report.TestSkipped:  1,
 		report.TestCanceled: 1,
 	}
-	if !maps.Equal(summary, expectedSummary) {
+	if !summary.Equal(expectedSummary) {
 		t.Errorf("expected summary %+v, got %+v", expectedSummary, summary)
 	}
 }
diff --git a/pkg/validate/command.go b/pkg/validate/command.go
index b2c7dc47..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), Summary: report.Summary{}},
+		report:  r,
 	}
 }
 
diff --git a/pkg/validate/command_test.go b/pkg/validate/command_test.go
index 0e85883e..77a2bcba 100644
--- a/pkg/validate/command_test.go
+++ b/pkg/validate/command_test.go
@@ -7,7 +7,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"maps"
 	"os"
 	"reflect"
 	"slices"
@@ -267,8 +266,8 @@ func TestValidatedDeleted(t *testing.T) {
 
 	t.Run("update summary", func(t *testing.T) {
 		expected := report.Summary{report.ValidationOK: 1, report.ValidationProblem: 2}
-		if !maps.Equal(cmd.report.Summary, expected) {
-			t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
+		if !cmd.report.Summary.Equal(&expected) {
+			t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 		}
 	})
 }
@@ -317,8 +316,8 @@ func TestValidatedDRPCAction(t *testing.T) {
 
 	t.Run("update summary", func(t *testing.T) {
 		expected := report.Summary{report.ValidationOK: 3, report.ValidationProblem: 1}
-		if !maps.Equal(cmd.report.Summary, expected) {
-			t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
+		if !cmd.report.Summary.Equal(&expected) {
+			t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 		}
 	})
 }
@@ -402,8 +401,8 @@ func TestValidatedDRPCPhaseError(t *testing.T) {
 		errors += len(group.cases)
 	}
 	expected := report.Summary{report.ValidationProblem: errors}
-	if !maps.Equal(cmd.report.Summary, expected) {
-		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
+	if !cmd.report.Summary.Equal(&expected) {
+		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
 }
 
@@ -444,8 +443,8 @@ func TestValidatedDRPCPhaseOK(t *testing.T) {
 	}
 
 	expected := report.Summary{report.ValidationOK: len(cases)}
-	if !maps.Equal(cmd.report.Summary, expected) {
-		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
+	if !cmd.report.Summary.Equal(&expected) {
+		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
 }
 
@@ -472,8 +471,8 @@ func TestValidatedDRPCProgressionOK(t *testing.T) {
 	})
 
 	expected := report.Summary{report.ValidationOK: 1}
-	if !maps.Equal(cmd.report.Summary, expected) {
-		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
+	if !cmd.report.Summary.Equal(&expected) {
+		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
 }
 
@@ -529,8 +528,8 @@ func TestValidatedDRPCProgressionError(t *testing.T) {
 	}
 
 	expected := report.Summary{report.ValidationProblem: len(progressions)}
-	if !maps.Equal(cmd.report.Summary, expected) {
-		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
+	if !cmd.report.Summary.Equal(&expected) {
+		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
 }
 
@@ -566,8 +565,8 @@ func TestValidatedVRGSTateOK(t *testing.T) {
 	}
 
 	expected := report.Summary{report.ValidationOK: len(cases)}
-	if !maps.Equal(cmd.report.Summary, expected) {
-		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
+	if !cmd.report.Summary.Equal(&expected) {
+		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
 }
 
@@ -609,8 +608,8 @@ func TestValidatedVRGSTateError(t *testing.T) {
 	}
 
 	expected := report.Summary{report.ValidationProblem: len(cases)}
-	if !maps.Equal(cmd.report.Summary, expected) {
-		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
+	if !cmd.report.Summary.Equal(&expected) {
+		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
 }
 
@@ -636,8 +635,8 @@ func TestValidatedProtectedPVCOK(t *testing.T) {
 	})
 
 	expected := report.Summary{report.ValidationOK: 1}
-	if !maps.Equal(cmd.report.Summary, expected) {
-		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
+	if !cmd.report.Summary.Equal(&expected) {
+		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
 }
 
@@ -676,8 +675,8 @@ func TestValidatedProtectedPVCError(t *testing.T) {
 	}
 
 	expected := report.Summary{report.ValidationProblem: len(cases)}
-	if !maps.Equal(cmd.report.Summary, expected) {
-		t.Fatalf("expected summary %v, got %v", expected, cmd.report.Summary)
+	if !cmd.report.Summary.Equal(&expected) {
+		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
 }
 
@@ -2126,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)
 	}
 }
 
@@ -2160,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, r *Report, expected report.Summary) {
-	if !maps.Equal(r.Summary, expected) {
-		t.Fatalf("expected summary %v, got %v", expected, r.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 702b7aa2..1b49f6d7 100644
--- a/pkg/validate/report.go
+++ b/pkg/validate/report.go
@@ -5,35 +5,12 @@ package validate
 
 import (
 	"fmt"
-	"maps"
 
 	"github.com/ramendr/ramenctl/pkg/report"
 )
 
-// Report created by validate sub commands.
-type Report struct {
-	*report.Report
-	Summary report.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 !maps.Equal(r.Summary, o.Summary) {
-		return false
-	}
-	return true
-}
-
 // addValidation adds a validation to the summary.
-func addValidation(s report.Summary, v report.Validation) {
+func addValidation(s *report.Summary, v report.Validation) {
 	switch v.GetState() {
 	case report.OK:
 		s.Add(report.ValidationOK)
@@ -45,12 +22,12 @@ func addValidation(s report.Summary, v report.Validation) {
 }
 
 // hasIssues returns true if there are any problems or stale results.
-func hasIssues(s report.Summary) bool {
+func hasIssues(s *report.Summary) bool {
 	return s.Get(report.ValidationStale) > 0 || s.Get(report.ValidationProblem) > 0
 }
 
 // summaryString returns a string representation.
-func summaryString(s report.Summary) string {
+func summaryString(s *report.Summary) string {
 	return fmt.Sprintf("%d ok, %d stale, %d problem",
 		s.Get(report.ValidationOK), s.Get(report.ValidationStale), s.Get(report.ValidationProblem))
 }
diff --git a/pkg/validate/report_test.go b/pkg/validate/report_test.go
index 88631f74..9a8d309f 100644
--- a/pkg/validate/report_test.go
+++ b/pkg/validate/report_test.go
@@ -4,7 +4,6 @@
 package validate
 
 import (
-	"maps"
 	"testing"
 
 	"sigs.k8s.io/yaml"
@@ -15,7 +14,7 @@ import (
 )
 
 func TestSummaryAdd(t *testing.T) {
-	s := report.Summary{}
+	s := &report.Summary{}
 
 	addValidation(s, &report.Validated{State: report.OK})
 	addValidation(s, &report.Validated{State: report.OK})
@@ -29,24 +28,24 @@ func TestSummaryAdd(t *testing.T) {
 		report.ValidationStale:   2,
 		report.ValidationProblem: 1,
 	}
-	if !maps.Equal(s, expected) {
-		t.Fatalf("expected %+v, got %+v", expected, s)
+	if !s.Equal(&expected) {
+		t.Fatalf("expected %+v, got %+v", expected, *s)
 	}
 }
 
 func TestSummaryHasProblems(t *testing.T) {
 	cases := []struct {
 		name     string
-		summary  report.Summary
+		summary  *report.Summary
 		expected bool
 	}{
-		{"empty", report.Summary{}, false},
-		{"ok", report.Summary{report.ValidationOK: 5}, false},
-		{"only stale", report.Summary{report.ValidationStale: 2}, true},
-		{"only problem", report.Summary{report.ValidationProblem: 4}, true},
+		{"empty", &report.Summary{}, false},
+		{"ok", &report.Summary{report.ValidationOK: 5}, false},
+		{"only stale", &report.Summary{report.ValidationStale: 2}, true},
+		{"only problem", &report.Summary{report.ValidationProblem: 4}, true},
 		{
 			"problem and stale",
-			report.Summary{report.ValidationStale: 2, report.ValidationProblem: 3},
+			&report.Summary{report.ValidationStale: 2, report.ValidationProblem: 3},
 			true,
 		},
 	}
@@ -58,7 +57,7 @@ func TestSummaryHasProblems(t *testing.T) {
 }
 
 func TestSummaryString(t *testing.T) {
-	s := report.Summary{
+	s := &report.Summary{
 		report.ValidationOK:      1,
 		report.ValidationProblem: 2,
 	}
@@ -70,7 +69,8 @@ func TestSummaryString(t *testing.T) {
 
 func TestReportEqual(t *testing.T) {
 	helpers.FakeTime(t)
-	r1 := &Report{Report: report.NewReport("name", &config.Config{}), Summary: report.Summary{}}
+	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) {
@@ -79,7 +79,8 @@ func TestReportEqual(t *testing.T) {
 		}
 	})
 	t.Run("equal reports", func(t *testing.T) {
-		r2 := &Report{Report: report.NewReport("name", &config.Config{}), Summary: report.Summary{}}
+		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)
@@ -89,30 +90,25 @@ func TestReportEqual(t *testing.T) {
 
 func TestReportNotEqual(t *testing.T) {
 	helpers.FakeTime(t)
-	r1 := &Report{Report: report.NewReport("name", &config.Config{}), Summary: report.Summary{}}
+	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{}),
-			Summary: report.Summary{},
-		}
+		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{}),
-			Summary: report.Summary{report.ValidationOK: 5},
-		}
-		r2 := &Report{
-			Report:  report.NewReport("name", &config.Config{}),
-			Summary: report.Summary{report.ValidationOK: 3},
-		}
+		r1 := report.NewReport("name", &config.Config{})
+		r1.Summary = &report.Summary{report.ValidationOK: 5}
+		r2 := report.NewReport("name", &config.Config{})
+		r2.Summary = &report.Summary{report.ValidationOK: 3}
 		if r1.Equal(r2) {
 			t.Fatal("reports with different summary should not be equal")
 		}
@@ -120,19 +116,17 @@ func TestReportNotEqual(t *testing.T) {
 }
 
 func TestReportRoundtrip(t *testing.T) {
-	r1 := &Report{
-		Report: report.NewReport("name", &config.Config{}),
-		Summary: report.Summary{
-			report.ValidationOK:      3,
-			report.ValidationStale:   2,
-			report.ValidationProblem: 1,
-		},
+	r1 := report.NewReport("name", &config.Config{})
+	r1.Summary = &report.Summary{
+		report.ValidationOK:      3,
+		report.ValidationStale:   2,
+		report.ValidationProblem: 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)
 	}

From 8fc7031108e88179f7feee8bb3b008021c0a2e3b Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Wed, 14 Jan 2026 22:49:24 +0200
Subject: [PATCH 7/8] report: Move Summary comparison tests to report package

Since Summary is now part of report.Base and Base.Equal() handles
Summary comparison, tests for this belong in report/report_test.go.

Changes:
- Add "summary" and "nil vs non-nil summary" test cases to TestBaseNotEqual
- Remove "different summary" test case from validate/report_test.go
- Remove "different summary" test case from test/report_test.go
- Remove TestSummaryMarshal from test/report_test.go (duplicate of
  report/summary_test.go)

Tests kept in validate package (test package-specific helpers):
- TestSummaryAdd: tests addValidation() helper
- TestSummaryHasProblems: tests hasIssues() helper
- TestSummaryString: tests summaryString() helper

Tests kept in test package (test package-specific helpers):
- TestReportSummary: tests AddStep updates Summary via addTest()
- TestSummaryString: tests summaryString() helper
- TestSummaryCount: tests addTest() helper

Assisted-by: Cursor/Claude Opus 4.5
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 pkg/report/report_test.go   | 17 +++++++++++++++++
 pkg/test/report_test.go     | 31 -------------------------------
 pkg/validate/report_test.go |  9 ---------
 3 files changed, 17 insertions(+), 40 deletions(-)

diff --git a/pkg/report/report_test.go b/pkg/report/report_test.go
index 0b28b397..8a53b3ad 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.ValidationOK: 5}
+		r2 := report.NewBase("name")
+		r2.Summary = &report.Summary{report.ValidationOK: 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/test/report_test.go b/pkg/test/report_test.go
index c17cd02c..07473d24 100644
--- a/pkg/test/report_test.go
+++ b/pkg/test/report_test.go
@@ -121,13 +121,6 @@ func TestReportEqual(t *testing.T) {
 		}
 	})
 
-	t.Run("different summary", func(t *testing.T) {
-		r2 := createReport()
-		r2.Summary = &report.Summary{report.TestPassed: 1, report.TestFailed: 1}
-		if r1.Equal(r2) {
-			t.Error("reports with different summary should not be equal")
-		}
-	})
 }
 
 func TestReportMarshaling(t *testing.T) {
@@ -170,30 +163,6 @@ func TestSummaryString(t *testing.T) {
 	}
 }
 
-func TestSummaryMarshal(t *testing.T) {
-	summary := &report.Summary{
-		report.TestPassed:   4,
-		report.TestFailed:   3,
-		report.TestSkipped:  2,
-		report.TestCanceled: 1,
-	}
-
-	bytes, err := yaml.Marshal(summary)
-	if err != nil {
-		t.Fatalf("failed to marshal summary: %v", err)
-	}
-
-	var unmarshaledSummary report.Summary
-	err = yaml.Unmarshal(bytes, &unmarshaledSummary)
-	if err != nil {
-		t.Fatalf("failed to unmarshal summary: %v", err)
-	}
-	if !summary.Equal(&unmarshaledSummary) {
-		t.Errorf("unmarshaled summary %+v does not match original summary %+v",
-			unmarshaledSummary, *summary)
-	}
-}
-
 func TestSummaryCount(t *testing.T) {
 	summary := &report.Summary{}
 
diff --git a/pkg/validate/report_test.go b/pkg/validate/report_test.go
index 9a8d309f..334d1186 100644
--- a/pkg/validate/report_test.go
+++ b/pkg/validate/report_test.go
@@ -104,15 +104,6 @@ func TestReportNotEqual(t *testing.T) {
 			t.Fatal("reports with different report should not be equal")
 		}
 	})
-	t.Run("summary", func(t *testing.T) {
-		r1 := report.NewReport("name", &config.Config{})
-		r1.Summary = &report.Summary{report.ValidationOK: 5}
-		r2 := report.NewReport("name", &config.Config{})
-		r2.Summary = &report.Summary{report.ValidationOK: 3}
-		if r1.Equal(r2) {
-			t.Fatal("reports with different summary should not be equal")
-		}
-	})
 }
 
 func TestReportRoundtrip(t *testing.T) {

From 8742a41ee9c954fe5524fa238cbb61425d481fd2 Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Wed, 14 Jan 2026 23:30:50 +0200
Subject: [PATCH 8/8] report: Move Summary keys to validate and test packages

External consumers of reports need access to Summary keys to query
validation or test results without hardcoding strings:

    if report.Summary.Get(validate.Problem) > 0 {
        // handle problems
    }

Keys are now defined in their respective packages:
- validate.OK, validate.Stale, validate.Problem
- test.Passed, test.Failed, test.Skipped, test.Canceled

Inside each package, the code reads like a struct with named fields:

    report.Summary{OK: 23, Problem: 1}

This is much cleaner than the previous report.ValidationOK prefix.

This also provides practical safety through code organization -
command packages don't import each other, so misusing keys from
another package would be obviously wrong in code review.

Assisted-by: Cursor/Claude Opus 4.5
Signed-off-by: Nir Soffer <nsoffer@redhat.com>
---
 pkg/report/report_test.go    |  4 ++--
 pkg/report/summary.go        | 17 +----------------
 pkg/report/summary_test.go   | 14 +++++++-------
 pkg/test/command_test.go     | 18 +++++++++---------
 pkg/test/report.go           | 20 ++++++++++++++------
 pkg/test/report_test.go      | 28 ++++++++++++++--------------
 pkg/validate/command_test.go | 32 ++++++++++++++++----------------
 pkg/validate/report.go       | 17 ++++++++++++-----
 pkg/validate/report_test.go  | 24 ++++++++++++------------
 9 files changed, 87 insertions(+), 87 deletions(-)

diff --git a/pkg/report/report_test.go b/pkg/report/report_test.go
index 8a53b3ad..c9c9ba88 100644
--- a/pkg/report/report_test.go
+++ b/pkg/report/report_test.go
@@ -164,9 +164,9 @@ func TestBaseNotEqual(t *testing.T) {
 	})
 	t.Run("summary", func(t *testing.T) {
 		r1 := report.NewBase("name")
-		r1.Summary = &report.Summary{report.ValidationOK: 5}
+		r1.Summary = &report.Summary{report.SummaryKey("ok"): 5}
 		r2 := report.NewBase("name")
-		r2.Summary = &report.Summary{report.ValidationOK: 3}
+		r2.Summary = &report.Summary{report.SummaryKey("ok"): 3}
 		if r1.Equal(r2) {
 			t.Error("reports with different summary should not be equal")
 		}
diff --git a/pkg/report/summary.go b/pkg/report/summary.go
index e9dde1e1..d3cc7ce9 100644
--- a/pkg/report/summary.go
+++ b/pkg/report/summary.go
@@ -6,25 +6,10 @@ 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 key constants for tests.
-const (
-	TestPassed   = SummaryKey("passed")
-	TestFailed   = SummaryKey("failed")
-	TestSkipped  = SummaryKey("skipped")
-	TestCanceled = SummaryKey("canceled")
-)
-
-// Summary key constants for validations.
-const (
-	ValidationOK      = SummaryKey("ok")
-	ValidationStale   = SummaryKey("stale")
-	ValidationProblem = SummaryKey("problem")
-)
-
 // Summary is a counter for report summaries.
-// Use TestXxx keys for test reports, ValidationXxx keys for validation reports.
 type Summary map[SummaryKey]int
 
 // Add increments the counter for the given key.
diff --git a/pkg/report/summary_test.go b/pkg/report/summary_test.go
index 7fa7ad45..6466c11f 100644
--- a/pkg/report/summary_test.go
+++ b/pkg/report/summary_test.go
@@ -14,10 +14,10 @@ import (
 
 func TestTestSummaryYAMLRoundtrip(t *testing.T) {
 	s1 := report.Summary{
-		report.TestPassed:   5,
-		report.TestFailed:   2,
-		report.TestSkipped:  1,
-		report.TestCanceled: 0,
+		report.SummaryKey("passed"):   5,
+		report.SummaryKey("failed"):   2,
+		report.SummaryKey("skipped"):  1,
+		report.SummaryKey("canceled"): 0,
 	}
 
 	data, err := yaml.Marshal(s1)
@@ -37,9 +37,9 @@ func TestTestSummaryYAMLRoundtrip(t *testing.T) {
 
 func TestValidationSummaryYAMLRoundtrip(t *testing.T) {
 	s1 := report.Summary{
-		report.ValidationOK:      10,
-		report.ValidationStale:   1,
-		report.ValidationProblem: 2,
+		report.SummaryKey("ok"):      10,
+		report.SummaryKey("stale"):   1,
+		report.SummaryKey("problem"): 2,
 	}
 
 	data, err := yaml.Marshal(s1)
diff --git a/pkg/test/command_test.go b/pkg/test/command_test.go
index 17a31af0..d30f492c 100644
--- a/pkg/test/command_test.go
+++ b/pkg/test/command_test.go
@@ -134,7 +134,7 @@ func TestRunPassed(t *testing.T) {
 		t,
 		test.report,
 		report.Passed,
-		report.Summary{report.TestPassed: len(testConfig.Tests)},
+		report.Summary{Passed: len(testConfig.Tests)},
 	)
 	if len(test.report.Steps) != 3 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
@@ -226,7 +226,7 @@ func TestRunTestsFailed(t *testing.T) {
 		t,
 		test.report,
 		report.Failed,
-		report.Summary{report.TestFailed: len(testConfig.Tests)},
+		report.Summary{Failed: len(testConfig.Tests)},
 	)
 	if len(test.report.Steps) != 3 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
@@ -254,7 +254,7 @@ func TestRunDisappFailed(t *testing.T) {
 		t,
 		test.report,
 		report.Failed,
-		report.Summary{report.TestPassed: 4, report.TestFailed: 2},
+		report.Summary{Passed: 4, Failed: 2},
 	)
 	if len(test.report.Steps) != 3 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
@@ -286,7 +286,7 @@ func TestRunTestsCanceled(t *testing.T) {
 		t,
 		test.report,
 		report.Canceled,
-		report.Summary{report.TestCanceled: len(testConfig.Tests)},
+		report.Summary{Canceled: len(testConfig.Tests)},
 	)
 	if len(test.report.Steps) != 3 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
@@ -310,7 +310,7 @@ func TestCleanPassed(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	checkReport(t, test.report, report.Passed, report.Summary{report.TestPassed: 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)
 	}
@@ -367,7 +367,7 @@ func TestCleanPurgeFailed(t *testing.T) {
 		t,
 		test.report,
 		report.Failed,
-		report.Summary{report.TestFailed: len(testConfig.Tests)},
+		report.Summary{Failed: len(testConfig.Tests)},
 	)
 	if len(test.report.Steps) != 2 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
@@ -393,7 +393,7 @@ func TestCleanPurgeCanceled(t *testing.T) {
 		t,
 		test.report,
 		report.Canceled,
-		report.Summary{report.TestCanceled: len(testConfig.Tests)},
+		report.Summary{Canceled: len(testConfig.Tests)},
 	)
 	if len(test.report.Steps) != 2 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
@@ -419,7 +419,7 @@ func TestCleanCleanupFailed(t *testing.T) {
 		t,
 		test.report,
 		report.Failed,
-		report.Summary{report.TestPassed: len(testConfig.Tests)},
+		report.Summary{Passed: len(testConfig.Tests)},
 	)
 	if len(test.report.Steps) != 3 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
@@ -447,7 +447,7 @@ func TestCleanCleanupCanceled(t *testing.T) {
 		t,
 		test.report,
 		report.Canceled,
-		report.Summary{report.TestPassed: len(testConfig.Tests)},
+		report.Summary{Passed: len(testConfig.Tests)},
 	)
 	if len(test.report.Steps) != 3 {
 		t.Fatalf("unexpected steps %+v", test.report.Steps)
diff --git a/pkg/test/report.go b/pkg/test/report.go
index 30edb3d3..8d164a6a 100644
--- a/pkg/test/report.go
+++ b/pkg/test/report.go
@@ -18,6 +18,14 @@ const (
 	CleanupStep  = "cleanup"
 )
 
+// 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
@@ -72,18 +80,18 @@ func (r *Report) Equal(o *Report) bool {
 func addTest(s *report.Summary, t *report.Step) {
 	switch t.Status {
 	case report.Passed:
-		s.Add(report.TestPassed)
+		s.Add(Passed)
 	case report.Failed:
-		s.Add(report.TestFailed)
+		s.Add(Failed)
 	case report.Skipped:
-		s.Add(report.TestSkipped)
+		s.Add(Skipped)
 	case report.Canceled:
-		s.Add(report.TestCanceled)
+		s.Add(Canceled)
 	}
 }
 
 func summaryString(s *report.Summary) string {
 	return fmt.Sprintf("%d passed, %d failed, %d skipped, %d canceled",
-		s.Get(report.TestPassed), s.Get(report.TestFailed),
-		s.Get(report.TestSkipped), s.Get(report.TestCanceled))
+		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 07473d24..4899b50c 100644
--- a/pkg/test/report_test.go
+++ b/pkg/test/report_test.go
@@ -60,10 +60,10 @@ func TestReportSummary(t *testing.T) {
 	r.AddStep(testsStep)
 
 	expectedSummary := &report.Summary{
-		report.TestPassed:   1,
-		report.TestFailed:   1,
-		report.TestSkipped:  1,
-		report.TestCanceled: 1,
+		Passed:   1,
+		Failed:   1,
+		Skipped:  1,
+		Canceled: 1,
 	}
 	if !r.Summary.Equal(expectedSummary) {
 		t.Errorf("expected summary %+v, got %+v", expectedSummary, r.Summary)
@@ -75,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 = &report.Summary{report.TestPassed: 2}
+		r.Summary = &report.Summary{Passed: 2}
 		return r
 	}
 
@@ -144,7 +144,7 @@ func TestReportMarshaling(t *testing.T) {
 			Duration: 1.0,
 		},
 	}
-	r.Summary = &report.Summary{report.TestPassed: 2, report.TestFailed: 1}
+	r.Summary = &report.Summary{Passed: 2, Failed: 1}
 
 	// Test roundtrip marshaling/unmarshaling
 	checkRoundtrip(t, r)
@@ -152,10 +152,10 @@ func TestReportMarshaling(t *testing.T) {
 
 func TestSummaryString(t *testing.T) {
 	summary := &report.Summary{
-		report.TestPassed:   5,
-		report.TestFailed:   2,
-		report.TestSkipped:  3,
-		report.TestCanceled: 1,
+		Passed:   5,
+		Failed:   2,
+		Skipped:  3,
+		Canceled: 1,
 	}
 	expectedString := "5 passed, 2 failed, 3 skipped, 1 canceled"
 	if summaryString(summary) != expectedString {
@@ -175,10 +175,10 @@ func TestSummaryCount(t *testing.T) {
 	addTest(summary, &report.Step{Status: report.Passed})
 
 	expectedSummary := &report.Summary{
-		report.TestPassed:   3,
-		report.TestFailed:   1,
-		report.TestSkipped:  1,
-		report.TestCanceled: 1,
+		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/command_test.go b/pkg/validate/command_test.go
index 77a2bcba..f9cb4497 100644
--- a/pkg/validate/command_test.go
+++ b/pkg/validate/command_test.go
@@ -265,7 +265,7 @@ func TestValidatedDeleted(t *testing.T) {
 	})
 
 	t.Run("update summary", func(t *testing.T) {
-		expected := report.Summary{report.ValidationOK: 1, report.ValidationProblem: 2}
+		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,7 +315,7 @@ func TestValidatedDRPCAction(t *testing.T) {
 	})
 
 	t.Run("update summary", func(t *testing.T) {
-		expected := report.Summary{report.ValidationOK: 3, report.ValidationProblem: 1}
+		expected := report.Summary{OK: 3, Problem: 1}
 		if !cmd.report.Summary.Equal(&expected) {
 			t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 		}
@@ -400,7 +400,7 @@ func TestValidatedDRPCPhaseError(t *testing.T) {
 	for _, group := range unstable {
 		errors += len(group.cases)
 	}
-	expected := report.Summary{report.ValidationProblem: errors}
+	expected := report.Summary{Problem: errors}
 	if !cmd.report.Summary.Equal(&expected) {
 		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
@@ -442,7 +442,7 @@ func TestValidatedDRPCPhaseOK(t *testing.T) {
 		})
 	}
 
-	expected := report.Summary{report.ValidationOK: len(cases)}
+	expected := report.Summary{OK: len(cases)}
 	if !cmd.report.Summary.Equal(&expected) {
 		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
@@ -470,7 +470,7 @@ func TestValidatedDRPCProgressionOK(t *testing.T) {
 		}
 	})
 
-	expected := report.Summary{report.ValidationOK: 1}
+	expected := report.Summary{OK: 1}
 	if !cmd.report.Summary.Equal(&expected) {
 		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
@@ -527,7 +527,7 @@ func TestValidatedDRPCProgressionError(t *testing.T) {
 		})
 	}
 
-	expected := report.Summary{report.ValidationProblem: len(progressions)}
+	expected := report.Summary{Problem: len(progressions)}
 	if !cmd.report.Summary.Equal(&expected) {
 		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
@@ -564,7 +564,7 @@ func TestValidatedVRGSTateOK(t *testing.T) {
 		})
 	}
 
-	expected := report.Summary{report.ValidationOK: len(cases)}
+	expected := report.Summary{OK: len(cases)}
 	if !cmd.report.Summary.Equal(&expected) {
 		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
@@ -607,7 +607,7 @@ func TestValidatedVRGSTateError(t *testing.T) {
 		})
 	}
 
-	expected := report.Summary{report.ValidationProblem: len(cases)}
+	expected := report.Summary{Problem: len(cases)}
 	if !cmd.report.Summary.Equal(&expected) {
 		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
@@ -634,7 +634,7 @@ func TestValidatedProtectedPVCOK(t *testing.T) {
 		}
 	})
 
-	expected := report.Summary{report.ValidationOK: 1}
+	expected := report.Summary{OK: 1}
 	if !cmd.report.Summary.Equal(&expected) {
 		t.Fatalf("expected summary %v, got %v", expected, *cmd.report.Summary)
 	}
@@ -674,7 +674,7 @@ func TestValidatedProtectedPVCError(t *testing.T) {
 		})
 	}
 
-	expected := report.Summary{report.ValidationProblem: len(cases)}
+	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, report.Summary{report.ValidationOK: 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, report.Summary{report.ValidationOK: 40})
+	checkSummary(t, validate.report, report.Summary{OK: 40})
 }
 
 func TestValidateClustersValidateFailed(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, report.Summary{report.ValidationProblem: 9})
+	checkSummary(t, validate.report, report.Summary{Problem: 9})
 }
 
 func TestValidateClustersCheckS3Failed(t *testing.T) {
@@ -1630,7 +1630,7 @@ func TestValidateClustersCheckS3Failed(t *testing.T) {
 	checkSummary(
 		t,
 		validate.report,
-		report.Summary{report.ValidationOK: 41, report.ValidationProblem: 1},
+		report.Summary{OK: 41, Problem: 1},
 	)
 }
 
@@ -1883,7 +1883,7 @@ func TestValidateApplicationPassed(t *testing.T) {
 	}
 	checkApplicationStatus(t, validate.report, expectedStatus)
 
-	checkSummary(t, validate.report, report.Summary{report.ValidationOK: 24})
+	checkSummary(t, validate.report, report.Summary{OK: 24})
 }
 
 func TestValidateApplicationValidateFailed(t *testing.T) {
@@ -2057,7 +2057,7 @@ func TestValidateApplicationGatherS3Failed(t *testing.T) {
 	checkSummary(
 		t,
 		validate.report,
-		report.Summary{report.ValidationOK: 23, report.ValidationProblem: 1},
+		report.Summary{OK: 23, Problem: 1},
 	)
 }
 
diff --git a/pkg/validate/report.go b/pkg/validate/report.go
index 1b49f6d7..74623b10 100644
--- a/pkg/validate/report.go
+++ b/pkg/validate/report.go
@@ -9,25 +9,32 @@ import (
 	"github.com/ramendr/ramenctl/pkg/report"
 )
 
+// Summary keys for validation reports.
+const (
+	OK      = report.SummaryKey("ok")
+	Stale   = report.SummaryKey("stale")
+	Problem = report.SummaryKey("problem")
+)
+
 // addValidation adds a validation to the summary.
 func addValidation(s *report.Summary, v report.Validation) {
 	switch v.GetState() {
 	case report.OK:
-		s.Add(report.ValidationOK)
+		s.Add(OK)
 	case report.Stale:
-		s.Add(report.ValidationStale)
+		s.Add(Stale)
 	case report.Problem:
-		s.Add(report.ValidationProblem)
+		s.Add(Problem)
 	}
 }
 
 // hasIssues returns true if there are any problems or stale results.
 func hasIssues(s *report.Summary) bool {
-	return s.Get(report.ValidationStale) > 0 || s.Get(report.ValidationProblem) > 0
+	return s.Get(Stale) > 0 || s.Get(Problem) > 0
 }
 
 // summaryString returns a string representation.
 func summaryString(s *report.Summary) string {
 	return fmt.Sprintf("%d ok, %d stale, %d problem",
-		s.Get(report.ValidationOK), s.Get(report.ValidationStale), s.Get(report.ValidationProblem))
+		s.Get(OK), s.Get(Stale), s.Get(Problem))
 }
diff --git a/pkg/validate/report_test.go b/pkg/validate/report_test.go
index 334d1186..93bb57e0 100644
--- a/pkg/validate/report_test.go
+++ b/pkg/validate/report_test.go
@@ -24,9 +24,9 @@ func TestSummaryAdd(t *testing.T) {
 	addValidation(s, &report.Validated{State: report.Problem})
 
 	expected := report.Summary{
-		report.ValidationOK:      3,
-		report.ValidationStale:   2,
-		report.ValidationProblem: 1,
+		OK:      3,
+		Stale:   2,
+		Problem: 1,
 	}
 	if !s.Equal(&expected) {
 		t.Fatalf("expected %+v, got %+v", expected, *s)
@@ -40,12 +40,12 @@ func TestSummaryHasProblems(t *testing.T) {
 		expected bool
 	}{
 		{"empty", &report.Summary{}, false},
-		{"ok", &report.Summary{report.ValidationOK: 5}, false},
-		{"only stale", &report.Summary{report.ValidationStale: 2}, true},
-		{"only problem", &report.Summary{report.ValidationProblem: 4}, true},
+		{"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{report.ValidationStale: 2, report.ValidationProblem: 3},
+			&report.Summary{Stale: 2, Problem: 3},
 			true,
 		},
 	}
@@ -58,8 +58,8 @@ func TestSummaryHasProblems(t *testing.T) {
 
 func TestSummaryString(t *testing.T) {
 	s := &report.Summary{
-		report.ValidationOK:      1,
-		report.ValidationProblem: 2,
+		OK:      1,
+		Problem: 2,
 	}
 	expected := "1 ok, 0 stale, 2 problem"
 	if summaryString(s) != expected {
@@ -109,9 +109,9 @@ func TestReportNotEqual(t *testing.T) {
 func TestReportRoundtrip(t *testing.T) {
 	r1 := report.NewReport("name", &config.Config{})
 	r1.Summary = &report.Summary{
-		report.ValidationOK:      3,
-		report.ValidationStale:   2,
-		report.ValidationProblem: 1,
+		OK:      3,
+		Stale:   2,
+		Problem: 1,
 	}
 	b, err := yaml.Marshal(r1)
 	if err != nil {