iface_update: Support new libvirt error message for boot order conflicts#6801
Open
BulaYoungR wants to merge 1 commit intoautotest:masterfrom
Open
iface_update: Support new libvirt error message for boot order conflicts#6801BulaYoungR wants to merge 1 commit intoautotest:masterfrom
BulaYoungR wants to merge 1 commit intoautotest:masterfrom
Conversation
WalkthroughReplaces per-attribute interface update code in libvirt/tests/src/virtual_network/iface_update.py with a generalized two-dictionary approach: Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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)
No actionable comments were generated in the recent review. 🎉 Comment |
Author
|
Before fix After fix |
![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
🤖 Fix all issues with AI agents
In `@libvirt/tests/src/virtual_network/iface_update.py`:
- Around line 267-282: The code calls re.search(expect_err_msg, real_err_msg)
without guarding for expect_err_msg being None (params.get() may return None),
causing a TypeError; update the logic in the block that computes match_found
(around the variables expect_err_msg, real_err_msg, alternative_patterns) to
first check that expect_err_msg is a non-empty string (or compiled pattern)
before invoking re.search, e.g., only perform the initial re.search when
expect_err_msg is truthy and otherwise skip to trying alternative_patterns;
ensure match_found is a boolean after these checks so the existing test.fail
path remains valid.
Comment on lines
+267
to
282
| # Support both old and new libvirt error messages for boot order conflicts | ||
| # Old: "per-device boot elements cannot be used together with os/boot elements" | ||
| # New: "boot order X is already used by another device" | ||
| alternative_patterns = [ | ||
| "boot order .* is already used by another device", | ||
| "per-device boot elements cannot be used together with os/boot elements" | ||
| ] | ||
| match_found = re.search(expect_err_msg, real_err_msg, re.IGNORECASE) | ||
| if not match_found: | ||
| # Try alternative patterns for known cases where error message changed | ||
| match_found = any(re.search(pattern, real_err_msg, re.IGNORECASE) | ||
| for pattern in alternative_patterns) | ||
|
|
||
| if not match_found: | ||
| test.fail("The real error msg:'%s' does not match expect one:" | ||
| '%s' % (real_err_msg, expect_err_msg)) |
There was a problem hiding this comment.
Handle potential None value for expect_err_msg before regex matching.
If expect_err_msg is not set in test params (line 121 uses params.get() which returns None), re.search(expect_err_msg, ...) at line 274 will raise TypeError: first argument must be string or compiled pattern.
The fallback logic for supporting both old and new libvirt error messages is well-designed for cross-version compatibility.
🛡️ Proposed fix to guard against None
alternative_patterns = [
"boot order .* is already used by another device",
"per-device boot elements cannot be used together with os/boot elements"
]
- match_found = re.search(expect_err_msg, real_err_msg, re.IGNORECASE)
+ match_found = False
+ if expect_err_msg:
+ match_found = re.search(expect_err_msg, real_err_msg, re.IGNORECASE)
if not match_found:
# Try alternative patterns for known cases where error message changed
match_found = any(re.search(pattern, real_err_msg, re.IGNORECASE)
for pattern in alternative_patterns)🤖 Prompt for AI Agents
In `@libvirt/tests/src/virtual_network/iface_update.py` around lines 267 - 282,
The code calls re.search(expect_err_msg, real_err_msg) without guarding for
expect_err_msg being None (params.get() may return None), causing a TypeError;
update the logic in the block that computes match_found (around the variables
expect_err_msg, real_err_msg, alternative_patterns) to first check that
expect_err_msg is a non-empty string (or compiled pattern) before invoking
re.search, e.g., only perform the initial re.search when expect_err_msg is
truthy and otherwise skip to trying alternative_patterns; ensure match_found is
a boolean after these checks so the existing test.fail path remains valid.
Fix negative test failure caused by libvirt error message format change
in newer versions.
The test checks that libvirt correctly rejects setting boot order on a
network interface when os/boot is already configured. However, libvirt
changed the error message format between versions.
Changed file: libvirt/tests/src/virtual_network/iface_update.py
Changed lines: 283-302
Old libvirt error message:
"per-device boot elements cannot be used together with os/boot elements"
New libvirt error message:
"boot order 1 is already used by another device"
The test was checking only for the old message format, causing it to fail
with newer libvirt versions even though the actual error was correct.
Fix: Modified error checking logic to accept both error message formats:
- First check the expected message from test config (expect_err_msg)
- If not matched, try alternative known patterns for boot order conflicts
- This ensures compatibility with both old and new libvirt versions
The solution uses fallback patterns instead of updating test config files,
making it work across different libvirt versions without needing separate
test configurations.
Committer: Bolatbek Issakh <bissakh@redhat.com>
BulaYoungR
force-pushed
the
iface_update
branch
from
408ae8c to
a6353fd
Compare
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
1 participant
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.
Fix negative test failure caused by libvirt error message format change in newer versions.
The test checks that libvirt correctly rejects setting boot order on a network interface when os/boot is already configured. However, libvirt changed the error message format between versions.
Changed file: libvirt/tests/src/virtual_network/iface_update.py
Changed lines: 283-302
Old libvirt error message:
"per-device boot elements cannot be used together with os/boot elements"
New libvirt error message:
"boot order 1 is already used by another device"
The test was checking only for the old message format, causing it to fail with newer libvirt versions even though the actual error was correct.
Fix: Modified error checking logic to accept both error message formats:
The solution uses fallback patterns instead of updating test config files, making it work across different libvirt versions without needing separate test configurations.
Committer: Bolatbek Issakh bissakh@redhat.com
Summary by CodeRabbit
Bug Fixes
Refactor
Other