🧪 crane validate: Automate validation of YAML report for live mode#647
🧪 crane validate: Automate validation of YAML report for live mode#647nachandr wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThe live compatibility e2e test now validates JSON and YAML reports, checks live metadata and resources, verifies output artifacts, and compares both formats. Shared validation helpers also assert the expected cluster context. ChangesLive report validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Tier0Test
participant ValidateRunner
participant ReportParser
Tier0Test->>ValidateRunner: run live validation as JSON
Tier0Test->>ValidateRunner: run live validation as YAML
ValidateRunner-->>Tier0Test: write validation reports
Tier0Test->>ReportParser: parse JSON and YAML reports
ReportParser-->>Tier0Test: return report metadata and resources
Tier0Test->>Tier0Test: compare report fields and resource results
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Test Coverage ReportTotal: 48.5% Per-package coverage
Full function-level detailsPosted by CI |
Signed-off-by: Nandini Chandra <nachandr@redhat.com>
Signed-off-by: Nandini Chandra <nachandr@redhat.com>
8f5c2a3 to
f985166
Compare
Test Coverage ReportTotal: 48.5% Per-package coverage
Full function-level detailsPosted by CI |
Signed-off-by: Nandini Chandra <nachandr@redhat.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
e2e-tests/utils/utils_validate.go (2)
19-25: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd absolute
resourcePluralexpectations to the shared validation contract. Cross-format equality cannot catch a shared incorrect value.
e2e-tests/utils/utils_validate.go#L19-L25: represent both expected API version and resource plural for each kind.e2e-tests/utils/utils_validate.go#L64-L78: assert each result’s plural against that expectation.e2e-tests/tests/tier0/mta_833_compatible_resources_live_test.go#L120-L138: provide expected plurals for all five resources.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e-tests/utils/utils_validate.go` around lines 19 - 25, Extend ValidationExpectations in e2e-tests/utils/utils_validate.go (lines 19-25) so each expected kind stores both API version and absolute resourcePlural; update the result assertions in e2e-tests/utils/utils_validate.go (lines 64-78) to compare each result’s plural against that expectation. Populate expected plurals for all five resources in e2e-tests/tests/tier0/mta_833_compatible_resources_live_test.go (lines 120-138).
95-105: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winVerify that
failuresis actually a directory.When failures are expected, a regular file at this path currently passes
os.Stat.Proposed fix
- _, err := os.Stat(failuresDir) + info, err := os.Stat(failuresDir) if expectations.ExpectFailuresDir { - Expect(err).NotTo(HaveOccurred(), "expected failures/ directory to exist") + Expect(err).NotTo(HaveOccurred(), "expected failures directory %q to exist", failuresDir) + Expect(info.IsDir()).To(BeTrue(), "expected %q to be a directory", failuresDir)As per coding guidelines, “Prefer explicit error messages with context in Go code.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e-tests/utils/utils_validate.go` around lines 95 - 105, Update the failures-directory validation in the expectations.ExpectFailuresDir branch to stat the path and assert that the returned FileInfo identifies a directory, not merely that os.Stat succeeded. Preserve the existing absence check when failures are not expected, and include contextual assertion messages for stat and directory validation failures.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@e2e-tests/utils/utils_validate.go`:
- Around line 19-25: Extend ValidationExpectations in
e2e-tests/utils/utils_validate.go (lines 19-25) so each expected kind stores
both API version and absolute resourcePlural; update the result assertions in
e2e-tests/utils/utils_validate.go (lines 64-78) to compare each result’s plural
against that expectation. Populate expected plurals for all five resources in
e2e-tests/tests/tier0/mta_833_compatible_resources_live_test.go (lines 120-138).
- Around line 95-105: Update the failures-directory validation in the
expectations.ExpectFailuresDir branch to stat the path and assert that the
returned FileInfo identifies a directory, not merely that os.Stat succeeded.
Preserve the existing absence check when failures are not expected, and include
contextual assertion messages for stat and directory validation failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aff62da2-7c2c-496f-93de-596dfacb01e2
📒 Files selected for processing (2)
e2e-tests/tests/tier0/mta_833_compatible_resources_live_test.goe2e-tests/utils/utils_validate.go
|
/rfr |
|
I'm working on addressing CR review comments . |
Verifies 631
Summary by CodeRabbit
Summary by CodeRabbit