Skip to content

Updated regex for slot_ibm to include optional -R\d+ segment.#6246

Open
maramsmurthy wants to merge 4 commits intoavocado-framework:masterfrom
maramsmurthy:smaram_physical_port_regex_update
Open

Updated regex for slot_ibm to include optional -R\d+ segment.#6246
maramsmurthy wants to merge 4 commits intoavocado-framework:masterfrom
maramsmurthy:smaram_physical_port_regex_update

Conversation

@maramsmurthy
Copy link
Contributor

@maramsmurthy maramsmurthy commented Nov 27, 2025

This allows matching both standard and extended slot formats: Standard: U50EE.001.WZS00TL-P3-C14
Extended: U50EE.001.WZS00TL-P3-C14-R1
Change ensures compatibility with newer hardware naming conventions that include resource identifiers after the slot number. Backward Compatibility:

Existing slot formats without -R remain fully supported. No impact on current parsing logic for standard slot names. Testing Impact:

Verified regex against both formats to ensure correct matching. Additional unit tests recommended for extended slot names to prevent regressions.

Summary by CodeRabbit

  • Bug Fixes
    • Improved PCI slot detection to support additional IBM hardware device code format variations with optional suffixes, enhancing compatibility with a broader range of systems.

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

This allows matching both standard and extended slot formats:
Standard: U50EE.001.WZS00TL-P3-C14
Extended: U50EE.001.WZS00TL-P3-C14-R1
Change ensures compatibility with newer hardware naming conventions
that include resource identifiers after the slot number.
Backward Compatibility:

Existing slot formats without -R remain fully supported.
No impact on current parsing logic for standard slot names.
Testing Impact:

Verified regex against both formats to ensure correct matching.
Additional unit tests recommended for extended slot names to prevent
regressions.

Signed-off-by: Maram Srimannarayana Murthy <msmurthy@linux.vnet.ibm.com>
@coderabbitai
Copy link

coderabbitai bot commented Nov 27, 2025

Walkthrough

A regex pattern in the get_slot_from_sysfs function of avocado/utils/pci.py was modified to extend IBM loc-code slot pattern matching. The pattern was updated to optionally accept a trailing "-R" suffix by adding a non-capturing optional group (?:-R\d+)? to the existing pattern. The modification preserves all existing matches while enabling matching of IBM loc-code strings with the additional suffix. The remainder of the control flow logic remains unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5-10 minutes

  • Verify the regex pattern correctly matches both with and without the "-R" suffix
  • Confirm backward compatibility with existing IBM loc-code strings that lack the suffix
  • Review the optional group syntax (?:-R\d+)? to ensure it captures the intended pattern

Pre-merge checks

✅ 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 describes the main change: a regex update to accept an optional -R\d+ segment in the IBM slot pattern.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 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 74b7379 and 9cc6d14.

📒 Files selected for processing (1)
  • avocado/utils/pci.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). (43)
  • GitHub Check: rpm-build:fedora-41-ppc64le
  • GitHub Check: rpm-build:fedora-41-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:fedora-41-s390x
  • GitHub Check: rpm-build:fedora-41-aarch64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:fedora-41-aarch64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: rpm-build:fedora-41-ppc64le
  • GitHub Check: rpm-build:fedora-41-s390x
  • GitHub Check: rpm-build:fedora-41-x86_64
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: Linux with Python 3.13
  • GitHub Check: Linux with Python 3.9
  • GitHub Check: Linux with Python 3.11
  • GitHub Check: Linux with Python 3.10
  • GitHub Check: Linux with Python 3.12
  • GitHub Check: rpm-build:fedora-41-aarch64
  • GitHub Check: rpm-build:centos-stream-9-x86_64
  • GitHub Check: rpm-build:fedora-42-x86_64
  • GitHub Check: rpm-build:epel-9-x86_64
  • GitHub Check: rpm-build:fedora-41-s390x
  • GitHub Check: rpm-build:fedora-41-x86_64
  • GitHub Check: rpm-build:fedora-43-x86_64
  • GitHub Check: rpm-build:fedora-rawhide-x86_64
  • GitHub Check: rpm-build:fedora-41-ppc64le
  • GitHub Check: Build Package (wheel/tarball) for Python 3.10
  • GitHub Check: Fedora selftests
  • GitHub Check: Version task ubi:8.8
  • GitHub Check: macOS with Python 3.11
  • GitHub Check: Windows with Python 3.13
  • GitHub Check: Version task debian:12.4
  • GitHub Check: Windows with Python 3.11
  • GitHub Check: Windows with Python 3.10
  • GitHub Check: Version task ubuntu:22.04
  • GitHub Check: Static checks
  • GitHub Check: Code Coverage (3.11)
🔇 Additional comments (1)
avocado/utils/pci.py (1)

203-205: Regex extension looks correct and backward‑compatible

The added optional non‑capturing group (?:-R\d+)? cleanly extends the IBM loc‑code pattern to cover the -R<digits> suffix while preserving all existing matches and group numbering (since it’s non‑capturing). Given the complexity of this regex and its importance, please ensure we also add/extend unit tests to cover examples like U50EE.001.WZS00TL-P3-C14-R1 (and still cover the legacy ...-P3-C14 case) so regressions are caught going forward.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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.

❤️ Share

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

@mr-avocado mr-avocado bot moved this to Review Requested in Default project Nov 27, 2025
@codecov
Copy link

codecov bot commented Nov 27, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.70%. Comparing base (74b7379) to head (32943e3).
⚠️ Report is 63 commits behind head on master.

Files with missing lines Patch % Lines
avocado/utils/pci.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6246      +/-   ##
==========================================
+ Coverage   73.48%   73.70%   +0.21%     
==========================================
  Files         206      206              
  Lines       22494    22620     +126     
==========================================
+ Hits        16530    16671     +141     
+ Misses       5964     5949      -15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@clebergnu clebergnu requested a review from richtja December 4, 2025 13:53
Copy link
Collaborator

@PraveenPenguin PraveenPenguin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@clebergnu clebergnu left a comment

Choose a reason for hiding this comment

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

@maramsmurthy thanks for this contribution. Could you please:

  1. Put an (external) reference to the format of the extended version of the identifiers
  2. Add a test that matches both old and new (extended) versions

Thanks!

@maramsmurthy
Copy link
Contributor Author

@clebergnu Following is the output for both the formats of. physical slot

from avocado.utils import pci
pci.get_slot_from_sysfs("0153:70:00.0")
'U78CD.001.FZHAK92-P2-C3'
pci.get_slot_from_sysfs("05b4:90:00.0")
'U50EE.001.WZS0011-P3-C20-R1'

Update worked successfully without any issues

U78CC.001.FZHAK92-P2-C3 which is exisitng format
U50EE.001.WZS0011-P3-C20-R1 which is new format

We are maitatining both the formats as both formats are required.
Both are returning expected output without any issues.

Signed-off-by: Maram Srimannarayana Murthy <msmurthy@linux.vnet.ibm.com>
@maramsmurthy maramsmurthy force-pushed the smaram_physical_port_regex_update branch from 0c3f452 to 32943e3 Compare March 3, 2026 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Review Requested

Development

Successfully merging this pull request may close these issues.

3 participants