virt_tests: extract to standalone virt-framework repository - #1005
virt_tests: extract to standalone virt-framework repository#1005trgill wants to merge 4 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe local Changesvirt-framework extraction
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 9✅ Passed checks (9 passed)
✨ 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 |
|
@coderabbitai review This is not ready for merge. Just testing withe the gh workflow. |
✅ Action performedReview finished.
|
|
✅ Action performedReview finished.
|
|
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/snapm.yml (1)
219-224: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove or update the
fedora44matrix entry.virt-framework’s README only listsfedora42,fedora43,centos-stream9, andcentos-stream10, so this job will likely fail on thefedora44lane until support is added.🤖 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/snapm.yml around lines 219 - 224, Update the matrix configuration used by the virt-framework invocation to remove the unsupported fedora44 entry, or replace it with a currently supported image such as fedora42, fedora43, centos-stream9, or centos-stream10. Keep the existing "$BASE_OS" wiring and other workflow arguments unchanged.
🤖 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/snapm.yml:
- Around line 148-151: Harden the workflow steps installing virt-framework and
checking out the repository: replace the mutable main reference with an
immutable commit or release, pin actions/checkout to its full commit SHA, and
set persist-credentials to false unless the workflow requires the checkout
token. Apply these changes to the relevant virt-framework installation steps,
including the one near line 219.
In `@virt_tests/README.md`:
- Line 10: Update the installation command in the README to reference an
immutable virt-framework commit SHA or release tag instead of the moving default
branch. Keep the existing pip install format and repository target unchanged.
---
Outside diff comments:
In @.github/workflows/snapm.yml:
- Around line 219-224: Update the matrix configuration used by the
virt-framework invocation to remove the unsupported fedora44 entry, or replace
it with a currently supported image such as fedora42, fedora43, centos-stream9,
or centos-stream10. Keep the existing "$BASE_OS" wiring and other workflow
arguments unchanged.
🪄 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.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d31398a1-a6ad-45fc-8c82-050d9d287b7a
📒 Files selected for processing (9)
.github/workflows/snapm.ymlCONTRIBUTING.mdsetup.cfgvirt_tests/README.mdvirt_tests/__init__.pyvirt_tests/__main__.pyvirt_tests/strategy.pyvirt_tests/testvm.pyvirt_tests/util.py
💤 Files with no reviewable changes (5)
- setup.cfg
- virt_tests/main.py
- virt_tests/util.py
- virt_tests/strategy.py
- virt_tests/testvm.py
| - name: Install virt-framework | ||
| run: | | ||
| set -euo pipefail | ||
| sudo pip install git+https://github.com/trgill/virt-framework.git@main |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".github/workflows/snapm.yml"
echo "== Line count =="
wc -l "$file"
echo
echo "== Relevant excerpt =="
nl -ba "$file" | sed -n '120,240p'
echo
echo "== Search for checkout / permissions / persist-credentials =="
rg -n "actions/checkout|permissions:|persist-credentials|fedora44|virt-framework|sudo pip install|sudo " "$file"Repository: snapshotmanager/snapm
Length of output: 273
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".github/workflows/snapm.yml"
echo "== Relevant excerpt with line numbers =="
awk 'NR>=120 && NR<=240 { printf "%4d %s\n", NR, $0 }' "$file"
echo
echo "== Checkout / permissions / persist-credentials / matrix markers =="
rg -n "actions/checkout|permissions:|persist-credentials|fedora44|virt-framework|sudo pip install|sudo " "$file"Repository: snapshotmanager/snapm
Length of output: 10160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".github/workflows/snapm.yml"
echo "== Top of workflow =="
awk 'NR>=1 && NR<=40 { printf "%4d %s\n", NR, $0 }' "$file"
echo
echo "== Checkout config details =="
rg -n -C 2 "uses: actions/checkout|persist-credentials|permissions:" "$file"Repository: snapshotmanager/snapm
Length of output: 1747
Don’t run mutable upstream code as root
Lines 148-151 and 219: virt-framework comes from mutable main and is executed with sudo, so upstream changes run as root on the runner. Pin it to an immutable commit/release, and pin actions/checkout to a full SHA; set persist-credentials: false unless the token is needed.
🤖 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/snapm.yml around lines 148 - 151, Harden the workflow
steps installing virt-framework and checking out the repository: replace the
mutable main reference with an immutable commit or release, pin actions/checkout
to its full commit SHA, and set persist-credentials to false unless the workflow
requires the checkout token. Apply these changes to the relevant virt-framework
installation steps, including the one near line 219.
Source: Path instructions
| This copy will be removed in a future release. Use the standalone | ||
| package instead: | ||
|
|
||
| pip install git+https://github.com/trgill/virt-framework.git |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'README excerpt:\n'
nl -ba virt_tests/README.md | sed -n '1,25p'
printf '\nSearch for other install references:\n'
rg -n "git\+https://github.com/trgill/virt-framework\.git|virt-framework" -S virt_tests/README.md . || trueRepository: snapshotmanager/snapm
Length of output: 215
🌐 Web query:
trgill/virt-framework GitHub releases tags
💡 Result:
There is no public GitHub repository found under the name "trgill/virt-framework". As of July 20, 2026, searches for this specific path yield no results on GitHub [1-10]. It is possible the repository name is incorrect, private, or has been deleted. If you are looking for virtualization-related frameworks on GitHub, several projects with similar names or purposes exist, such as: - vandycknick/virt-fwk: Rust bindings for Apple's Virtualization Framework [1]. - vortemain/vorte-framework: An AI-first Python API framework [2]. - projectbeskar/virtrigaud: A storage-agnostic cross-hypervisor migration tool [3][4]. - kubevirt/kubevirt: A virtualization API for Kubernetes [5]. Please verify the repository owner or name.
Citations:
- 1: https://github.com/vandycknick/virt-fwk
- 2: https://github.com/vortemain/vorte-framework/releases/tag/v1.0.8
- 3: https://github.com/projectbeskar/virtrigaud/releases
- 4: https://github.com/projectbeskar/virtrigaud/releases/tag/v0.3.11
- 5: https://github.com/kubevirt/kubevirt/releases
Pin the install target. pip install git+https://github.com/trgill/virt-framework.git pulls the repository’s moving default branch, so users can get different code over time. Use an immutable commit SHA or release tag instead.
🤖 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 `@virt_tests/README.md` at line 10, Update the installation command in the
README to reference an immutable virt-framework commit SHA or release tag
instead of the moving default branch. Keep the existing pip install format and
repository target unchanged.
Source: Path instructions
Replace virt-install auto-discovery (--install <os>) with explicit --location URLs pointing to official CDN endpoints. This avoids random mirror selection that can cause network flakiness in CI, particularly on Fedora. Also add inst.repo= parameter to ensure package downloads use the same reliable CDN endpoints. Addresses: snapshotmanager#914 Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Todd Gill <tgill@redhat.com>
Replace the embedded virt_tests module with the standalone virt-framework package installed from GitHub. Changes: - Add step to pip install virt-framework from trgill/virt-framework - Replace 'python3 -m virt_tests' with 'virt-framework' command The virt-framework repository will be moved from trgill/ to snapshotmanager/ and the install URL updated accordingly. Related: snapshotmanager#941 Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Todd Gill <tgill@redhat.com>
The virt_tests module has been extracted to a standalone repository at https://github.com/trgill/virt-framework and will be removed from this repo in a future release. Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Todd Gill <tgill@redhat.com>
ca29e22 to
c2b0e8d
Compare
|
I've had a quick look at the new repo, and there is a lot there that I like: I think this is a good direction for the VM tests to head in, and I think it will enable us to land other useful features in future (e.g.: converting the test strategy from code to data). I haven't done a thorough review of the code yet (there's a lot of net-new code in the refactored
Lastly, and I get this is a big ask that adds extra work, but I think it would be easier to review & reason about these changes if we made the changes to the in-tree copy first, and then separate out the extraction to a separate repo as a separate PR once all those changes have been merged. There are some |
|
(*) e.g. this is a really common triple quoted anti-pattern I see in Python code: some_thing = f"""
foo bar baz quux
lorem ipsum dolor sit amet
"""The user might expect This is really common and can lead to funky/unexpected formatting when these strings are output or written to a file ( |
c2b0e8d to
3c65ac9
Compare
Remove the virt_tests source code that has been extracted to the standalone virt-framework repository. The CI workflow now installs the external package via pip. The deprecation README is retained to direct users to the new repository at https://github.com/trgill/virt-framework. Changes: - Remove __init__.py, __main__.py, strategy.py, testvm.py, util.py - Remove virt_tests exclude from setup.cfg (no longer needed) - Update CONTRIBUTING.md to remove virt_tests reference Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Todd Gill <tgill@redhat.com>
3c65ac9 to
9d61c93
Compare
2 participants
Extract the virt_tests module to a standalone pip-installable package
(virt-framework) so it can be reused by other projects in the snapm
ecosystem (e.g. boom-boot).
The standalone repository is at https://github.com/trgill/virt-framework
and will be moved to https://github.com/snapshotmanager/virt-framework.
Changes:
virt_testsis so flakey on Fedora #914)Testing: CI workflow runs the external package against the full
matrix (firmware x storage x OS) to validate the integration.
Addresses: #914
Summary by CodeRabbit
New Features
Documentation
Refactor