-
Notifications
You must be signed in to change notification settings - Fork 12
Unify Report Types for HTML Report Generation #364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
43785f8 to
e9ddf71
Compare
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>
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>
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>
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>
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>
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>
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>
5f58c44 to
0d392f3
Compare
parikshithb
previously approved these changes
Jan 16, 2026
Member
parikshithb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Well structured, was easy to follow.
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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactors the report types to enable generic HTML report generation. Previously,
validateandtestpackages had their own Report types with separate Summary implementations, making it difficult to create a single HTML template.Key Changes
Unified Summary Type (
report.Summary)Summarytype asmap[SummaryKey]intinreportpackagevalidate.OK,validate.Stale,validate.Problemtest.Passed,test.Failed,test.Skipped,test.CanceledSummary.Equal()method for nil-safe pointer comparisonreport.Baseso bothreport.Reportandtest.Reportshare itreportpackage, remove duplicatesPackage-scoped Summary Keys
Keys are exported for external consumers to query reports without hardcoding strings:
Inside each package, the code reads like a struct with named fields:
Eliminate validate.Report
validate.Reportstruct - validate package now usesreport.ReportdirectlyaddValidation(),hasIssues(),summaryString()Simplify test.Report
Summaryfield fromtest.Report(gets it from embeddedBase)addTest(),summaryString()Refactor YAML Report Writing
report.WriteYAML(io.Writer, any)for generic YAML serializationcommand.OpenReport(format)for file managementcommandhandles files,reporthandles serializationOther
.cursor/rules.mdwith project conventions for AI assistantsResult
With these changes, both
validateandtestcommands share the same Summary type throughreport.Base. This is a prerequisite for creating a single HTML template that works with all command reports.