- Issue created by @codebymikey
- 🇪🇸Spain fjgarlin
I would just go as far as step 1. Do we really need step 2?
It has a default value, so if somebody wants to alter it, the whole variable can be overwriten. This is something that you would only change for some particular tests.
I was weighing the need for it, the main advantage of the additional variable is that it makes it a bit easier to add specific patterns to your project/test while also leveraging the default global pattern set on the template.
Personally would find it easier to simply add a dedicated
stage_file_proxy\.services\.yml
value to the pipeline variable, and have it be a bit more obvious as to why its being included rather than copying and updating the default variables from upstream.However I don't have a strong opinion either way as its an edge case variable and will be an improvement over the current behaviour.
- 🇪🇸Spain fjgarlin
Yeah, I can see how that second variable will make things more readable and it'd have the added benefit that if we make changes upstream, you'd still get them.
Note that the same changes would be needed for
test-only-d7.sh
(we know D7 is EOL but if the changes are easy to port we are trying our best).Would you be willing to produce an MR for this?
I wasn't aware that you could actually modify the manual pipeline variables before running them, so initially worked on supporting test-only changes via the web pipeline.
This can be tested on the https://git.drupalcode.org/issue/stage_file_proxy-3521812/-/pipelines/new issue fork.
Test case 1: Merge request
Previous behaviour: Tests can't be ran
New behaviour: Test runs and fails gracefully.Test case 2: Manual pipeline with no tests
Branch: 3.1.x
Variables:_GITLAB_TEMPLATES_REPO: issue/gitlab_templates-3539542 _GITLAB_TEMPLATES_REF: 3539542-test-only-patterns _TEST_ONLY_FILE_PATTERN: stage_file_proxy\.services\.yml CI_DEBUG_TRACE: true
Result - doesn't create the "Test-only changes" job, because there are no test file changes.
Test case 2: Manual pipeline with tests
Branch: 3521812-test-web-pipeline
Variables:_GITLAB_TEMPLATES_REPO: issue/gitlab_templates-3539542 _GITLAB_TEMPLATES_REF: 3539542-test-only-patterns _TEST_ONLY_FILE_PATTERN: stage_file_proxy\.services\.yml CI_DEBUG_TRACE: true
Result - creates the "Test-only changes" job since there are actual test file changes before the current branch and the default one.
On a sidenote, even though the
3521812-test-web-pipeline
branch has the_GITLAB_TEMPLATES_REPO
and_GITLAB_TEMPLATES_REF
variables overridden and hardcoded in the.gitlab-ci.yml
, new manual pipelines which don't explicitly specify the override end up using:_GITLAB_TEMPLATES_REPO: project/gitlab_templates _GITLAB_TEMPLATES_REF: default-ref
- Merge request !388Issue #3539542: Allow users to add extra files to the Test-only changes job → (Merged) created by codebymikey
- 🇪🇸Spain fjgarlin
Yeah, sometimes variables inheritance / precedence gets in the way with those global variables.
https://project.pages.drupalcode.org/gitlab_templates/info/testing-mrs/Thanks for the test links and the different cases. I'm having a look at the code.
- 🇪🇸Spain fjgarlin
The code looks great and the new additions are really useful in the context of the test-only job.
I've raised one minor thing about the new documentation as the image will not render as it is right now. - 🇪🇸Spain fjgarlin
I think this looks good. There was additional testing on #8.
It keeps what we have and only adds things on top (like triggering via web UI and adding the extra pattern), so I think it's also low risk
RTBC. -
fjgarlin →
committed 4dc6e19e on main authored by
codebymikey →
Issue #3539542 by codebymikey, fjgarlin: Users should be able to update...
-
fjgarlin →
committed 4dc6e19e on main authored by
codebymikey →
- 🇪🇸Spain fjgarlin
Included in the latest tag and made available for all contrib too as part of the default ref.
- 🇬🇧United Kingdom jonathan1055
This is an excellent additional feature, thanks codebymikey. I was wondering if we might be able to make it even better, as I don't see any reason to restrict the new "compare to another branch, when not in a MR" feature to only work if the pipeline was submitted from the web UI
$CI_PIPELINE_SOURCE == "web"
. If other pipeline_source values were allowed, or maybe we just detect if$_TEST_ONLY_TARGET_BRANCH
has a different value from$CI_DEFAULT_BRANCH'
to be the criteria for the job rule. This would allow the 'test only' job to be run in pipeines on non-default branches, when triggered by other means, which would allow us to work on the Gitlab Templates DownStream issue ✨ Add coverage for test-only changes job Active