Software Test Description (STD) template and examples#2881
Software Test Description (STD) template and examples#2881rnetser merged 34 commits intoRedHatQE:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds STD (Test Descriptions as Code) documentation, a new CLI utility that AST-parses Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/SOFTWARE_TEST_DESCRIPTION.md (3)
93-101: Add a language to this fenced code block for markdownlintThe “Expected:” example block currently uses a bare triple-backtick fence; adding a language (e.g.
text) will satisfy MD040 and improve rendering consistency.-**Example:** -``` +**Example:** +```text Expected: - VM is Running - File content equals "data-before-snapshot" - File /data/after.txt does NOT exist - Ping fails with 100% packet loss -``` +```
242-246: Standardize on “Parametrize” spelling to avoid mixed variantsThe table row for Matrix Testing uses “Parameterize tests…” while the rest of the document (headings and sections) uses “Parametrize”. For consistency with pytest terminology and to address the EN_WORD_COHERENCY hint, consider using a single variant.
-| **Matrix Testing** | Parameterize tests for multiple scenarios | `storage_class_matrix`, `run_strategy_matrix` | +| **Matrix Testing** | Parametrize tests for multiple scenarios | `storage_class_matrix`, `run_strategy_matrix` |
442-447: Replace the hard tab in the Example 5 markers section with spacesIn the Example 5 code snippet, the
- gatingline is indented with a tab, which triggers MD010; switching to spaces keeps indentation consistent across the doc.- Markers: - - gating + Markers: + - gating
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/SOFTWARE_TEST_DESCRIPTION.md(1 hunks)tests/network/flat_overlay/test_flat_overlay.py(7 hunks)tests/storage/online_resize/test_online_resize.py(2 hunks)tests/virt/cluster/vm_lifecycle/test_restart.py(2 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 2469
File: utilities/sanity.py:139-142
Timestamp: 2025-11-08T07:36:57.616Z
Learning: In the openshift-virtualization-tests repository, user rnetser prefers to keep refactoring PRs (like PR #2469) strictly focused on moving/organizing code into more granular modules without adding new functionality, error handling, or behavioral changes. Such improvements should be handled in separate PRs.
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 1244
File: utilities/os_utils.py:248-250
Timestamp: 2025-07-08T05:51:06.314Z
Learning: User rnetser prefers fail-fast code design where functions should raise exceptions immediately when encountering unexpected or malformed input, rather than implementing defensive error handling with fallbacks. They value making problems visible immediately during development over graceful degradation.
📚 Learning: 2025-09-29T19:05:24.987Z
Learnt from: rnetser
Repo: RedHatQE/openshift-virtualization-tests PR: 0
File: :0-0
Timestamp: 2025-09-29T19:05:24.987Z
Learning: For PR #1904 test execution, the critical validation point is test_connectivity_over_migration_between_localnet_vms which should fail gracefully on cloud clusters but pass on bare-metal/PSI clusters, representing the core nmstate conditional logic functionality.
Applied to files:
tests/network/flat_overlay/test_flat_overlay.py
📚 Learning: 2025-08-28T12:30:40.692Z
Learnt from: servolkov
Repo: RedHatQE/openshift-virtualization-tests PR: 1776
File: tests/network/bgp/conftest.py:35-54
Timestamp: 2025-08-28T12:30:40.692Z
Learning: The BGP test suite in tests/network/bgp/ relies on session-scoped validation in tests/network/conftest.py via the network_sanity fixture's _verify_bgp() function, which validates required environment variables (VLAN_TAG, EXTERNAL_FRR_STATIC_IPV4, BGP_CLUSTER_DOMAIN_GROUP) before any BGP tests run, making individual fixture-level validation redundant.
Applied to files:
tests/network/flat_overlay/test_flat_overlay.py
📚 Learning: 2025-06-23T19:24:28.327Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1210
File: tests/virt/cluster/general/mass_machine_type_transition_tests/test_mass_machine_type_transition.py:97-104
Timestamp: 2025-06-23T19:24:28.327Z
Learning: In OpenShift Virtualization machine type transition tests, the test_machine_type_transition_without_restart method with restart_required=false parameter validates that VM machine types do NOT change when the lifecycle job runs with restart disabled, so the assertion should check against the original machine type rather than the target machine type.
Applied to files:
tests/virt/cluster/vm_lifecycle/test_restart.py
📚 Learning: 2025-08-14T10:28:22.958Z
Learnt from: vsibirsk
Repo: RedHatQE/openshift-virtualization-tests PR: 1785
File: tests/virt/cluster/common_templates/utils.py:65-66
Timestamp: 2025-08-14T10:28:22.958Z
Learning: The restart_qemu_guest_agent_service function in tests/virt/cluster/common_templates/utils.py is only called for RHEL7 VMs, with OS version checks handled by the calling test code, not within the function itself. Guest agent functionality is verified by subsequent tests in the test class.
Applied to files:
tests/virt/cluster/vm_lifecycle/test_restart.py
📚 Learning: 2025-06-03T14:35:13.475Z
Learnt from: akri3i
Repo: RedHatQE/openshift-virtualization-tests PR: 1105
File: tests/virt/upgrade/utils.py:230-258
Timestamp: 2025-06-03T14:35:13.475Z
Learning: In the openshift-virtualization-tests repository, docstrings are not required for wrapper functions when parameter names are self-explanatory and the underlying class (like VirtualMachineForTestsFromTemplate) already has comprehensive documentation.
Applied to files:
tests/virt/cluster/vm_lifecycle/test_restart.py
📚 Learning: 2025-09-03T07:23:37.045Z
Learnt from: RoniKishner
Repo: RedHatQE/openshift-virtualization-tests PR: 1946
File: tests/infrastructure/instance_types/supported_os/test_windows_os.py:30-31
Timestamp: 2025-09-03T07:23:37.045Z
Learning: In tests/infrastructure/instance_types/supported_os/test_windows_os.py, test_vm_deletion should depend on test_create_vm (not test_start_vm) to ensure resource cleanup happens even if the VM fails to start, preventing resource leaks in the test environment.
Applied to files:
tests/virt/cluster/vm_lifecycle/test_restart.py
📚 Learning: 2025-06-18T09:15:25.436Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1178-1180
Timestamp: 2025-06-18T09:15:25.436Z
Learning: In tests/observability/metrics/conftest.py, the `stopped_vm_metric_1` fixture is intentionally designed to stop the VM and leave it in that state - it does not need to restart the VM afterward as this is the desired behavior for the tests that use it.
Applied to files:
tests/virt/cluster/vm_lifecycle/test_restart.py
📚 Learning: 2025-06-22T13:47:35.014Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1183-1186
Timestamp: 2025-06-22T13:47:35.014Z
Learning: In tests/observability/metrics/conftest.py, the `stopped_windows_vm` fixture is designed to temporarily stop the Windows VM for a test, then restart it during teardown (after yield) because the Windows VM is module-scoped and needs to be available for other tests that depend on it being in a running state.
Applied to files:
tests/virt/cluster/vm_lifecycle/test_restart.py
📚 Learning: 2025-06-18T09:21:34.315Z
Learnt from: OhadRevah
Repo: RedHatQE/openshift-virtualization-tests PR: 1166
File: tests/observability/metrics/conftest.py:1065-1077
Timestamp: 2025-06-18T09:21:34.315Z
Learning: In tests/observability/metrics/conftest.py, when creating fixtures that modify shared Windows VM state (like changing nodeSelector), prefer using function scope rather than class scope to ensure ResourceEditor context managers properly restore the VM state after each test, maintaining test isolation while still reusing expensive Windows VM fixtures.
Applied to files:
tests/virt/cluster/vm_lifecycle/test_restart.py
📚 Learning: 2025-10-30T10:43:48.886Z
Learnt from: josemacassan
Repo: RedHatQE/openshift-virtualization-tests PR: 2300
File: tests/storage/online_resize/conftest.py:94-96
Timestamp: 2025-10-30T10:43:48.886Z
Learning: In tests/storage/online_resize/conftest.py, the `running_rhel_vm` fixture is intentionally designed to only invoke `running_vm(vm=rhel_vm_for_online_resize)` without returning or yielding a value. It's used as a dependency fixture to ensure the VM is running before other fixtures/tests execute, while the actual VM object is accessed via the `rhel_vm_for_online_resize` fixture.
Applied to files:
tests/storage/online_resize/test_online_resize.py
🧬 Code graph analysis (1)
tests/network/flat_overlay/test_flat_overlay.py (2)
tests/network/flat_overlay/conftest.py (5)
flat_overlay_vma_vmb_nad(85-93)vma_flat_overlay(135-148)vmb_flat_overlay(152-165)vmd_flat_overlay(185-198)vmc_flat_overlay_ip_address(258-259)utilities/network.py (1)
assert_ping_successful(754-775)
🪛 LanguageTool
docs/SOFTWARE_TEST_DESCRIPTION.md
[uncategorized] ~245-~245: Do not mix variants of the same word (‘parameterize’ and ‘parametrize’) within a single text.
Context: ... | | Matrix Testing | Parameterize tests for multiple scenarios | `storage...
(EN_WORD_COHERENCY)
🪛 markdownlint-cli2 (0.18.1)
docs/SOFTWARE_TEST_DESCRIPTION.md
94-94: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
443-443: Hard tabs
Column: 5
(MD010, no-hard-tabs)
🔇 Additional comments (4)
tests/network/flat_overlay/test_flat_overlay.py (1)
1-3: STD-aligned docstrings for flat overlay tests look consistentModule, class, and test-level docstrings correctly capture markers, preconditions, numbered steps, and Expected outcomes that match the implemented pings/negative checks; this is a solid application of the new STD style with no behavior changes.
Also applies to: 24-35, 43-56, 71-85, 101-113, 127-141, 160-174, 182-189, 200-212
tests/virt/cluster/vm_lifecycle/test_restart.py (1)
2-2: Restart lifecycle test documentation matches behaviorThe updated module header and the detailed docstring for
test_vm_restartaccurately describe the restart/stop/start flow and final SSH connectivity check, aligned with the existing implementation.Also applies to: 36-52
tests/storage/online_resize/test_online_resize.py (1)
4-4: Sequential online resize test is well-described and in sync with codeThe module summary and
test_sequential_disk_expanddocstring clearly state preconditions, a single repeated step, and the expected outcome of six successful resizes, all matching the existing test logic.Also applies to: 45-61
docs/SOFTWARE_TEST_DESCRIPTION.md (1)
1-461: STD design doc is clear and matches how tests are being documentedThe two-phase workflow, templates, and examples (including negative and parametrized tests) align well with the updated test docstrings in this PR and give reviewers a concrete contract (Preconditions/Steps/Expected) to work from, without imposing behavior changes. Suggestions above are limited to small lint/style fixes to keep this PR focused on documentation.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1625 🏁 Script executed: Length of output: 2326 🏁 Script executed: Length of output: 272 🏁 Script executed: Length of output: 1735 🧠 Learnings used |
- Remove redundant int() cast on floor division result - Add comment explaining intentional SyntaxError catch skip - Use bool flag for standalone function header tracking - Add descriptive failure messages to test assertions - Add ClassVar annotation to mutable class attribute - Replace manual logging handler with pytest caplog fixture
|
/verified the flow was tested as part of https://github.com/RedHatQE/openshift-virtualization-tests/tree/main/tests/network/user_defined_network/ip_specification |
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:
Your deliverable: Test Execution Plan
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: Before setting "Run smoke tests: True", you MUST use shell scripts to: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 4880 🏁 Script executed: Length of output: 1735 🧠 Learnings used |
docs/SOFTWARE_TEST_DESCRIPTION.md
Outdated
| | Pattern | Description | Example | | ||
| |--------------------------|-------------------------------------------|-----------------------------------------------| | ||
| | **Fixture-based Setup** | Use pytest fixtures for resource creation | `vm_to_restart`, `namespace` | | ||
| | **Matrix Testing** | Parameterize tests for multiple scenarios | `storage_class_matrix`, `run_strategy_matrix` | |
There was a problem hiding this comment.
I support parameterization at the test or fixture level with or without content (of the parameters) inputed through helper functions. However, I am not ok with magic usage of dynamic fixtures, which include embedding the parameters in the fake/stub fixture naming.
This kind of pattern does not fit a large project like this, where maintenance is a big deal and magic results in more mistakes than advanatges.
From my perspective, this is a python and pytest abuse and I do not want to see any of these in sig-network tests.
I suggest to soften the recommendation and name it per the classic form with an example for extreme cases when one wants to do something special.
Pattern:
Matrix Testing =>> Parameterize Testing
Example:
Give only one of these matrix ones and mention it is for advance usages.
Provide the basic one which uses plain parameterize in test or fixtures.
|
/verified the flow was tested as part of https://github.com/RedHatQE/openshift-virtualization-tests/tree/main/tests/network/user_defined_network/ip_specification |
|
/approve |
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:
Your deliverable: Test Execution Plan
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: Before setting "Run smoke tests: True", you MUST use shell scripts to: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1610 🏁 Script executed: Length of output: 1735 🧠 Learnings used |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
Short description:
Introduces Software Test Description (STD) documentation standards and a script to track STD adoption across the test suite.
More details:
Establishes a structured format for test documentation using STD docstrings
(Preconditions/Steps/Expected format) to improve test clarity and maintainability.
What this PR does / why we need it:
1. STD Documentation (
docs/SOFTWARE_TEST_DESCRIPTION.md)2. Placeholder Detection Script (
scripts/std_placeholder_stats/)3. CLAUDE.md Update
__test__ = Falsepattern used in STD placeholder testsBenefits:
Special notes for reviewer:
Assisted-by: Claude
jira-ticket:
Summary by CodeRabbit
Documentation
New Features
Chores
Tests