Skip to content

Conversation

@smitterl
Copy link
Contributor

@smitterl smitterl commented Dec 5, 2025

None of the tests applies to s390x: no UEFI, no NVRAM, no TPM.

Summary by CodeRabbit

  • Chores
    • Updated migration test configurations to refine architecture-specific exclusions for improved test coverage and reliability.

✏️ Tip: You can customize this high-level summary in your review settings.

None of the tests applies to s390x: no UEFI, no NVRAM, no TPM.

Signed-off-by: Sebastian Mitterle <[email protected]>
@smitterl smitterl requested review from cliping and dzhengfy December 5, 2025 08:55
@coderabbitai
Copy link

coderabbitai bot commented Dec 5, 2025

Walkthrough

The changes modify test configuration exclusions in the migration shared-filesystems test file. A new "no s390-virtio" exclusion was added under the migrate_vm_back section, while s390-virtio was simultaneously removed from the boot-type bios variant exclusion list, which now contains only "no aarch64". These adjustments alter which architecture variants are tested in specific migration scenarios.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Configuration file modifications with straightforward exclusion list changes
  • Changes involve moving/reordering architecture-specific exclusions across test variants
  • Verify that the moved exclusion (s390-virtio removal from bios variant) aligns with intended test coverage for both the migrate_vm_back and boot-type sections

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 accurately and concisely describes the main objective of the pull request: disabling the migration/canonical_paths tests on s390x architecture.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 f5225cd and 2087597.

📒 Files selected for processing (1)
  • libvirt/tests/cfg/migration/migration_misc/canonical_paths_in_shared_filesystems.cfg (2 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.9
  • GitHub Check: Python 3.8
  • GitHub Check: Python 3.12
  • GitHub Check: Python 3.11
🔇 Additional comments (2)
libvirt/tests/cfg/migration/migration_misc/canonical_paths_in_shared_filesystems.cfg (2)

49-50: Top-level no s390-virtio correctly encodes the s390x disablement intent

Adding the no s390-virtio at the test level cleanly disables all variants of this canonical_paths test on s390x, which matches the PR description (no UEFI/NVRAM/TPM there). This is a good central place to express the architecture-wide exclusion, rather than per-variant.


58-61: Removing s390-virtio from the BIOS variant’s no list is consistent cleanup

Given the new top-level no s390-virtio, dropping it from the BIOS boot_type no list just removes redundancy while keeping the existing semantics for non-s390x (still excluding aarch64 only). This change looks correct assuming s390-virtio is the only s390x env used for this test matrix.


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

Copy link
Contributor

@dzhengfy dzhengfy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dzhengfy dzhengfy removed the request for review from cliping December 16, 2025 13:11
@dzhengfy dzhengfy merged commit 3758214 into autotest:master Dec 16, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants