Fix CI rules-if comparison with Regexp variables
What does this MR do and why?
When the right side of a =~
or !~
comparison is a regexp variable, we consider this as a string. However, we need to try converting it to a regexp if possible.
In this MR, we start to try to consider the right side of an expression as a regexp.
Related to #35438 (closed)
This fix is behind a feature flag ci_fix_rules_if_comparison_with_regexp_variable
(#359740 (closed)).
Screenshots or screen recordings
- Example CI pipeline config:
variables:
teststring: 'abcde'
pattern: '/^ab.*/'
test1:
script: exit 0
rules:
- if: '$teststring =~ $pattern'
test2:
script: exit 0
rules:
- if: '$teststring =~ /^ab.*/'
- Run a pipeline;
test1
is not included in the pipeline.
- Enable
ci_fix_rules_if_comparison_with_regexp_variable
and run a pipeline;
Feature.enable(:ci_fix_rules_if_comparison_with_regexp_variable)
test1
is included in the pipeline.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Merge request reports
Activity
changed milestone to %15.0
assigned to @furkanayhan
Suggested Reviewers (beta)
This is an experimental ML-based code reviewer recommendation system created by ~"group::applied ml".
The individuals below may be good candidates to participate in the review based on various factors.
After you review all recommendations, please assign reviewers manually, as this is not done automatically.
You can use slash commands in comments to quickly assign
/assign_reviewer @user1
.Reviewers @marcel.amirault
,@axil
,@eread
,@cnorris
,@sselhorn
If you do not believe these recommendations are useful or if you do not want to use any of the suggestions, please apply the label Bad Suggested Reviewer. You can also provide feedback for this feature on this issue:
https://gitlab.com/gitlab-org/gitlab/-/issues/357923
.Automatically generated by Suggested Reviewers Bot
Edited by GitLab Reviewer-Recommender Botmentioned in issue #359740 (closed)
2 Warnings ⚠ e9b42a6e: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. ⚠ 184da569: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. 2 Messages 📖 CHANGELOG missing: If you want to create a changelog entry for GitLab FOSS, add the
Changelog
trailer to the commit message you want to add to the changelog.If you want to create a changelog entry for GitLab EE, also add the
EE: true
trailer to your commit message.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
📖 This merge request adds or changes documentation files. A review from the Technical Writing team before you merge is recommended. Reviews can happen after you merge. If needed, you can retry the
🔁 danger-review
job that generated this comment.Documentation review
The following files require a review from a technical writer:
doc/ci/yaml/index.md
The review does not need to block merging this merge request. See the:
-
Metadata for the
*.md
files that you've changed. The first few lines of each*.md
file identify the stage and group most closely associated with your docs change. - The Technical Writer assigned for that stage and group.
- Documentation workflows for information on when to assign a merge request for review.
Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer backend Sam Figueroa ( @sam.figueroa
) (UTC+2, 1 hour behind@furkanayhan
)Charlie Ablett ( @cablett
) (UTC+12, 9 hours ahead of@furkanayhan
)To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
Generated by
🚫 Dangermarked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed
- Resolved by Furkan Ayhan
@marcel.amirault Do you think we can write something to our documentation regarding this change?
requested review from @marcel.amirault
added 1 commit
- 5e24752a - Fix CI rules-if comparison with Regexp variables
- Resolved by Furkan Ayhan
- Resolved by Furkan Ayhan
Allure report
allure-report-publisher
generated test report!package-and-qa-ff-enabled:
📝 test report
package-and-qa-ff-disabled:📝 test reportpackage-and-qa-ff-disabled:
❗ test report for e9b42a6eexpand test summary
+-------------------------------------------------------------------+ | suites summary | +----------------------+--------+--------+---------+-------+--------+ | | passed | failed | skipped | flaky | result | +----------------------+--------+--------+---------+-------+--------+ | Manage | 91 | 0 | 6 | 0 | ✅ | | Create | 148 | 0 | 7 | 11 | ❗ | | Verify | 33 | 0 | 8 | 5 | ❗ | | Plan | 53 | 0 | 0 | 2 | ❗ | | Secure | 15 | 0 | 6 | 7 | ❗ | | Package | 0 | 0 | 3 | 0 | ➖ | | Version sanity check | 0 | 0 | 1 | 0 | ➖ | | Release | 6 | 0 | 0 | 2 | ❗ | | Fulfillment | 2 | 0 | 10 | 0 | ✅ | | Configure | 0 | 0 | 3 | 0 | ➖ | | Protect | 2 | 0 | 0 | 0 | ✅ | | Non-devops | 2 | 0 | 0 | 0 | ✅ | +----------------------+--------+--------+---------+-------+--------+ | Total | 352 | 0 | 44 | 27 | ❗ | +----------------------+--------+--------+---------+-------+--------+
❗ test report for e9b42a6eexpand test summary
+-------------------------------------------------------------------+ | suites summary | +----------------------+--------+--------+---------+-------+--------+ | | passed | failed | skipped | flaky | result | +----------------------+--------+--------+---------+-------+--------+ | Create | 148 | 0 | 7 | 11 | ❗ | | Release | 6 | 0 | 0 | 2 | ❗ | | Configure | 0 | 0 | 3 | 0 | ➖ | | Non-devops | 2 | 0 | 0 | 0 | ✅ | | Verify | 33 | 0 | 8 | 5 | ❗ | | Plan | 53 | 0 | 0 | 2 | ❗ | | Package | 0 | 0 | 3 | 0 | ➖ | | Manage | 91 | 0 | 6 | 0 | ✅ | | Fulfillment | 2 | 0 | 10 | 0 | ✅ | | Secure | 15 | 0 | 6 | 7 | ❗ | | Version sanity check | 0 | 0 | 1 | 0 | ➖ | | Protect | 2 | 0 | 0 | 0 | ✅ | +----------------------+--------+--------+---------+-------+--------+ | Total | 352 | 0 | 44 | 27 | ❗ | +----------------------+--------+--------+---------+-------+--------+
❗ test report for e9b42a6eexpand test summary
+-------------------------------------------------------------------+ | suites summary | +----------------------+--------+--------+---------+-------+--------+ | | passed | failed | skipped | flaky | result | +----------------------+--------+--------+---------+-------+--------+ | Manage | 31 | 0 | 2 | 29 | ❗ | | Create | 24 | 0 | 2 | 22 | ❗ | | Protect | 2 | 0 | 0 | 2 | ❗ | | Plan | 41 | 0 | 1 | 41 | ❗ | | Package | 0 | 0 | 1 | 0 | ➖ | | Verify | 12 | 0 | 1 | 12 | ❗ | | Configure | 0 | 0 | 1 | 0 | ➖ | | Version sanity check | 0 | 0 | 1 | 0 | ➖ | +----------------------+--------+--------+---------+-------+--------+ | Total | 110 | 0 | 9 | 106 | ❗ | +----------------------+--------+--------+---------+-------+--------+
removed review request for @marcel.amirault
added 505 commits
-
5e24752a...c83404e8 - 503 commits from branch
master
- e214334f - Fix CI rules-if comparison with Regexp variables
- 24dc96da - Add the documentation
-
5e24752a...c83404e8 - 503 commits from branch
requested review from @marcel.amirault
- Resolved by Furkan Ayhan
@matteeyah As you already know the context from the issue, can you please do the first review?
(I want to assign it to
@grzesiek
for the final review)
requested review from @matteeyah
- Resolved by Furkan Ayhan
added docsfeature label
removed review request for @marcel.amirault
👋 @marcel.amirault
, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.
For more info, please refer to the following links:
requested review from @grzesiek
removed review request for @grzesiek and @matteeyah
requested review from @grzesiek
- Resolved by Furkan Ayhan
- Resolved by Furkan Ayhan
- Resolved by Furkan Ayhan
- Resolved by Grzegorz Bizon
- Resolved by Furkan Ayhan
@furkanayhan Nice work! A few questions.
- Resolved by Furkan Ayhan
removed review request for @grzesiek
requested review from @grzesiek
- Resolved by Furkan Ayhan
Thanks @furkanayhan, I responded to your questions!
🏓
removed review request for @grzesiek
requested review from @grzesiek
- The
gitlab-qa-mirror
downstream pipeline for !85310 (dc53dc7a) passed.✅ - The
gitlab-qa-mirror
downstream pipeline for !85310 (dc53dc7a) failed!💥 - The
gitlab-qa-mirror
downstream pipeline for !85310 (e9b42a6e) passed.✅ - The
gitlab-qa-mirror
downstream pipeline for !85310 (e9b42a6e) passed.✅
- The
mentioned in issue #35438 (closed)
- Resolved by Grzegorz Bizon
- Resolved by Furkan Ayhan
- Resolved by Furkan Ayhan
- Resolved by Furkan Ayhan
Seems to be almost ready! A few minor questions @furkanayhan.
removed review request for @grzesiek