{CI} Fix ignore logic #31784
{CI} Fix ignore logic #31784
Conversation
wangzelin007
requested review from
bebound,
calvinhzy,
jiasli and
kairu-ms
as code owners
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
microsoft-github-policy-service
bot
requested review from
Pan-Qi,
jsntcy and
yonzhan
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the extension ignore logic in the CI script by adding spaces around extension names to ensure exact matches rather than partial ones.
- Wraps both the ignore list and current extension in spaces when using
=~ - Prevents substrings from incorrectly triggering the ignore condition
scripts/ci/test_extensions.sh
Outdated
| echo | ||
| # Use regex to detect if $ext is in $ignore_list | ||
| if [[ $ignore_list =~ $ext ]]; then | ||
| if [[ " $ignore_list " =~ " $ext " ]]; then |
There was a problem hiding this comment.
Quoting the regex pattern on the right-hand side disables regex interpretation and may not produce the intended match. Consider removing the quotes around the pattern and using a bracket expression for spaces, e.g.: if [[ " $ignore_list " =~ [[:space:]]$ext[[:space:]] ]]; then.
| if [[ " $ignore_list " =~ " $ext " ]]; then | |
| if [[ " $ignore_list " =~ [[:space:]]$ext[[:space:]] ]]; then |
There was a problem hiding this comment.
@wangzelin007, is the copilot suggestion reasonable?
There was a problem hiding this comment.
This line is indeed doing literal match, so it is ok to quote it.
There was a problem hiding this comment.
Yes, as jiashuo said, this is intentional.
scripts/ci/test_extensions.sh
Outdated
| echo | ||
| # Use regex to detect if $ext is in $ignore_list | ||
| if [[ $ignore_list =~ $ext ]]; then | ||
| if [[ " $ignore_list " =~ " $ext " ]]; then |
There was a problem hiding this comment.
[nitpick] For clearer literal matching and to avoid regex pitfalls, you could use a case statement instead, e.g.: case " $ignore_list " in *" $ext "*) echo \"Ignore extension: $ext\"; continue;; esac.
scripts/ci/test_extensions.sh
Outdated
| echo | ||
| # Use regex to detect if $ext is in $ignore_list | ||
| if [[ $ignore_list =~ $ext ]]; then | ||
| if [[ " $ignore_list " =~ " $ext " ]]; then |
There was a problem hiding this comment.
Surrounding the string with spaces to achieve full match is a hack. Consider using ^ and $:
https://www.gnu.org/savannah-checkouts/gnu/bash/manual/bash.html#index-_005b_005b
The match succeeds if the pattern matches any part of the string. If you want to force the pattern to match the entire string, anchor the pattern using the ‘^’ and ‘$’ regular expression operators.
There was a problem hiding this comment.
I would prefer to use a for loop, which is clearer and more intuitive.
5 participants
Currently, load extension task skips ml test:
Add spaces around extension names for exact matching
Related command
Description
Testing Guide
History Notes
[Component Name 1] BREAKING CHANGE:
az command a: Make some customer-facing breaking change[Component Name 2]
az command b: Add some customer-facing featureThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.