crane validate: Test scanner deduplication#557
Open
nachandr wants to merge 1 commit into
Open
Conversation
Signed-off-by: Nandini Chandra <nachandr@redhat.com>
📝 WalkthroughWalkthroughAdds two Kubernetes Deployment YAML fixtures ( ChangesMTA-385 Duplicate GVK E2E Test
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 46.6% Per-package coverage
Full function-level detailsPosted by CI |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
e2e-tests/testdata/issue-385-duplicate-deployment-1.yaml (1)
1-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRename fixture files to match the required YAML naming convention.
issue-385-duplicate-deployment-1.yaml(and its siblingissue-385-duplicate-deployment-2.yaml) does not followKind_group_version_namespace_name.yaml. Please rename both fixtures and update the corresponding references ine2e-tests/tests/tier1/mta_385_validate_duplicate_gvk_test.go(Lines 38-39).As per coding guidelines, "
**/*.yaml: Resource names should follow format:Kind_group_version_namespace_name.yaml."🤖 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/testdata/issue-385-duplicate-deployment-1.yaml` around lines 1 - 21, The fixture files issue-385-duplicate-deployment-1.yaml and issue-385-duplicate-deployment-2.yaml do not follow the required naming convention of Kind_group_version_namespace_name.yaml. Rename both files to match the convention based on their content: use Deployment as the Kind, apps as the group, v1 as the version, test-namespace as the namespace, and test-deployment-1 and test-deployment-2 as the names respectively. Then update the corresponding file path references in the mta_385_validate_duplicate_gvk_test.go test file to point to the newly renamed fixture files.Source: Coding guidelines
🧹 Nitpick comments (1)
e2e-tests/tests/tier1/mta_385_validate_duplicate_gvk_test.go (1)
101-103: ⚡ Quick winAssert the exact cluster context value, not just non-empty.
Given this test’s goal is report metadata correctness,
report.ClusterContextshould matchscenario.KubectlTgtNonAdmin.Context; a non-empty check can miss regressions that write the wrong context.Suggested assertion update
- Expect(report.ClusterContext).NotTo(BeEmpty(), "expected clusterContext to be set in live mode") + Expect(report.ClusterContext).To(Equal(scenario.KubectlTgtNonAdmin.Context), + "expected clusterContext to match the target non-admin context")🤖 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/tests/tier1/mta_385_validate_duplicate_gvk_test.go` around lines 101 - 103, Replace the NotTo(BeEmpty()) assertion on report.ClusterContext with a direct equality check to verify it matches the expected value scenario.KubectlTgtNonAdmin.Context. Change the Expect statement to compare report.ClusterContext with scenario.KubectlTgtNonAdmin.Context using Equal() matcher to ensure the correct context is being set in the report, rather than just checking for a non-empty value which could miss regressions where an incorrect context is written.
🤖 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/testdata/issue-385-duplicate-deployment-1.yaml`:
- Around line 1-21: The fixture files issue-385-duplicate-deployment-1.yaml and
issue-385-duplicate-deployment-2.yaml do not follow the required naming
convention of Kind_group_version_namespace_name.yaml. Rename both files to match
the convention based on their content: use Deployment as the Kind, apps as the
group, v1 as the version, test-namespace as the namespace, and test-deployment-1
and test-deployment-2 as the names respectively. Then update the corresponding
file path references in the mta_385_validate_duplicate_gvk_test.go test file to
point to the newly renamed fixture files.
---
Nitpick comments:
In `@e2e-tests/tests/tier1/mta_385_validate_duplicate_gvk_test.go`:
- Around line 101-103: Replace the NotTo(BeEmpty()) assertion on
report.ClusterContext with a direct equality check to verify it matches the
expected value scenario.KubectlTgtNonAdmin.Context. Change the Expect statement
to compare report.ClusterContext with scenario.KubectlTgtNonAdmin.Context using
Equal() matcher to ensure the correct context is being set in the report, rather
than just checking for a non-empty value which could miss regressions where an
incorrect context is written.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 38063999-ce56-4f56-8223-9baf438d9af8
📒 Files selected for processing (3)
e2e-tests/testdata/issue-385-duplicate-deployment-1.yamle2e-tests/testdata/issue-385-duplicate-deployment-2.yamle2e-tests/tests/tier1/mta_385_validate_duplicate_gvk_test.go
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Fixes #385
Summary by CodeRabbit