WIP: [WIP][Observability] Mark arm64 tests #1001
Conversation
WalkthroughThe changes add the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (24)
🚧 Files skipped from review as they are similar to previous changes (17)
⏰ Context from checks skipped due to timeout of 90000ms (6)
🔇 Additional comments (11)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Report bugs in Issues The following are automatically added:
Available user actions:
PR will be approved when the following conditions are met:
Approvers and Reviewers
Supported /retest check runs
Supported labels
|
ghost
requested a review
from ![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/observability/metrics/test_metrics.py (1)
23-23: Consider documenting ARM64 test strategy at module levelThe file already has module-level pytest markers. Consider adding a comment explaining the ARM64 test strategy - specifically that Linux tests are marked for ARM64 while Windows tests are excluded because they're not supported on ARM64 in version 4.19.
pytestmark = [pytest.mark.post_upgrade, pytest.mark.sno] +# Note: ARM64 markers are applied to specific test classes. +# Windows VM tests are intentionally not marked as ARM64 as they are not supported on ARM64 in v4.19.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
tests/observability/metrics/test_cdi_metrics.py(1 hunks)tests/observability/metrics/test_cnao_metrics.py(1 hunks)tests/observability/metrics/test_cpu_usage_metrics.py(1 hunks)tests/observability/metrics/test_general_metrics.py(3 hunks)tests/observability/metrics/test_instance_type_metrics.py(1 hunks)tests/observability/metrics/test_key_metrics.py(1 hunks)tests/observability/metrics/test_metrics.py(5 hunks)tests/observability/metrics/test_metrics_cpu.py(1 hunks)tests/observability/metrics/test_migration_metrics.py(1 hunks)tests/observability/metrics/test_network_metrics.py(1 hunks)tests/observability/metrics/test_prometheus_service_monitor.py(1 hunks)tests/observability/metrics/test_recording_rules.py(1 hunks)tests/observability/metrics/test_ssp_metrics.py(1 hunks)tests/observability/metrics/test_total_created_vms.py(1 hunks)tests/observability/metrics/test_version_metrics.py(1 hunks)tests/observability/metrics/test_virt_resource_mutation_metrics.py(1 hunks)tests/observability/metrics/test_vm_template_metrics.py(1 hunks)tests/observability/metrics/test_vms_metrics.py(1 hunks)tests/observability/network/test_cnao_observability.py(1 hunks)tests/observability/runbook_url/test_runbook_url.py(1 hunks)tests/observability/storage/test_hpp_observability.py(1 hunks)tests/observability/test_healthy_cluster_no_alerts.py(1 hunks)tests/observability/virt/test_virt_controller_ready.py(1 hunks)tests/observability/virt/test_virt_metrics_alerts.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: build-container
- GitHub Check: tox
- GitHub Check: can-be-merged
🔇 Additional comments (32)
tests/observability/test_healthy_cluster_no_alerts.py (1)
6-6: Apply ARM64 pytest marker at module level
This correctly tags all tests in this module for the ARM64 architecture without altering test logic.tests/observability/metrics/test_vm_template_metrics.py (1)
7-7: Add ARM64 marker alongside existing SNO marker
The module-levelpytestmarknow includes bothsnoandarm64, ensuring tests run under both environments as intended.tests/observability/runbook_url/test_runbook_url.py (1)
11-11: Module-level ARM64 pytest marker applied
Correctly addspytestmark = pytest.mark.arm64to tag all runbook URL tests for ARM64.tests/observability/virt/test_virt_controller_ready.py (1)
6-6: ARM64 pytest marker at module level
This line ensures all tests in this module are filtered for the ARM64 architecture.tests/observability/metrics/test_recording_rules.py (1)
6-6: LGTM: ARM64 marker added correctlyThe
pytest.mark.arm64marker has been added to the existing list of module-level markers, which aligns with the PR objective of marking tests related to the arm64 architecture.tests/observability/virt/test_virt_metrics_alerts.py (1)
18-18: LGTM: ARM64 marker added correctlyThe
pytest.mark.arm64marker has been properly added to the existing pytestmark list.tests/observability/storage/test_hpp_observability.py (1)
13-16: LGTM: ARM64 marker added correctlyThe
pytest.mark.arm64marker has been properly added to the module-level markers list. Formatting with one marker per line is maintained consistently.tests/observability/metrics/test_cnao_metrics.py (1)
9-10: LGTM: ARM64 marker added correctlyThe
pytest.mark.arm64marker has been added as a module-level marker, which is consistent with the PR's objective.tests/observability/metrics/test_total_created_vms.py (1)
7-8: Adding arm64 marker aligns with PR objectiveAdding the
arm64marker to this module allows for selective running/filtering of these tests when testing ARM64 architecture, consistent with the PR's purpose.tests/observability/metrics/test_key_metrics.py (1)
12-12: Correctly added arm64 marker to existing markers listGood job maintaining the existing markers (
post_upgradeandsno) while adding thearm64marker to the list. This preserves the original test categorization while adding the new ARM64 architecture designation.tests/observability/metrics/test_prometheus_service_monitor.py (1)
6-7: Adding arm64 marker aligns with PR objectiveThe added
arm64marker correctly applies to all tests in this module, maintaining consistency with other files in the observability metrics suite.tests/observability/metrics/test_network_metrics.py (1)
8-9: Adding arm64 marker aligns with PR objectiveThe added
arm64marker correctly applies to all tests in this network metrics module, consistent with the pattern applied across the observability test suite.tests/observability/metrics/test_version_metrics.py (2)
9-9: Module-level marker applied correctly.
Addingpytestmark = pytest.mark.arm64at the top of the file cleanly tags all tests in this module for ARM64 environments.
9-9:✅ Verification successful
Ensure
arm64marker is registered.
The newpytestmark = pytest.mark.arm64will trigger a PytestUnknownMarkWarning unlessarm64is declared in yourpytest.ini(undermarkers) or in a project-levelconftest.py.Run:
🏁 Script executed:
pytest --markers | grep -E "^arm64"Length of output: 134
🏁 Script executed:
#!/bin/bash set -e # Search for pytest.ini at the root echo "Looking for pytest.ini..." find . -maxdepth 2 -type f -name pytest.ini # If pytest.ini exists, inspect markers and arm64 entries if [ -f pytest.ini ]; then echo "Contents of pytest.ini around markers section:" grep -nE '^\[pytest\]' -n pytest.ini || true grep -nE '^markers' -n pytest.ini || true grep -nR 'arm64' -n pytest.ini || true fi # Search for any conftest.py in the repository echo "Locating conftest.py files..." find . -type f -name conftest.py # Inspect conftest.py files for marker registration echo "Searching for pytest markers registration in conftest.py..." grep -R -nE 'pytest_configure|markers' --include='conftest.py' . grep -R -n 'arm64' --include='conftest.py' .Length of output: 5649
No action needed:
arm64marker is already registered
The project’spytest.iniincludes under[pytest]→markers:markers = arm64: tests that require ARM-based cluster supportedTherefore using
pytest.mark.arm64will not trigger an unknown‐mark warning.tests/observability/metrics/test_cpu_usage_metrics.py (1)
19-19: Class-level ARM64 marker usage.
AnnotatingTestCpuUsageMetricswith@pytest.mark.arm64correctly scopes the ARM64 tag to all tests in this class.tests/observability/metrics/test_metrics_cpu.py (1)
9-9: Extend module markers witharm64.
Updating the globalpytestmarklist to[pytest.mark.post_upgrade, pytest.mark.sno, pytest.mark.arm64]aligns this file’s markers with the rest of the observability suite.tests/observability/metrics/test_migration_metrics.py (1)
25-25: Module-level ARM64 marker addition.
Applyingpytestmark = pytest.mark.arm64at module scope correctly tags all migration metrics tests for ARM64 targets.tests/observability/metrics/test_vms_metrics.py (1)
47-47: Module-level ARM64 marker.
Usingpytestmark = pytest.mark.arm64here ensures consistent ARM64 tagging across all tests in this file.tests/observability/metrics/test_virt_resource_mutation_metrics.py (1)
29-29: LGTM: Added arm64 marker to existing pytestmark listThe change appropriately adds the
pytest.mark.arm64marker alongside the existingpytest.mark.snomarker, following the PR objective to mark tests for ARM64 architecture.tests/observability/metrics/test_instance_type_metrics.py (1)
25-26: LGTM: Added arm64 marker at module levelThe change appropriately adds a module-level
pytest.mark.arm64marker, aligning with the PR objective to mark tests for ARM64 architecture.tests/observability/metrics/test_ssp_metrics.py (1)
18-19: LGTM: Added arm64 marker at module levelThe change appropriately adds a module-level
pytest.mark.arm64marker, aligning with the PR objective to mark tests for ARM64 architecture.tests/observability/metrics/test_cdi_metrics.py (1)
7-8: LGTM: Added arm64 marker at module levelThe change appropriately adds a module-level
pytest.mark.arm64marker in a list format, aligning with the PR objective to mark tests for ARM64 architecture.tests/observability/metrics/test_general_metrics.py (4)
48-49: LGTM: Added arm64 marker to TestVmiNodeCpuAffinityLinux classThe change appropriately adds the
@pytest.mark.arm64decorator to theTestVmiNodeCpuAffinityLinuxclass, aligning with the PR objective to mark tests for ARM64 architecture.
87-88: LGTM: Added arm64 marker to TestVmNameInLabel classThe change appropriately adds the
@pytest.mark.arm64decorator to theTestVmNameInLabelclass, aligning with the PR objective to mark tests for ARM64 architecture.
105-106: LGTM: Added arm64 marker to TestVirtHCOSingleStackIpv6 classThe change appropriately adds the
@pytest.mark.arm64decorator to theTestVirtHCOSingleStackIpv6class, aligning with the PR objective to mark tests for ARM64 architecture.
1-115: Consider marking TestVmiNodeCpuAffinityWindows class for consistencyI notice that the
TestVmiNodeCpuAffinityWindowsclass (lines 78-85) isn't marked withpytest.mark.arm64while all other test classes in this file are. Given the PR description that "arm Windows virtual machines are not supported in version 4.19", this might be intentional, but it's worth confirming.tests/observability/metrics/test_metrics.py (6)
146-147: Adding ARM64 marker to Linux VM tests looks goodThis correctly adds the ARM64 marker to Linux VM metrics tests, allowing them to be specifically targeted or filtered during test runs.
189-190: Windows VM tests correctly excluded from ARM64 markingI see that you're intentionally not marking the Windows VM tests with the ARM64 marker, which aligns with your PR description that "arm Windows virtual machines are not supported in version 4.19".
235-236: ARM64 marker application to memory tests looks goodCorrectly marking the memory delta tests for ARM64.
282-283: ARM64 marker application to daemonset tests looks goodThe ARM64 marker is appropriately applied to the TestKubeDaemonsetStatusNumberReady class.
293-294: ARM64 marker application to API metrics tests looks goodCorrectly marking the API request metrics tests for ARM64.
304-305: ARM64 marker application to allocatable nodes tests looks goodThe ARM64 marker is appropriately applied to the TestAllocatableNodes class.
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-1001 published |
|
/retest build-container |
|
/build-and-push-container |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-1001 published |
rnetser
added
tox:verify-tc-requirement-polarion:running
|
/wip |
ghost
changed the title
Signed-off-by: Harel Meir <[email protected]>
ghost
added
tox:verify-tc-requirement-polarion:running
|
/build-and-push-container |
ghost
added
tox:verify-bugs-are-open:running
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-1001 published |
ghost
added
tox:verify-bugs-are-open:passed
Short description:
Mark Arm64 tests.
More details:
arm windows VM is not supported in 4.19.
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
Summary by CodeRabbit