Tested here and it works - using drupalci/php-7.4-ubuntu-apache:production
https://git.drupalcode.org/project/scheduler/-/jobs/3428789
As per the discussion on #33 in #3486021-33: Unable to run "composer (previous major)" from pipeline UI; no image → I have drafted a CR https://www.drupal.org/node/3488684 →
Thank you, that's really helpful. Sorry I missed it before.
I just discovered that I had one extra commit locally for this MR. It was only to re-write the comments to explain what to do in MR257 on 📌 Update to new driver when D11 "current" uses Nightwatch 3 Postponed
Here's a patch so you can see what I had. Maybe this could be committed? I can open a new MR. Given that the changes may be postponed for a while, and I have already done the thinking about it and written the details, it would be good to have accurate @todo comments in the code ready.
Shall I still create a change record for 🐛 SQLite version too low for Drupal 11 Active given the new issue you created? If that may take some time to get done we can have the change record anyway.
Nice idea! This would really help to reduce support issues and maintain BC in customized templates.
Yes that is a BC break for customized jobs which pin PHP at 8.1
There have been a couple of places recently (either issues or on Slack) where @fjgarlin has explained the design principles of Gitlab Templates, where things will be always moving forward to keep the majority of projects working with the latest core versions, etc, and that this may cause disruption for some custtomized tempates. We always have simple ways to solve the problem, and I think that we should document these on one the pages. Then it makes it easy for maintainers to find the support they need and see examples of the fixes. We can have a table of the version, date and explanation, much like change records.
I can work on that if it is approved as a plan.
Actually it is not this issue which caused your problem, it is an earlier one 🐛 SQLite version too low for Drupal 11 Active
I've tried but I cannot replicate this error. Here are the steps I have followed:
- Install clean Drupal 10.3.8 (or 10.4-dev)
- Login as admin and create a user
- Install Scheduler 8.x-1.5
- Use Scheduler OK
- Update to Scheduler 2.1.0
- Run necessary database updates (8) via admin
- Log out - OK
- Log in as ordinary user - OK
Do you know of anything else which might be interferring with your site?
Just checking, will this not be committed to 11.0.x ?
I made a change to add an example of setting _TARGET_PHP_IMAGE_VARIANT: "apache" on docs/info/common.md#test-different-sqlite-versions
I don't have anything else to add here, so this is ready for review.
If I'm reading the logs correctly, the new selenium service takes two seconds to load. The earliest and latest timestamps, for example, are
[service:selenium/standalone-chrome-selenium] 2024-11-15T16:42:14
[service:selenium/standalone-chrome-selenium] 2024-11-15T16:42:16
The legacy driver is much quicker, mostly loading within one second
[service:drupalci/webdriver-chromedriver-chrome] 2024-11-15T16:42:14.27
[service:drupalci/webdriver-chromedriver-chrome] 2024-11-15T16:42:14.28
The tests already have CI_DEBUG_SERVICES: 1 and there are lots of timestamps in the log. They are not all in chronological order, but I will extract the times and try to see what it shows us, if anything.
It was my error - newly added test files do trigger the 'test-only changes' job, which is good. I had OPT_IN_TEST_CURRENT: 0 which meant that the job was not added, regardless of the changes:
Existing behavior without MR292 on Scheduler, when a nightwatch .js test is added. The 'test-only changes' job is run when it should not.
https://git.drupalcode.org/project/scheduler/-/pipelines/343379
Using MR292 - the 'test-only chnages' job is correctly not added
https://git.drupalcode.org/project/scheduler/-/pipelines/343391
With MR292 and also making a change to a PHPUnit test - the job is added as required
https://git.drupalcode.org/project/scheduler/-/pipelines/343395
I also tested with Decoupled Pages, which has Nightwatch tests in the repo. The MR makes a change to the .js test file
"before" test shows the 'test-only changes' job is incorrectly added to the pipeline
https://git.drupalcode.org/issue/decoupled_pages-3422594/-/pipelines/343168
"after" using MR292 shows that the 'test-only changes' job is not added
https://git.drupalcode.org/issue/decoupled_pages-3422594/-/pipelines/343244
This is ready for review.
When adding a new test as proof that a bug fix has coverage that would not be shown in the normal phpunit tests, becuase the modified/fixed code would also be included. So it would have been helpful if new fles did trigger the job. But as they don't, there not a lot we can do. I will test on decoupled_pages instead.
Tested https://git.drupalcode.org/project/scheduler/-/pipelines/342184
In addition to using this MR289 the Scheduler MR also:
- removes
_TARGET_PHP_IMAGE_VARIANT: "apache"from a custom Drupal 9 job, and the tests still pass showing that this variable is propagated via writing tobuild.env. - makes a change to a test file, to demonstrate that the CURL variables are being propagated as inrtended for test-only changes.
I will also review the doc pages as I expect there will be a few small changes relating to this.
Tested in this pipeline with default-ref (not this MR) to check that the test-only jobs runs when it is should not. The problem is that it did not run. The repo does not have any nightwatch tests but the MR was adding one. The composer log shows:
$ git show -2 --stat --oneline
cc9a4dc Merge branch 'mr292-3488104-test-only-ignore-nightwatch' into '2.x'
.gitlab-ci.yml | 60 +++++++++++++++++++++++++++++++++++++-------
tests/src/Nightwatch/temp.js | 1 +
2 files changed, 52 insertions(+), 9 deletions(-)
03a8821 Baseline without using MR292
.gitlab-ci.yml | 60 +++++++++++++++++++++++++++++++++++++-------
tests/src/Nightwatch/temp.js | 1 +
2 files changed, 52 insertions(+), 9 deletions(-)It seems that the existing rule changes: - tests/**/* does not trigger when the change is adding a new file. It would be good to make that work properly, because the concept of 'test-only changes' should cater for new tests, not necessarily just changes to existing tests.
The test-only job rule is:
- changes:
- tests/**/*
when: manual
This needs to include tests/src/Functional, tests/src/FunctionalJavascript, tests/src/Kernel and tests/src/Unit but not tests/src/Nighwatch.
Is it too restrictive to include those four directories explicitly? Maybe it's OK, because 'test-only changes' can be documented as only working with the standard directory-naming structure, and if a project has tests elsewhere?
The PHPUnit jobs use the following .phpunit-tests-exist-rule
- exists:
- '**/tests/**/*Test.php'
when: on_success
So maybe the test-only rule could be changed to be
- changes:
- tests/**/*Test.php
when: manual
I notice also that the test-only rule does not have the leading '**' and maybe that should also be added, to find tests in sub-modules too.
jonathan1055 → created an issue.
jonathan1055 → created an issue.
I created an issue fork to do this, but has it already been done? Neither the 2.0.x nor the 1.0.x branches have any beforer_script for cspell, and they both already have a .cspell-project-words.txt file.
jonathan1055 → made their first commit to this issue’s fork.
Am I right in thinking that your original two commits on this MR were invesitgation into why the Nightwatch tests failed, and that these changes can be removed now? There were conflict, and also now eslint failures (double to single quotes), but I think all the changes to tests/src/Nightwatch/Tests/keyboardTest.js can be reverted? I won't do that until you confirm.
Now that Drupal 11 is current, you probably want to also test with Drupal 10, so I will add the opt_in for that. There is no 'Max PHP' variant at the moment, either, because D11 only supports PHP8.3, it is not ready for PHP8.4 yet. That's why this pipeline has fewer jobs that before. You can leave OPT_IN_TEST_MAX_PHP:1 and as soon as core supports PHP8.4 the variant will be added to the pipeline.
There have been a few changes in Gitlab Templates since 1st Ocober. In particular 📌 Update templates so 11.0 is the default/current branch RTBC which was on 8th October. I will rebase the MR and then you can decide how to proceed, as there is no 'next major' job at the moment.
Hi ramreddy.kancherla
Please could you tell me what Drupal core version you are using? It was not a problem when that code was written, but maybe things have changed since then.
Postponed on 🐛 Service not known with OPT_IN_TEST_NEXT_MINOR Active because MR291 will change the way services are defined.
Probably also postponed until core 11.1 is released and we update CORE_STABLE to '11.1.0'
The curl change was prompted by the error in 🐛 #3480767 Causes break in behavior of tests configuration Active
Changed title and updated IS, as this issue now covers the CURL variable. Because it was all to do with build.env I added them in to this MR. But if you would prefer the two enhancements separated out, that's fine I can make a new MR for the curl variables.
Readly for initial review and feedback.
Tested on https://git.drupalcode.org/project/decoupled_pages/-/merge_requests/8
Pipeline https://git.drupalcode.org/issue/decoupled_pages-3422594/-/pipelines/340026
All three Nightwatch jobs pass.
Current
Core Drupal: 11.0.7, Nightwatch 2.4.2
DRUPAL_TEST_WEBDRIVER_HOSTNAME='localhost'
DRUPAL_TEST_WEBDRIVER_CHROME_ARGS='--disable-dev-shm-usage --disable-gpu --headless'
DRUPAL_TEST_WEBDRIVER_W3C=false
DRUPAL_TEST_WEBDRIVER_PORT='9515'
Previous Major
Core Drupal: 10.3.8, Nightwatch 2.4.2
Four variables same values as above.
Next Minor
Core Drupal: 11.1.0-dev, Nightwatch 3.7.0
DRUPAL_TEST_WEBDRIVER_HOSTNAME='selenium'
DRUPAL_TEST_WEBDRIVER_CHROME_ARGS='--disable-dev-shm-usage --disable-gpu --headless'
DRUPAL_TEST_WEBDRIVER_W3C=true
DRUPAL_TEST_WEBDRIVER_PORT='4444'
I did not know (and did not even think we could) load both services, I thought there would be some clash or conflict. This is such a simple solution, and we already have the environment variables that make it work. It clashes with, but will simplify 📌 Update to new driver when D11 "current" uses Nightwatch 3 Postponed
The documentation pages for PHPUnit and Nightwatch will need updating if we are going to do this change.
Re-using MR6 to test Gitlab Templates MR921 on 🐛 Service not known with OPT_IN_TEST_NEXT_MINOR Active
Thanks for triggering the downstream pipeline. Yes I will open a draft [ignore] MR to add CI_DEBUG_SERVICES: true, but it looks promising as all three Nightwatch variants passed.
@liammorland Testing with https://www.drupal.org/project/gcds → locally is not so straight-forward because it does not appear to have any public packaged releases for Composer to get. I guess I could go back to downloading the zip file. Or do you know offhand of other themes which require modules?
the files are present (see composer job log).
Ah, yes, of course. I've not worked on the 'Upgrade Status' job, so I'm not familiar with what it does. The pipeline is running with drupal_core 10.3.6 so it does not matter that simplify_menu has no D11 version. It is already loaded.
Here's an initial test
https://git.drupalcode.org/project/scheduler/-/pipelines/339622
PHPUnit jobs for 'current' and 'next minor' both passed. At the job start-up they both show:
[service:selenium/standalone-chrome-selenium]
[service:drupalci/webdriver-chromedriver-chrome]
[service:drupalci/webdriver-chromedriver-chrome] 2024-11-15T10:14:50.401430414Z ChromeDriver was started successfully.
The 'next minor' has more debug during runtime, and that shows [service:selenium/standalone-chrome-selenium] is being used during the tests. I'm not sure why there is no detailed services debug during the runtime tests on 'current', maybe the drivers do it differently. But the initial testing of this one javascript test shows that this is worth further investigation.
Now I need to test on a project that has Nightwatch tests. Any suggestions?
Did anyone ever test launching both CI images and just choosing which image is actually connected to
I don't recall ever seeing that suggestion. It is definitely worth investigating.
The upgrade status jobs log shows
The repository at "/builds/project/gcds" does not have the correct ownership and git refuses to use it:
fatal: detected dubious ownership in repository at '/builds/project/gcds'not sure if that is relevant though.
At the end we just have
In ThemeInstaller.php line 176:
Unable to install theme: 'gcds' due to unmet module dependencies: 'simplify_menu, twig_tools'. I don't think this is a fault in gitlab templates, is it? simplify_menu → does not have a Drupal 11 version yet.
Good to hear that. Just looked at your MR and I am pleased that the concise first option in #6 worked for you. It means that if you ever add another service to the phpunit job, the 'next minor' will automatically stay in sync with it.
Pushed the inital removal and changes, but any testing on this will fail for 'current' because 11.0.7 still needs the legacy chrome browser.
jonathan1055 → changed the visibility of the branch main to hidden.
Now that 📌 Update templates so 11.0 is the default/current branch RTBC is complete we can unpostpone this and start work. It will not be ready for merging yet, but we can at least investigate the timeline for when we can do it.
As it seems your tests do not need the chrome driver at all, you could try (untested)
phpunit (next minor):
services: !reference [phpunit, services]
so that what ever you have in the 'current' phpunit services will be re-used in the 'next minor'. Or you could just duplicate your definition.
The phpunit 'next minor' job currently has to redefine the services, because the regular phpunit uses 'with-chrome-legacy' whereas the next minor has 'with-chrome'
https://git.drupalcode.org/project/gitlab_templates/-/blob/main/includes...
So your custom override is only affecting the 'current' phpunit job.
You can probably fix this in multiple ways, either by repeating the customisation in 'next minor', or re-using the services from your 'current' job.
Yes, absolutely. That issue was already referencing this one.
Thanks for the above info.
Looking at .calculate-gitlab-ref it should pull off the _GITLAB_TEMPLATES_* that we do have to set.
The testing that I do involves defining the gitlab MR repo and ref directly in the file:
include:
# MR 289 build.env and internal variables
- project: issue/gitlab_templates-3487169
ref: 3487169-build-env-internal-variables
file:
- '/includes/include.drupalci.main.yml'
- '/includes/include.drupalci.variables.yml'
- '/includes/include.drupalci.workflows.yml'
as per the second section of the documentation. So the _GITLAB_TEMPLATES_ variables are not set, and are not actually used to drive the testing. I do it this way because I have multiple gitlab templates MRs being tested simultaneously, so I have a project MR for each one. Using the form would be tedious as it would require continual changing if I wanted to switch back to testing another MR.
We did do quite a bit of thinking and testing when I first started working on gitlab templates MRs, and we had to introduce the _CURL_ variables for a very good reason. I will dig back and find the issue where we had to do that.
Maybe another part of our testing process, just before merging, could be to run a pipeline from the UI form, as that would have highlighted this error.
Pretty good to get the hot fix deployed in under 2 hours of the fault being reported. Thank you.
Oh, that was a bad error. I'm sorry about that. I think what happened is when we test a MR we have to set those two CURL variables manully (so that the test actually yses the new/modified fies). But this masked the fact that they were not created for real. Sorry!
I'm not sure how we could have tested and discovered this error before commiting the MR.
This was the first time we curl a file somewhere other than composer job. Maybe a solution would be to add them to build.env in the composer job, then they would be available in all job, and we'd avoid this problem. I can try that in
📌
Document internal variables and make them consistent using build.env
Active
jonathan1055 → changed the visibility of the branch main to hidden.
Thanks for agreeing to the 120 array line length.
The 'help us' documentation needs to be reviewed and updated, to include the bit about standards and using run-local-checks.php etc, but I will do that on
📌
Documentation pages
Active
.
Regarding my comment in #3 we can allow longer lines using "printWidth": 120 in .prettierrc.json, so I have pushed that change. This length matches the Core standard for inline PHP arrays, but the YAML standard has not caught up yet (I may raise that within the Standards project). I think this change makes the variable definitions easier to read. But if you don't like it I will reverse it out.
Updated the IS with explanation about quotes.
Thank you. That's the answer I wanted to hear.
I've changed the issue title, as you often use the pre-prepared text for the commit message. Not sure I've got it exactly right, but when looking through the commits it is better to have what is done, not what the problem was.
jonathan1055 → created an issue.
Thanks for confirming the results.
Yes I understand your concern. The problem is that the config file is linked to the phpunit version which has to vary depending on which core version is being tested. So an alternative would be for gitlab_templates to maintain multiple phpunit.xml in /assets and have logic to determine to the correct one to copy depending on which phpunit --version is being used. That would be quite possible, and the benefit is we just edit and maintain those files and do not have to use the new script to adjust the file at runtime. The downside is we have to keep up with necessary changes in those file, as and when Core make them and/or new phpunit versions/configurations are needed.
I can see then benefit in both solutions, and would be OK to explore that solution if necessary ...
Test when logo.png does exist in /docs folder
https://git.drupalcode.org/project/scheduler/-/jobs/3333337
This produces the result as shown in #17 with the alternative rainbow ring logo
Test when logo.png does not exist in /docs folder
https://git.drupalcode.org/project/scheduler/-/jobs/3334239
This produces the result as shown in #15 above.
I suggest that the pwd && ls can stay in, as that gives useful info in the 'pages' log, given that we are manipulating those files.
Ready for review.
That works. I added a different logo.png to /docs in the MR and the log shows
$ pwd && ls -la logo* && ls -la docs/logo* || true
/builds/project/scheduler
-rw-rw-rw- 1 root root 11897 Nov 12 17:05 logo.png
-rw-rw-rw- 1 root root 17790 Nov 12 17:05 docs/logo.png
$ if [[ -f logo.png ]]; then # collapsed multi-line command
logo.png not copied to /docs because that file already exists.The docs site is now rebuilt with the alternative logo as required
Indeed, it seems that you were copying the file to the docs folder after running mkdocs on your MR.
Yes, I was playing around with the sequence. The logic for copying the file might work better than I first thought it would. When copying just to /docs before calling mkdocs the file is used but is not permanently stored in the repo /docs which is good, because that means if the maintainer has not created an alternative then the main logo file is changed it will get copied again. I have re-run the pipeline and each time it shows /docs/logo.png does not exist, and proceeds to copy the file.
Now need to test what happens when a file is stored in the repo /docs
ha ha, that's a silly mistake. Thank you! I was testing what you said above, but forgot to revert it.
The file had been copied to https://project.pages.drupalcode.org/scheduler/logo.png now, but just not showing in the docs page.
OK, fair enough. Yes I will add the check not to overwrite, and give a message if not doing so.
I would have thought that the cp would have a permanent effect as the public directory is built from there. I don't think we need to play with the URLs.
Maybe the file did not appear to be copied on my test because it was running in a MR branch. I changed the 'pages' rules in a custom override, as normally it only runs on commit to default branch. The documentation pages were updated this way, but the image was not copied. It would be nice to test this change somehow before commiting, but now sure how I can do that.
I naively added test -f logo.png && cp -v logo.png docs/logo.png into the page job, and of course this does copy the file within the pipeline job, but that has no permanent effect. Do we need to use $CI_MERGE_REQUEST_PROJECT_URL which contains https://git.drupalcode.org/project/{the-project} then append /docs ?
New releases coming quite frequently now. 11.0.6 and 10.3.7 have only just made it into the default-ref tag.
jonathan1055 → created an issue.
Good idea to test that. I changed the mkdocs.yml
theme:
logo: ../logo.png
favicon: ../logo.png
but it did not make any difference.
I like the idea of copying the logo file in the 'pages' job, and I think it should be done regardless of whether it already exists in docs folder. The two files should always be the same, there is no reason for a different logo version in the documentation site. But more importantly, if it is only copied the first time, then the maintainer modifies the logo, they will not know why the docs site still has the old version. It is better to keep them always synchronized.
I've pushed an initial change, not tested yet, but will try it out with Scheduler, using a modifed 'pages' that runs on MRs not just commits to the default branch.
There's one more line of unnecessary output I will remove, but it will not affect the RTBC
Thanks. I was just inspecting the source, and saw this '404 not found' error on https://project.pages.drupalcode.org/scheduler/logo.png and was going to ask how we get files there.
Would it be useful to add a line about this into the MkDocs page?
jonathan1055 → created an issue.
Renamed, removed debug.
Re-testing here https://git.drupalcode.org/project/scheduler/-/pipelines/336191
Ready for review.
I spotted cmlara's comment at the end of the issue summary
add a check for '--no-configuration' flag.
This is important, because if the maintainer has explicitly said they do not want configuration then we should not add the core file. I extended the regex pattern to ! $_PHPUNIT_EXTRA =~ (-c |--configuration|--no-configuration) to check for any of those.
Test with --no-configuration
https://git.drupalcode.org/project/scheduler/-/pipelines/335644
Test with --configuration
https://git.drupalcode.org/project/scheduler/-/pipelines/335699
What do you think about the name of the new script phpunit-xml.php . Is that OK?
I've created MR21 in your testing issue
📌
[IGNORE] GitLab CI Sandbox
Active
https://git.drupalcode.org/project/vault/-/merge_requests/21
It looks like it has worked, because the final job in MR19 had
Code Coverage Report:
2024-11-09 17:04:38
Summary:
Classes: 0.03% (1/3321)
Methods: 0.24% (44/18454)
Lines: 0.31% (434/141283)
but the new output in MR21 is
Code Coverage Report:
2024-11-12 04:45:16
Summary:
Classes: 0.00% (0/16)
Methods: 51.72% (45/87)
Lines: 79.35% (465/586)
jonathan1055 → made their first commit to this issue’s fork.
OK, thanks I will create a new MR
Here's a test on Scheduler when the project has their own phpunit.xml.
https://git.drupalcode.org/project/scheduler/-/pipelines/335494
You can see the ls output in the log and the message "Using the project's existing phpunit.xml(.dist)"
Hi @klausi,
The priority of this is set to 'major' and it has been RTBC for a week. Any chance you could commit it and make new Coder release. The longer the sniff stays in the more grief we are causing for developers, until the standard has been agreed.
Thanks.
Good point. I pushed a change - tested locally with multiple 'coverage' and 'source' items and this removes them all.
Test MR https://git.drupalcode.org/project/scheduler/-/pipelines/335472
@cmlara - are you OK if I modify your Vault MR19 to test this change?
From #3
And I guess another option would be to not use any configuration file (that's how it was before the other issue was merged) and only use core's one via opt-in or something similar.
I chose not to do it this way as I don't think it would be very helpful. We added the config file so that generated .html worked properly, so if anyone wanted the artifacts they would then have to opt in. If they also had code coverage, they would want to opt out. But could not do both simultaneously. That might be a rare case, I know. But I think modifying the file like the script does, and removing the things we don't want, is a reasonable way forward. It may also be useful to have this functionality for other future changes in the phpunit.xml config.
This is ready for feedback and discussion. I've left in the debug to display the file before and after.
The D10 phpunit.xml.dist has coverage whereas in D11 phpunit.xml.dist it is source - at least that what it looks like from the comment.
Tested for both here https://git.drupalcode.org/project/scheduler/-/pipelines/335352 - the file is modified as expected, and is used in the phpunit job, we can see Configuration: /builds/project/scheduler/phpunit.xml in D11. Composer 'previous major' is having trouble downloading everything, but that may resolve itself later.
I've just pushed a first draft of a script, I wrote this before you posted your comments above.
Hi cmlara,
Yes, when we made the change to use the core file, I saw those 'coverage' lines, and was was wondering whether they are correct for a contrib project. But then we moved on, and I forgot to ask and investigate. It clearly does affect your output, so we need to do something to fix that. We can't have a customised phpunit.xml in /assets, because the file has to vary between core versions, and we can't maintain duplicates for each core version being tested. Ideally we want to use the core file (from whatever core version is being tested) but post-process it to remove customisations that are not applicable to contrib. Or alternatively, re-write the file picking out the specific keys/data items we need, and dropping everything else. I'm sure there is a good solution to this.
Attached is the file I will upload to the Scheduler project, and is shown in the issue summary. Thank you to everyone who has contributed here. I am marking this 'fixed' and I have tried to give correct credit. Also need to add credit to @urvashi_vora from the original issue.
Thanks jvbrian but we've already settled on the design, not going to completely change it now.
Updated issue summary with the reduced size file, as required.
The failing test may be caused by the Twig version, the composer job log shows twig/twig (v3.14.1) which has a timeout problem, and woud cause the problem of not finding expected elements on the form. The previous pipeline on this MR had (v3.14.0) which does not have the problem. However, jobs today should be using v3.14.2 which was released yesterday to fix the problem. Is this project locked to twig 3.14.1 ?
💬
Updating phpstan and twig via Composer generates a blank page when editing a node
Active
has the background.
I decided to make my own, with the corrections I wanted. I also realised that the logo should not be rounded at the corners, as that is explicitly reqiested. However, in our case the rectangle was representing a notebook/calendar which had round corners, so it might have been OK anyway.
100% agree with all of it.
Thank you. I have move the three variables to the hidden file.
Good hint about using the fork repository -> pipelines -> new, then selecting the branch. Attached are screen shots of the form on the default 2.x branch and the MR fork, showing that the three variables are no longer in the form.
This is ready for review.
Tested in MR for Drupal 7 and all OK
https://git.drupalcode.org/project/scheduler/-/pipelines/332982
I'd prefer to move the variables to the hidden file, as we did with _TARGET_CORE and are intending to do with _TARGET_PHP, possibly in preparation for deprecating them (as we did with target_core). These variables which have to differ between variants I don't think belong in the UI form. Changing the values for these critical variables can be done easily in a project's .gitlab-ci.yml file, and that should be the preferred way. Or a user can manually add the variable to the form, but that's OK because they are taking responsibility for it. In the majority of cases the UI should be used for modifying the proper front-end options (skip, concurrent, opt_in, etc) which are proper global variables that do not need to change between variants (which is the origin of this issue/problem)
I have not moved the variables (yet) to the hidden file, but just created internal variables $PHP_IMAGE_VARIANT and $PHP_IMAGE_TAG, which will never appear in the UI form, so can always be set correctly in the main.yml and main-d7.yml files. These can still be added manually in the UI if a user wants to force the values, but that would be the rarer case.
Pipeline from Scheduler MR all ran as expected. I add the values of the two new variables into the printout at the end of the log.
https://git.drupalcode.org/project/scheduler/-/pipelines/332884
Now we need to be able to test this change using a UI form. I'm not sure how to do that. Can we access the form on a MR branch?
Yes, I also think it is the variable precedence. I recall we discovered before that variables in the UI form override the variables in .gitlab-ci..yml, at all levels, so even if a job re-defines it's own value the form value will still be used. We could create a new variable, but also we could investigate whether moving it to the hidden-vars.yml would help?
I re-read your comments above, and see that you ran the pipeline from the UI. I have just tried this and sure enough it does fail for Previous Major, because it is trying to use image drupalci/php-8.1-ubuntu-apache:production
See https://git.drupalcode.org/project/scheduler/-/jobs/3285264
So I think we do have an issue for Gitlab Templates, when running from the UI. Therefore I have re-opened this, even though there are some unrelated problems above, as it does have good info in the comments. But if @fjgarlin would prefer a new issue we can do that.
I was curious about the indentation, because knowledge of this is important for when working on and maintaining Gitlab Templates. So I did a test, with indented variables
https://git.drupalcode.org/project/scheduler/-/merge_requests/201/diffs?...
and I'm pleased to say that the pipeline behaved exactly as before
https://git.drupalcode.org/project/scheduler/-/pipelines/332261
So I think the problems that caused this issue are not related to the indents. Which is good news for our understanding of how Gitlab pipelines are built.
Tested here
https://git.drupalcode.org/project/scheduler/-/pipelines/332086
10.3.7 and 11.0.6 both have Twig 3.14.2 and the tests all pass, when they failed earlier with Twig 3.14.1, so this can be RTBC now.
#3485956-42: Recursion limit exceeded with Twig v3.14.1 when editing a node or a block → comments 42 - 47, a new version 3.14.2 of Twig has been released today, which fixes the problems in general for Contrib 10.3.5 and 11.0.5. So we can proceed to bump the versions providing these do not lock in the incorrect version.
I will run a pipeline using this MR to check the composer logs.
Pipelines for Contrib at 10.3.6 and 11.0.5 at the start of the week both had Locking twig/twig (v3.14.0) and ran fine.
Earlier today both jobs had Locking twig/twig (v3.14.1) which caused many test failures.
Re-running now gives Locking twig/twig (v3.14.2) and the tests pass, so in principle this has solved it for some contrib at least.
With this MR and $DIVIDER removed - 'environment check' before. The ended green because the pipe syntax was removed and the 'exit 1' was therefore not executed
https://git.drupalcode.org/project/scheduler/-/jobs/3279475
With latest commit to this MR, 'environment check' has the correct message box and ends red as required.
https://git.drupalcode.org/project/scheduler/-/jobs/3280383
I have resolved the various MR comment threads where the code has been fixed, and also reverted the temporoary change that allowed the 'pages' job to be tested.
Unrelated pipeline fail, due to 'check versions' showing that 11.0.6 and 10.3.7 have been released. This is RTBC from me, but will let @fjgarlin do the final review.
Given that this is quite a major change, I'd like to use this MR a little more before you merge it. Also I think the issue summary needs to add the fact of the quotes change, why we are doing it, and some links to the Prettier documentation. They have a nice rule which says "the aim is to minimise use of backslash" So you can use double quotes where the string contains a single quote. But singles are prefered if all else is equal. That needs to be said somewhere.
Thanks for fixing the message. Here is the test before
https://git.drupalcode.org/project/scheduler/-/jobs/3272612
and after
https://git.drupalcode.org/project/scheduler/-/jobs/3279263
All looks good now, but I will make an adjustment for that extra $divider variable in the 'check environment' job
I found a bug in unformatted-links.php when using it in Schedule. It required some better debug to resolve, so I left that in too.
Thanks for the link, but I don't think that's the problem. It is the Prettier rules that complain, the eslint rules are fine. There were two places where we had one blank line and Prettier wanted it removed - this is not a configurable option - see https://prettier.io/docs/en/rationale#empty-lines
The first was fixed it in this commit for gitlab-ci/template.gitlab-ci.yml
The second place we had already added an unwanted comment in hidden-variables.yml and I removed it in this commit
If you are OK with these two changes then we are all sorted and this is ready for review.
Thanks. I agree. Even if it makes some conflicts in the short term it is better to follow core standards in the long run. I have made the changes for doiuble to single quotes. There will still be a couple of prettier failures - blank lines are automatically stripped at the start and end of a block, and we have no control over that rule, it is not an option. So we can remove the blank lines, add just a # at the start, or maybe find another solution.
I left a further comment in the MR
Tested on D11
https://git.drupalcode.org/project/scheduler/-/pipelines/331061
With Composer Max PHP I used PHP_VERSION: $CORE_PHP_NEXT which cannot resolve to an installable set of dependencies yet, so it automatically retried twice (I did not re-run it manually). So that is working as expected, even though re-running would not help in this situation.
Tested on on D7
https://git.drupalcode.org/project/scheduler/-/pipelines/331067
Thanks for working on this manojsaha. However, it does not look like you have created a new file from scratch, which is what is needed here. It looks like you have just taken my mock-up and made some changes. When zoomed in I can see the rough edges of the graphics, exactly duplicated from my mock-up copy/paste. Also the mis-alligment and uneven spacing is identical to the mockup. The shapes need to be properly aligned and distribued evenly. See the two anotated attachments.