Skip to content

Conversation

@smitterl
Copy link
Contributor

@smitterl smitterl commented Oct 14, 2025

Depending on the libvirt version some substrings might be present in the error message or not, such as 'internal error'.

Instead of adding another possible pattern, reduce the regex to match to two with that contain the essential message: the command name and the reason.

Summary by CodeRabbit

  • Tests
    • Refined error pattern matching for guest agent commands during filesystem freeze/thaw scenarios.
    • Reduces spurious test failures when the agent is disabled or in a frozen state, improving test reliability without changing runtime behavior.

Depending on the libvirt version some substrings might be present
in the error message or not, such as 'internal error'.

Instead of adding another possible pattern, reduce the regex to
match to two with that contain the essential message: the command
name and the reason.

Signed-off-by: Sebastian Mitterle <[email protected]>
@smitterl
Copy link
Contributor Author

JOB ID     : 7bf8d59b15fa74ce5a8324815fffbc505ff791ce
JOB LOG    : /var/log/avocado/job-results/job-2025-10-14T07.01-7bf8d59/job.log
 (1/1) type_specific.io-github-autotest-libvirt.virsh.domfsfreeze_domfsthaw.positive.normal: STARTED
 (1/1) type_specific.io-github-autotest-libvirt.virsh.domfsfreeze_domfsthaw.positive.normal:  PASS (75.92 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /var/log/avocado/job-results/job-2025-10-14T07.01-7bf8d59/results.html
JOB TIME   : 77.04 s

@coderabbitai
Copy link

coderabbitai bot commented Oct 14, 2025

Walkthrough

The test file virsh_domfsfreeze_domfsthaw.py updates run_agent_command_when_frozen by reducing the set of failure-matching patterns for guest agent command errors, narrowing them to messages beginning with “unable to execute QEMU agent command …”. No other logic or public interfaces were changed.

Changes

Cohort / File(s) Summary
Virsh domfsfreeze/thaw test logic
libvirt/tests/src/virsh_cmd/domain/virsh_domfsfreeze_domfsthaw.py
Reduced fail_patts from three to two patterns, removing matches for “error: guest agent command failed: …” and “error: internal error: unable to execute QEMU agent command …”, keeping only “unable to execute QEMU agent command …” variants; control flow unchanged.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant T as Test (run_agent_command_when_frozen)
  participant GA as Guest Agent
  participant QEMU as QEMU/Monitor

  T->>GA: Execute agent command while FS frozen
  GA-->>QEMU: Request execution
  QEMU-->>GA: Response (success/error)
  GA-->>T: Return message

  rect rgba(230,240,255,0.5)
    note over T: Updated failure matching
    T->>T: Check message against\n"unable to execute QEMU agent command ..." patterns
    alt Matches pattern
      T->>T: Treat as failure
    else Does not match
      T->>T: Not classified as failure here
    end
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my nose at frozen calls,
Trim patterns down in testy halls.
QEMU whispers, agents freeze—
I hop through logs with practiced ease.
Fewer echoes, clearer trail,
Carrot-strong, I will not fail. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly references the virsh_domfsfreeze functionality and accurately describes the core change of relaxing the error message matching logic, directly reflecting the primary update in the test code.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_fstrim_older

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 919ea51 and f7ceff8.

📒 Files selected for processing (1)
  • libvirt/tests/src/virsh_cmd/domain/virsh_domfsfreeze_domfsthaw.py (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Python 3.12
  • GitHub Check: Python 3.11
  • GitHub Check: Python 3.8
  • GitHub Check: Python 3.9
🔇 Additional comments (1)
libvirt/tests/src/virsh_cmd/domain/virsh_domfsfreeze_domfsthaw.py (1)

139-140: LGTM! Error pattern relaxation achieves cross-version compatibility.

The simplified patterns correctly match the essential error message components (command name and failure reason) without requiring version-specific prefixes like "error:" or "internal error:". This aligns well with the PR objective to handle libvirt version variability.

The regex patterns are well-formed:

  • Both patterns consistently start with "unable to execute QEMU agent command"
  • The '\S+' placeholders (with literal single quotes) correctly match QEMU agent command name format
  • Each pattern captures a distinct failure reason (command not allowed vs. agent in frozen state)

The successful test results in the PR comments confirm this approach works in practice.


Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant