feat: run integration tests on more platforms#760
Open
Molter73 wants to merge 6 commits into
Open
Conversation
📝 WalkthroughWalkthroughThis PR adds RHEL and RHEL-ARM64 platforms to the integration test matrix, configures Podman/Docker runtimes via Ansible group variables, restructures test execution to clone and run tests within the target VM using pytest and gRPC stubs, and updates test logic for cross-version compatibility. Multi-Platform Integration Testing
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Molter73
force-pushed
the
mauro/tests/more-platforms
branch
from
0900afe to
0e9261a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #760 +/- ##
=======================================
Coverage 27.96% 27.96%
=======================================
Files 21 21
Lines 2596 2596
Branches 2596 2596
=======================================
Hits 726 726
Misses 1867 1867
Partials 3 3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Molter73
force-pushed
the
mauro/tests/more-platforms
branch
10 times, most recently
from
9a0d020 to
33ec932
Compare
Add support for running integration tests on Python 3.9 through 3.14+ with minimal changes using modern Python features. Changes: - event.py: Add __future__ annotations import to enable modern type hint syntax (int | None) on Python 3.9, with fallback for @OverRide decorator (Python 3.12+) - test_config_hotreload.py: Handle both builtin TimeoutError (Python 3.11+) and concurrent.futures.TimeoutError (Python 3.9-3.10) - test_path_rmdir.py: Reverse event order for Python < 3.10 due to different shutil.rmtree deletion ordering This avoids using deprecated Union/Optional types while maintaining backward compatibility. Tested: 102 tests pass on Python 3.9.25 Assisted-by: Claude Code (claude-sonnet-4-5@20250929)
Molter73
force-pushed
the
mauro/tests/more-platforms
branch
from
b997b40 to
3ec1fe4
Compare
This fixes a connectivity issue with RHEL 10 VMs that only happens on CI.
Molter73
force-pushed
the
mauro/tests/more-platforms
branch
from
3ec1fe4 to
f6346aa
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/integration-tests.yml (1)
77-94:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSerialize the Quay credentials instead of embedding them raw in YAML.
Lines 83-85 write the secret values straight into
vars.yml. A valid username/password containing:,#,{},*, or a newline can make the file unparsable or change the parsed scalar, which breaks the later-e@vars.yml`` calls before the playbooks run. Build this file with a serializer instead (for example, emit JSON with Python and pass that file to-e @...) or robustly escape the values before writing 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 @.github/workflows/integration-tests.yml around lines 77 - 94, Secrets are being written raw into vars.yml which can break YAML parsing for special characters; instead serialize credentials safely (e.g., emit JSON) and consume that file. Replace the here-doc that writes vars.yml with a step that generates a serialized file (JSON or safely escaped YAML) using a small Python step that reads JOB_ID, FACT_IMAGE_NAME, FACT_VERSION and the GitHub secrets (secrets.QUAY_RHACS_ENG_RO_USERNAME / secrets.QUAY_RHACS_ENG_RO_PASSWORD) and writes a vars.json, then update downstream usage from -e `@vars.yml` to -e `@vars.json` (or keep `@vars.yml` if you serialize valid YAML); ensure the producer uses json.dump (or a proper YAML serializer) so characters like :, #, { } and newlines are escaped correctly.
🤖 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.
Inline comments:
In @.github/workflows/integration-tests.yml:
- Around line 123-127: The for-loop "for file in logs/*.tar.gz; do" will run
once with the literal pattern if no files exist; fix by either enabling bash
nullglob before the loop (shopt -s nullglob; ...; shopt -u nullglob) so the
pattern expands to an empty list, or guard the loop with a compgen check
(compgen -G 'logs/*.tar.gz' >/dev/null || true && for file in logs/*.tar.gz; do
...) so the loop only runs when matches exist; apply the change around the
existing cd "/tmp/fact/tests" and the tar xzf/rm sequence to avoid tar trying to
extract "logs/*.tar.gz".
In `@ansible/group_vars/platform_rhcos.yml`:
- Line 3: The platform RHCOS/RHEL group vars override the socket but not the
runtime command, so tasks still take the Docker path; update
ansible/group_vars/platform_rhcos.yml,
ansible/group_vars/platform_rhcos_arm64.yml,
ansible/group_vars/platform_rhel.yml, and
ansible/group_vars/platform_rhel_arm64.yml to also set runtime_command: podman
(or alternatively add that override into a shared Podman-specific group file
used by those platforms) so run-tests.yml and the Podman auth-copy task detect
and use podman instead of docker.
In `@ansible/run-tests.yml`:
- Around line 78-93: The fetch/archive tasks ("Retrieve results", "Compress log
files", "Fetch log files") assume artifacts always exist; add an
ansible.builtin.stat for /tmp/fact/tests/results.xml and for
/tmp/fact/tests/logs before these tasks, then make each task conditional (when:
stat_result.stat.exists) so they only run if present; for fetch tasks set
fail_on_missing: false (and keep validate_checksum: false) so missing artifacts
don’t mask the real failure from the earlier shell block.
In `@tests/test_path_rmdir.py`:
- Around line 158-161: The test currently uses pytest.mark.skipif to skip the
test on Python < 3.10, which contradicts the PR description claiming the test
reverses event order for those versions; update the implementation to match the
description by removing the skip and adding conditional logic in
tests/test_path_rmdir.py (around the pytest.mark.skipif and the test_path_rmdir
test) to reverse the expected shutil.rmtree event order when sys.version_info <
(3, 10), or alternatively change the PR description and test comment to
explicitly state that the test is intentionally skipped on Python < 3.10; choose
one approach and make the code and description consistent.
---
Outside diff comments:
In @.github/workflows/integration-tests.yml:
- Around line 77-94: Secrets are being written raw into vars.yml which can break
YAML parsing for special characters; instead serialize credentials safely (e.g.,
emit JSON) and consume that file. Replace the here-doc that writes vars.yml with
a step that generates a serialized file (JSON or safely escaped YAML) using a
small Python step that reads JOB_ID, FACT_IMAGE_NAME, FACT_VERSION and the
GitHub secrets (secrets.QUAY_RHACS_ENG_RO_USERNAME /
secrets.QUAY_RHACS_ENG_RO_PASSWORD) and writes a vars.json, then update
downstream usage from -e `@vars.yml` to -e `@vars.json` (or keep `@vars.yml` if you
serialize valid YAML); ensure the producer uses json.dump (or a proper YAML
serializer) so characters like :, #, { } and newlines are escaped correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Enterprise
Run ID: 6b5840b5-b53d-44ae-9379-c321d345e575
📒 Files selected for processing (13)
.github/workflows/integration-tests.ymlCHANGELOG.mdansible/ansible.cfgansible/group_vars/all.ymlansible/group_vars/platform_rhcos.ymlansible/group_vars/platform_rhcos_arm64.ymlansible/group_vars/platform_rhel.ymlansible/group_vars/platform_rhel_arm64.ymlansible/run-tests.ymltests/event.pytests/test_config_hotreload.pytests/test_path_rename.pytests/test_path_rmdir.py
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
2 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.
Description
Add support for running integration tests on Python 3.9 through 3.14+ with minimal changes using modern Python features.
Changes:
test_rmdir_recursivefor Python < 3.10 due to different and inconsistent shutil.rmtree deletion orderingThis avoids using deprecated Union/Optional types while maintaining backward compatibility.
Assisted-by: Claude Code (claude-sonnet-4-5@20250929)
Checklist
Automated testing
If any of these don't apply, please comment below.
Testing Performed
Tested: 102 tests pass on Python 3.9.25
Summary by CodeRabbit
New Features
Tests
Chores