[TRTLLM-11861][infra] Support wildcard in bot stage-list/extra-stage commands#12881
[TRTLLM-11861][infra] Support wildcard in bot stage-list/extra-stage commands#12881mzweilz wants to merge 2 commits intoNVIDIA:mainfrom
Conversation
|
/bot run --stage-list "A10-Build_Docs" |
📝 WalkthroughWalkthroughAdded wildcard pattern matching support for stage filtering across bot commands and Jenkins pipelines. Introduced helper functions to evaluate stage names against wildcard patterns (supporting Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
jenkins/L0_Test.groovy (2)
1381-1404: Extract the wildcard matcher into a shared helper.
stageMatchesPatternis now duplicated here and injenkins/L0_MergeRequest.groovy, so the bot can easily drift into validating one wildcard syntax and executing another later. Centralizing this logic, or at least covering both call sites with the same tests, would make the behavior much safer to evolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jenkins/L0_Test.groovy` around lines 1381 - 1404, The wildcard matching logic in stageMatchesPattern (and its caller stageMatchesAnyPattern) is duplicated and should be centralized to avoid drift; extract stageMatchesPattern into a shared helper (e.g., a common utility class/function) and replace local implementations in both locations to call that single helper, update stageMatchesAnyPattern to delegate to the new helper, and add a small unit test covering exact, prefix, suffix, contains and multi-wildcard cases to both call sites to ensure identical behavior.
3824-3835: Enforce a single resolved stage when--debugis enabled.A single wildcard like
*PerfSanity*can now expand to many stages here, but the debug flow later assumes one target container. Without a post-filter check,--debugcan open multiple interactive sessions and effectively wedge the job.Suggested guard
if (testFilter[EXTRA_STAGE_LIST] != null) { echo "Use EXTRA_STAGE_LIST for filtering. Stages: ${testFilter[(EXTRA_STAGE_LIST)]}." parallelJobsFiltered += parallelJobs.findAll { stageMatchesAnyPattern(it.key, testFilter[(EXTRA_STAGE_LIST)]) } println parallelJobsFiltered.keySet() } + + if (testFilter[(DEBUG_MODE)] && parallelJobsFiltered.size() != 1) { + error "--debug requires the stage selection to resolve to exactly one stage. Matched stages: ${parallelJobsFiltered.keySet()}" + } checkStageName(fullSet)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jenkins/L0_Test.groovy` around lines 3824 - 3835, After resolving TEST_STAGE_LIST and EXTRA_STAGE_LIST into parallelJobsFiltered (using stageMatchesAnyPattern), add a guard when running with --debug: if the debug flag is set and parallelJobsFiltered does not contain exactly one entry, fail fast with a clear error/echo and exit (or throw) to avoid opening multiple interactive sessions; reference the symbols TEST_STAGE_LIST, EXTRA_STAGE_LIST, parallelJobsFiltered and stageMatchesAnyPattern and perform this check immediately after the combined filtering block so the debug flow only proceeds when exactly one stage is selected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@jenkins/L0_MergeRequest.groovy`:
- Around line 1316-1320: The Build-Docker-Images check currently treats patterns
as glob-style matches via matchesBuildDocker/ stageMatchesPattern, causing broad
patterns (e.g., *Build*) to incorrectly opt into the top-level docker build;
change matchesBuildDocker to only consider an exact literal match to
"Build-Docker-Images" (e.g., pattern == "Build-Docker-Images") and update the
subsequent removeAll calls to only remove entries that exactly equal
"Build-Docker-Images" (rather than using stageMatchesPattern) so dockerBuildJob
is only included when the user explicitly opted into Build-Docker-Images.
---
Nitpick comments:
In `@jenkins/L0_Test.groovy`:
- Around line 1381-1404: The wildcard matching logic in stageMatchesPattern (and
its caller stageMatchesAnyPattern) is duplicated and should be centralized to
avoid drift; extract stageMatchesPattern into a shared helper (e.g., a common
utility class/function) and replace local implementations in both locations to
call that single helper, update stageMatchesAnyPattern to delegate to the new
helper, and add a small unit test covering exact, prefix, suffix, contains and
multi-wildcard cases to both call sites to ensure identical behavior.
- Around line 3824-3835: After resolving TEST_STAGE_LIST and EXTRA_STAGE_LIST
into parallelJobsFiltered (using stageMatchesAnyPattern), add a guard when
running with --debug: if the debug flag is set and parallelJobsFiltered does not
contain exactly one entry, fail fast with a clear error/echo and exit (or throw)
to avoid opening multiple interactive sessions; reference the symbols
TEST_STAGE_LIST, EXTRA_STAGE_LIST, parallelJobsFiltered and
stageMatchesAnyPattern and perform this check immediately after the combined
filtering block so the debug flow only proceeds when exactly one stage is
selected.
🪄 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: CHILL
Plan: Pro
Run ID: f73283b9-7c14-4e8a-870d-99c152c98464
📒 Files selected for processing (3)
.github/workflows/bot-command.ymljenkins/L0_MergeRequest.groovyjenkins/L0_Test.groovy
|
PR_Github #42488 [ run ] triggered by Bot. Commit: |
504517e to
6bd1dea
Compare
…commands Add wildcard '*' support for --stage-list and --extra-stage bot commands. Users can now specify patterns like "*PerfSanity*" to match all stages containing "PerfSanity" instead of listing each stage individually. Signed-off-by: Abby Wei <mengzew@nvidia.com>
6bd1dea to
9db1163
Compare
|
/bot run --stage-list "A10*" |
|
PR_Github #42505 [ run ] triggered by Bot. Commit: |
|
PR_Github #42488 [ run ] completed with state |
|
/bot run --stage-list "A10*" |
|
PR_Github #42526 [ run ] triggered by Bot. Commit: |
|
PR_Github #42505 [ run ] completed with state |
|
PR_Github #42526 [ run ] completed with state |
|
/bot run |
Replace hand-rolled imperative loop with Groovy regex matching. Uses Pattern.quote() to safely handle special characters in stage names. Behavior is unchanged - same glob semantics with cleaner implementation. Signed-off-by: Abby Wei <mengzew@nvidia.com>
5090d40 to
8723faf
Compare
|
/bot run |
|
PR_Github #42650 [ run ] triggered by Bot. Commit: |
|
PR_Github #42652 [ run ] triggered by Bot. Commit: |
|
PR_Github #42650 [ run ] completed with state |
Summary
*support for--stage-listand--extra-stagebot commands"*PerfSanity*"to match all stages containing "PerfSanity" instead of listing each stage individuallyA10-*), suffix (*-1), contains (*PerfSanity*), and multi-segment (H100*Post-Merge*1) patterns[]in stage names are handled correctlyTest plan
/bot run --stage-list "*PerfSanity*"— verify wildcard substring matching/bot run --stage-list "A10-PyTorch-1, *PerfSanity*"— verify mixed exact + wildcard/bot run --stage-list "A10-PyTorch-1"— verify backward compatibility/bot run --stage-list "*Nonexistent*"— verify error on unmatched patternSummary by CodeRabbit
Release Notes
*pattern matching (e.g.,"*PerfSanity*") for more flexible stage selection.