[4.19][Storage] Move tests away from test_import_http.py and replace using datasource cloning#5202
Conversation
…py and replace using datasource cloning (RedHatQE#4646) Manual backport of RedHatQE#4455 <!-- full-ticket-url needs to be provided. This would add a link to the pull request to the jira and close it when the pull request is merged If the task is not tracked by a Jira ticket, just write "NONE". --> --------- Signed-off-by: Emanuele Prella <eprella@redhat.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
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
AI Features
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
openshift-virtualization-qe-bot-3
requested review from
RoniKishner and
vsibirsk
openshift-virtualization-qe-bot-3
requested review from
Ahmad-Hafe,
dalia-frank,
dshchedr,
geetikakay,
josemacassan,
jpeimer,
kgoldbla and
rnetser
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-5202 published |
|
Verification failed for PR #5202. Execution details |
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-5202 published |
|
/lgtm |
|
/approve |
|
/verified cancel |
|
/verified |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get current PR HEAD SHA and first changed file info
PR_DATA=$(gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5202)
HEAD_SHA=$(echo "$PR_DATA" | jq -r '.head.sha')
echo "HEAD SHA: $HEAD_SHA"
# Get first changed file and patch info
gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5202/files | jq '.[0] | {filename, sha, patch: .patch[0:600]}'Length of output: 1154 🏁 Script executed: #!/bin/bash
HEAD_SHA="174bc63eec39d933180754533d460c19ec54a542"
BODY="## Test Execution Plan
**Run smoke tests: False**
No traced dependency path from any smoke-marked test to the modified symbols (\`skip_access_mode_rwo_scope_function\` removal in \`tests/conftest.py\`, fixture changes in \`tests/storage/cdi_import/conftest.py\`, \`_cache_admin_client\` change in \`utilities/pytest_matrix_utils.py\`).
**Run gating tests: True**
Path: \`tests/storage/cdi_import/test_import_http.py\` (contains gating-marked tests) → fixtures directly refactored in \`tests/storage/cdi_import/conftest.py\` (modified in this PR).
**Affected tests to run**
- \`tests/storage/cdi_import/test_import_http.py\` (gating + tier2 — fixtures refactored, tests removed/retained)
- \`tests/storage/general/test_storage_behavior.py\` (tier2 — new tests moved from \`test_import_http.py\`: CNV-675, CNV-3065, CNV-3632)
- \`tests/storage/cdi_import/test_import_registry.py::test_importer_pod_annotation\` (tier2 — shares refactored \`cdi_import/conftest.py\`)
**Concrete commands:**
\`\`\`bash
# Gating tests in the directly modified file
pytest -s -o log_cli=true -m gating tests/storage/cdi_import/test_import_http.py
# New tests moved to storage/general
pytest -s -o log_cli=true -m tier2 --jira tests/storage/general/test_storage_behavior.py
# Registry import test sharing refactored conftest
pytest -s -o log_cli=true tests/storage/cdi_import/test_import_registry.py::test_importer_pod_annotation
\`\`\`"
gh api \
--method POST \
repos/RedHatQE/openshift-virtualization-tests/pulls/5202/reviews \
-f commit_id="$HEAD_SHA" \
-f body="" \
-f event="COMMENT" \
-f "comments[][path]=tests/conftest.py" \
-F "comments[][position]=5" \
-f "comments[][body]=$BODY"Length of output: 3547 |
|
/approve |
|
Clean rebase detected — no code changes compared to previous head ( |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-5202 published |
|
/verified All tests passed for PR #5202. Execution details |
|
/verified cancel |
|
/verified |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
|
|
||
|
|
||
| @pytest.fixture() | ||
| def skip_access_mode_rwo_scope_function(storage_class_matrix__function__): |
There was a problem hiding this comment.
Test Execution Plan
Run smoke tests: False
No traced dependency path from any smoke-marked test to the modified symbols (skip_access_mode_rwo_scope_function removal in tests/conftest.py, fixture changes in tests/storage/cdi_import/conftest.py, _cache_admin_client change in utilities/pytest_matrix_utils.py).
Run gating tests: True
Path: tests/storage/cdi_import/test_import_http.py (contains gating-marked tests) → fixtures directly refactored in tests/storage/cdi_import/conftest.py (modified in this PR).
Affected tests to run
tests/storage/cdi_import/test_import_http.py(gating + tier2 — fixtures refactored, tests removed/retained)tests/storage/general/test_storage_behavior.py(tier2 — new tests moved fromtest_import_http.py: CNV-675, CNV-3065, CNV-3632)tests/storage/cdi_import/test_import_registry.py::test_importer_pod_annotation(tier2 — shares refactoredcdi_import/conftest.py)
Concrete commands:
# Gating tests in the directly modified file
pytest -s -o log_cli=true -m gating tests/storage/cdi_import/test_import_http.py
# New tests moved to storage/general
pytest -s -o log_cli=true -m tier2 --jira tests/storage/general/test_storage_behavior.py
# Registry import test sharing refactored conftest
pytest -s -o log_cli=true tests/storage/cdi_import/test_import_registry.py::test_importer_pod_annotation
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get current PR HEAD SHA and first changed file info
PR_DATA=$(gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5202)
HEAD_SHA=$(echo "$PR_DATA" | jq -r '.head.sha')
echo "HEAD SHA: $HEAD_SHA"
# Get first changed file and patch info
gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5202/files | jq '.[0] | {filename, sha, patch: .patch[0:800]}'Length of output: 1154 🏁 Script executed: #!/bin/bash
HEAD_SHA="e11b15409442b212694cf08d91ee5db02b382f5c"
BODY="## Test Execution Plan
**Run smoke tests: False**
No traced dependency path from any smoke-marked test to the modified symbols (\`skip_access_mode_rwo_scope_function\` removal in \`tests/conftest.py\`, fixture changes in \`tests/storage/cdi_import/conftest.py\`, \`_cache_admin_client\` change in \`utilities/pytest_matrix_utils.py\`).
**Run gating tests: True**
Path: \`tests/storage/cdi_import/test_import_http.py\` (contains gating-marked tests) → fixtures directly refactored in \`tests/storage/cdi_import/conftest.py\` (modified in this PR).
**Affected tests to run**
- \`tests/storage/cdi_import/test_import_http.py\` (gating + tier2 — fixtures refactored, tests removed/retained)
- \`tests/storage/general/test_storage_behavior.py\` (tier2 — new tests moved from \`test_import_http.py\`: CNV-675, CNV-3065, CNV-3632)
- \`tests/storage/cdi_import/test_import_registry.py::test_importer_pod_annotation\` (tier2 — shares refactored \`cdi_import/conftest.py\`)
**Concrete commands:**
\`\`\`bash
# Gating tests in the directly modified file
pytest -s -o log_cli=true -m gating tests/storage/cdi_import/test_import_http.py
# New tests moved to storage/general
pytest -s -o log_cli=true -m tier2 --jira tests/storage/general/test_storage_behavior.py
# Registry import test sharing refactored conftest
pytest -s -o log_cli=true tests/storage/cdi_import/test_import_registry.py::test_importer_pod_annotation
\`\`\`"
gh api \
--method POST \
repos/RedHatQE/openshift-virtualization-tests/pulls/5202/reviews \
-f commit_id="$HEAD_SHA" \
-f body="" \
-f event="COMMENT" \
-f "comments[][path]=tests/conftest.py" \
-F "comments[][position]=5" \
-f "comments[][body]=$BODY"Length of output: 3547 |
10 participants
What this PR does / why we need it:
Refactor tests in test_import_http.py by replacing the http import with data sources cloning when possible, and move tests to a more suitable folder (general). Along with these main changes, we are also improving some assertion messages and moving fixtures and helpers to follow already existing test structure.
Which issue(s) this PR fixes:
CNV-86954
Special notes for reviewer:
This is a manual backport of #4646
jira-ticket:
https://redhat.atlassian.net/browse/CNV-86954