Remove duplicated T2 must-gather tests covered by T1 upstream#4013
Open
OhadRevah wants to merge 1 commit intoRedHatQE:mainfrom
Open
Remove duplicated T2 must-gather tests covered by T1 upstream#4013OhadRevah wants to merge 1 commit intoRedHatQE:mainfrom
OhadRevah wants to merge 1 commit intoRedHatQE:mainfrom
Conversation
Remove T2 tests that duplicate validation already performed by T1 tests in the upstream kubevirt/must-gather repo. T1 cluster-scoped-resources validation (lines 101-161) checks that each expected resource directory exists, opens the YAML file, fetches the same resource from the cluster, and does reflect.DeepEqual on the full spec. Our T2 tests only compared selected fields (uid/name or uid/name/spec), so T1 is strictly stricter. - CNV-3042 (NetworkAddonsConfig): T1 validates networkaddonsconfigs at cluster-scoped-resources/ with full spec DeepEqual. https://github.com/kubevirt/must-gather/blob/main/tests/validate_must_gather_test.go#L104 - CNV-3373 (CDIConfig): T1 validates cdiconfigs at cluster-scoped-resources/ with full spec DeepEqual. https://github.com/kubevirt/must-gather/blob/main/tests/validate_must_gather_test.go#L102 T1 VM validation (lines 263-282) iterates VMs per namespace, builds the path at kubevirt.io/virtualmachines/{type}/{name}.yaml, then calls validateVmFile (lines 389-406) which does reflect.DeepEqual on the full spec. Our T2 test only compared uid/name/spec fields. - CNV-3043 (VirtualMachine): T1 validates VM YAMLs with full spec DeepEqual via validateVmFile. https://github.com/kubevirt/must-gather/blob/main/tests/validate_must_gather_test.go#L263-L282 https://github.com/kubevirt/must-gather/blob/main/tests/validate_must_gather_test.go#L389-L406 - CNV-2939 (ImageStreamTag count): Not a T1 duplicate. Consolidation within our test suite — CNV-9236 in test_must_gather_images.py already validates ImageStreamTag uid/name. The count validation from CNV-2939 is merged into CNV-9236, so no coverage is lost. Additional changes: - Add pytest.mark.dependency between test_no_new_cnv_crds (CNV-8508) and test_crd_resources (CNV-2724), since an outdated CRD list means test_crd_resources validates an incomplete set. - Clean up unused imports (CDIConfig, NetworkAddonsConfig, ImageStreamTag, VirtualMachine). assisted by: claude code claude-opus-4-6 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughRestructures must-gather test suite: removes resource checks for NetworkAddonsConfig and CDIConfig; moves ImageStreamTag test to dedicated method; removes VirtualMachine test from parametrization; adds test dependency ordering for CNV CRD validation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
openshift-virtualization-qe-bot-5
requested review from
RoniKishner,
dshchedr,
rlobillo and
vsibirsk
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/install_upgrade_operators/must_gather/test_must_gather_images.py`:
- Around line 49-63: The test_image_stream_tag_gather should defensively handle
a missing istag_dir: before calling os.listdir(istag_dir) check
os.path.exists(istag_dir) (or os.path.isdir) and assert with a clear message
like "must-gather did not produce ImageStreamTag directory at {istag_dir}" or
skip the test if that represents no data; then proceed to compare
len(os.listdir(istag_dir)) to list(ImageStreamTag.get(...)) as before and call
check_list_of_resources with the same parameters (resource_type=ImageStreamTag,
temp_dir=gathered_images, resource_path=f"{resource_path}/{{name}}.yaml",
checks=VALIDATE_UID_NAME, filter_resource="redhat").
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
tests/install_upgrade_operators/must_gather/test_must_gather.pytests/install_upgrade_operators/must_gather/test_must_gather_images.pytests/install_upgrade_operators/must_gather/test_must_gather_vms.py
💤 Files with no reviewable changes (1)
- tests/install_upgrade_operators/must_gather/test_must_gather_vms.py
Contributor
Author
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-4013 published |
dshchedr
approved these changes
Mar 4, 2026
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
6 participants
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.
Short description:
Remove T2 tests that duplicate validation already performed by T1 tests in the upstream kubevirt/must-gather repo and duplicate tests.
T1 cluster-scoped-resources validation (lines 101-161) checks that each expected resource directory exists, opens the YAML file, fetches the same resource from the cluster, and does reflect.DeepEqual on the full spec. Our T2 tests only compared selected fields (uid/name or uid/name/spec), so T1 is strictly stricter.
T1 VM validation (lines 263-282) iterates VMs per namespace, builds the path at kubevirt.io/virtualmachines/{type}/{name}.yaml, then calls validateVmFile (lines 389-406) which does reflect.DeepEqual on the full spec. Our T2 test only compared uid/name/spec fields.
CNV-3043 (VirtualMachine): T1 validates VM YAMLs with full spec DeepEqual via validateVmFile. https://github.com/kubevirt/must-gather/blob/main/tests/validate_must_gather_test.go#L263-L282 https://github.com/kubevirt/must-gather/blob/main/tests/validate_must_gather_test.go#L389-L406
CNV-2939 (ImageStreamTag count): Not a T1 duplicate. Consolidation within our test suite — CNV-9236 in test_must_gather_images.py already validates ImageStreamTag uid/name. The count validation from CNV-2939 is merged into CNV-9236, so no coverage is lost.
Additional changes:
assisted by: claude code claude-opus-4-6
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
https://issues.redhat.com/browse/CNV-35404
Summary by CodeRabbit