🇬🇧United Kingdom @jonathan1055

The work previously done in MR257 removed the legacy override in the 'current' phpunit job, but adds it back only for 'previous major'. Which means that 'previous minor' 11.0 would use the new values. This is wrong, just like it was wrong for Nightwatch. The downstream failures show this - https://git.drupalcode.org/issue/gitlab_templates-3514999/-/pipelines/45...

I've pushed the changes to show the new approach, by testing the drupal core version. We have to get the actual version used, not just rely on the value of DRUPAL_CORE as this could contain composer-like constraints such as ^11.

This is working ok apart from 'next minor' 11.2, which needs some investigation.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/47...

Pushed a change so the message is given only when the value was blank and is being set. In the normal job we get

Nightwatch version is 3.9.0
DRUPAL_TEST_WEBDRIVER_HOSTNAME set to 'selenium'
DRUPAL_TEST_WEBDRIVER_PORT set to '4444'
DRUPAL_TEST_WEBDRIVER_W3C set to 'true'

But when overrides are used we get get the version line but not anything else - shown in this job

Here is the updated documentation page

I have removed the temporary code which skips the other downstream jobs, and this is ready for review.

I have also triggered the downstream pipelines for d10-plus and d9-basic, both passed and the log shows what we expect. In particular, in d9-basic, the "current" variant automatically uses the legacy values, because it is running Drupal 10 not 11. The "previous major" is running Drupal 9.5 and an even earlier Nightwatch version 1.7, and it uses the same legacy values.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...

Here is a test of overriding the variables in Max PHP and here is the job showing that it worked. The legacy values were used, but the log message is confusing, so I'm going to improve that. The message is shown, then we do the checks for if the variable is empty, and may not make any changes. This needs to be better.

jonathan1055 → created an issue.

I have made the changes to implement option 3. Keeping BC by defaulting the actual variables to blank, and only setting if non-blank. The legacy variables remain, as they may be referred to directly in customisations.

The PHPUnit changes are better done in a separate issue, so I have removed them here. The work is similar, but testing separately is easier.

To make progress here, it needs to be in a Merge Request so that pipeline testing is run.

I've pushed some changes, the detection of the Nightwatch version is working, but now I see why the variables are empty in build.env - thi si s because they are only defined in .nightwatch-base so are empty at the time the composer job is running. We have several ways forward -

1. move the definition of the variables from .nightwatch-base into .composer-base, or
2. move the definition of the variables from .nightwatch-base into hidden-variables.yml
3. do the derivation of the variables in .nightwatch-base no .composer-base

I think option 3 is better, as then the work is only done if the Nightwatch job is actually being run. It will sometimes be wasted effort if doing it always in Composer job.

jonathan1055 → created an issue.

Just reviewing this issue to see if there anything else we need to do.

From #2
We have files with all those extensions except .css and .md.
I have raised 📌 Add test coverage for stylelint job Active for the css checking.
Only the 'main' branch has a .md file, but this is not triggered in the downstream pipelines. Do we need to add a .md file into the branches? Some phpcs checks are done on .md files. Or we could add 'GTD Main' into the downstream pipeline list? I was thinking that we don't have downstream coverage of the 'pages' job, so maybe we could use the main branch for that?

From #3

• eslint now also checks the Nightwatch .js file in addition to the .yml file. Running with _ESLINT_EXTRA = --verbose, search for yield: to show the files found and whether they were tested. It shows that two are checked gitlab_templates_downstream.info.yml and downstream-one.js
• we now get PHPStan jobs because we added .php files for the phpunit tests
• stylelint will be covered by the above issue
• phpcs was run even though there were no files to check. Running with _PHPCS_EXTRA = -v shows 0 files - see this example on 'main' https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/47... Maybe there is a mismatch between the job rules and the list of php files we derive. Could add issue in Gitlab Templates to investigate this

The problem in #15 was fixed a while ago.

I have added a paragraph about the git alias for comparing against the parent. Here is the updated file in MR14. Do I need to add the explanation as per in slack ?

I think next time, before I create the issue I will ping you on slack, so that I don't do wasted effort :-)

we can probably build a test case "somewhere" to replicate that issue and make sure it's fixed.

Yes absolutely. That's what I'm going to do for 🐛 Pipeline task fails with composer.json file Active

Hi ggh,
The current phpcs jobs pass, there are no problems to fix. Take a look at the Standards paragraph → on the Scheduler project page.
Sorry you had some wasted effort.

I found a simple way to test this MR within the Scheduler pipeline, getting the file directly from here. I add the following custom before_script

phpunit:
  before_script:
    # Test address module MR69 from https://www.drupal.org/project/address/issues/3512975
    - cd $CI_PROJECT_DIR/$_WEB_ROOT/modules/contrib/address
    - curl https://git.drupalcode.org/project/address/-/merge_requests/69.diff | patch -p1 --verbose
    - ls -l address.*
    - head -15 < address.module

This is the bit where the file is patched

The test at Drupal 11.1 (current) passed, and the test at 11.2 (next minor) now has no deprecation warnings for Address. It does have plenty of warnings for other modules, so I have raised Scheduler issue 📌 Autoloading hooks in the file tokens.inc is deprecated D11.2 Active to track all of them.

So from my point of view (only considering Scheduler testing) you have fixed the problem. Thank you for woking on this. Depending on when you merge, I may continue to use that patch, or try to get Address 2.x-dev when you commit it.

jonathan1055 → created an issue.

I tested via UI to make sure that the change to the javascript still works with BEFORE_SCRIPT_ACTIONS nightwatch-fail and it fails as intended, so all is good.
d9-basic https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
d10-plus https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...

I searched the issue queue and did not see this (not sure why) so I opened 📌 Bump core stable versions to 11.1.5 and 10.4.5 Active which I have now closed.

Duplicate of 📌 Bump core versions to 11.1.5/10.4.5 Active

jonathan1055 → created an issue.

Thanks for working on this. I will try to see if I can use this MR issue branch in Scheduler's pipeline, so test the changes. It might be tricky to do. So I may apply a patch from here within the job.

All done. Cherry-picked the Nightwatch change to d10-composer, just to keep the test file in line.
Also aligned the workflow changes to all other branches.

Thanks for that idea. I tried using browser.assert.hasOwnProperty but that gave false for each of the conditions, so no checking was done.
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/p...

I then tried using typeof browser.assert.containsText === 'function' but that resulted in true for both cases.
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/p...

Then after more research I found browser.expect.element().text.to.equal() and this worked in both the old and new Nightwatch variants without any conditionals.
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/p...

I tidied up the code, putting it all in one statement, which I thought was going to be OK, but it wasn't. The whole thing failed, and there was also an eslint error, which must be related. 21:28 is the point in the middle of element('h1').text suggesting a line break. That was incorrect too.
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/p...

So I put the browser.expect into a separate statement, which works just fine. Here are the proposed changes and this is now ready for review.

Thanks for letting us know.

Thank you for that info. I tested d9-basic with the old function .assert.containsText and it passes on D10 and D9, however we get a deprecation warning in the D10 'current' variant, which I am not surprised about.

So instead of having the old function and deprecation, we can leave the test as-is but use nightwatch (previous major): before_script to edit the function name, and thus only change it for the specifc variant. Tested in this pipeline and both Nightwatch jobs pass with no deprecations.

There is no MR for this change, I just did it on the generic testing issue fork - here are the changes.

In addition the Nightwatch before_script there is a change to the workflow so that a push to any issue branch on 📌 Plan for initial codebase Active always triggers a pipeline, and a fix to the display of the Nightwatch version. I will align the workflow change on the other principle branches too.

This is ready for review.

Thanks @codebymikey it is helpful to know that I was not missing something simple. Yes I agree even this scenario is hard to replicate in normal circumstances we still need to cater for it and prevent the loop. I can work on a test for this.

Thanks for the review. Merged MR12 into d10-plus.

Cherry picked to d9-basic and the Nightwatch test runs ok on 'current' which is Drupal 10, but it fails in 'previous major' which is Drupal 9.5 whichs uses Nighhtwatch 1.7. The test starts OK and the user is logged in, but we then get Error: Unknown api method "textContains"
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/46...

There may be a way to fix this, but is it worth the trouble investigating? Do we need Nightwatch test coverage at Drupal 9? I'm inclined to just prevent this variant from running Nightwatch via a customised when: never. Is that OK?

jonathan1055 → created an issue.

All of the forced failed runs had

WARNING: Event retrieved from the cluster: 0/4 nodes are available: 2 Insufficient cpu, 2 node(s) didn't match Pod's node affinity/selector. preemption: 0/4 nodes are available: 2 No preemption victims found for incoming pod, 2 Preemption is not helpful for scheduling.
WARNING: Event retrieved from the cluster: 0/5 nodes are available: 1 node(s) had untolerated taint {node.kubernetes.io/not-ready: }, 2 Insufficient cpu, 2 node(s) didn't match Pod's node affinity/selector. preemption: 0/5 nodes are available: 2 No preemption victims found for incoming pod, 3 Preemption is not helpful for scheduling.
WARNING: Event retrieved from the cluster: 0/6 nodes are available: 2 Insufficient cpu, 2 node(s) didn't match Pod's node affinity/selector, 2 node(s) had untolerated taint {node.kubernetes.io/not-ready: }. preemption: 0/6 nodes are available: 2 No preemption victims found for incoming pod, 4 Preemption is not helpful for scheduling.

for example here. This did not seem right, but was probably unrelated, so I re-ran a couple and they ended with the normal failure, and did not have the above message. Just another random thing, I guess.

This is ready for review. I have also added a change to remove the SKIP_ variables as it is better to leave these to default. I know this is partly out of scope, but the SKIP_NIGHTWATCH variable was not in that list, and that's what made me think it is better to have none there.

Re-run of both jobs was fine, they passed.

Here's a test from the pipeline UI, using BEFORE_SCRIPT_ACTIONS 'nightwatch-fail'
gitlab_templates_downstream-3513352/-/pipelines/450184

Seems there was something wrong with the curl execution to get the default phpcs.xml.dist
The passing job had

This project has no (.)phpcs.xml(.dist), getting default from assets/phpcs.xml.dist
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100   575  100   575    0     0   4802      0 --:--:-- --:--:-- --:--:--  4831

but the failed job had

This project has no (.)phpcs.xml(.dist), getting default from assets/phpcs.xml.dist
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100    12  100    12    0     0     93      0 --:--:-- --:--:-- --:--:--    93
100    12  100    12    0     0     92      0 --:--:-- --:--:-- --:--:--    93

Does this show that we only got 12 (K?) instead of the usual 575? We've had curl failures before (eg when the name is wrong) and it would be good to be able to trap that when it happens. But that's another issue. I will re-run and see what happens.

That's odd. I only added a nightwatch: before_script to the project's .gitlab-ci.yml but we now get failures for phpcs and phpstan, when they passed 20 mins before that.

https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...

The new nightwatch test (borrowed from Core) passes in all five variants in this pipeline. At the end of the composer job we can see that 'current', 'max php' and 'next minor' use Nightwatch 3.9.0 and 'previous minor' and 'previous major' use Nightwatch 2.4.2

Next, add $BEFORE_SCRIPT_ACTIONS =~ "nightwatch-fail" to make the job fail on request.

jonathan1055 → created an issue.

jonathan1055 → changed the visibility of the branch 3445052-test-without-mr to hidden.

Of course, we can't run javascript phpunit tests in Drupal 7. But I still added the appropriate version info in composer after_script.

jonathan1055 → changed the visibility of the branch 3512740-javascript-test-d7 to hidden.

Thanks for the review. I have expanded the test slightly, to create a user account and log in, thus actually testing a bit more. I also made a minor change to the version output, to include the PHP version, and remove extraneous text. But this is still RTBC and I will merge and cherry-pick to d9-basic, then see what we can do in the D7 branches.

jonathan1055 → created an issue.

This is ready for review. In addition to the Javascript test, I have added some lines to < code>.composer-base: after_script: to show the actual core version, phpunit version and nightwatch version. This has proved useful info to have in a single place, when comparing across all varaints.

Excellent, you've reduced it from 24 unresolved threads to just 9.

One odd thing I commented on is why it appears that the 'Docblock' sniff is being excluded here?
https://git.drupalcode.org/project/drupal/-/merge_requests/505#note_477884

jonathan1055 → created an issue.

The changes made for the Nightwatch variants are all still valid, nothing needs to be done. I re-ran the Decoupled Pages MR8
current 11.1.4, max php, next minor (11.2x-dev) all run Nightwatch 3.9.0
previous minor (11.0.12) and previoius major (10.4.4) both use Nightwatch 2.4.2
See this pipeline for MR8 for the details

PHPUnit

The changes in the PHPUnit jobs involve the variable MINK_DRIVER_ARGS_WEBDRIVER
.test-variables has the two definitions MINK_DRIVER_ARGS_WEBDRIVER and MINK_DRIVER_ARGS_WEBDRIVER_LEGACY

As it stands in the main branch, the phpunit 'current' job has the override to use the legacy variable
MINK_DRIVER_ARGS_WEBDRIVER: $MINK_DRIVER_ARGS_WEBDRIVER_LEGACY
which means that all the variants would use this value. However, the two 'next' variants contain
MINK_DRIVER_ARGS_WEBDRIVER: !reference [.test-variables, variables, MINK_DRIVER_ARGS_WEBDRIVER]
meaning that they use the new value not the legacy one.

The existing work done in MR257 removes the legacy override in the 'current' phpunit job, but adds it back only for 'previous major'. Which means that 'previous minor 11.0 is using the new values. We need to check if this is right, because for Nightwatch the 'previous minor' still needed the legacy value.

As there is no MR here, the status can be put back to 'active'.
This is so we can more easily identify which issues have been started and really do need work to complete.

As there is no MR here, the status can be put back to 'active'.
This is so we can more easily identify which issues have been started and really do need work to complete.

jonathan1055 → changed the visibility of the branch mr334-3506040-remove-hardcoded-custom to hidden.

✨ Try to remove most of the hardcoded "/custom" paths and use DRUPAL_PROJECT_FOLDER to calculate the right ones Active caters for this feature request and MR334 has just been merged. We picked the best from this issue, plus more. So I think this can be closed as a duplicate, even though this issue was opened earlier.

I assumed that this error was easily reproducable, but I just tried to follow the steps above. At step 3

3. Then manually transition to that "unpublished" state before the date.

how exactly is this achieved?

My workflow has "archived" as the unpublished state which cannot transition to itself, but that is just a name difference. If you edit the node and select the archived state manually I get the following validation error
and you cannot save.

If I use the content views I am also prevented from saving the manual change

I presume there is another way that the moderation state can be changed manually, so please can this be added in to the Steps to Reproduce in the summary.

Is anyone on this thread starting to work on adding test coverage? If no one is, then I will, but do not want to waste/duplicate effort if you have already started. If I don't hear back in a couple of days, I'll start on it.

Yes we need test coverage for this changes. I made a commen in the MR and test coverage would hopefully show that error.

jonathan1055 → created an issue.

I have checked the scenario you describe but cannot replicate the error. I used -

• SCMI 3.0.3 which was the latest release at the time you raised the issue (4 Sep)
• Scheduler 2.1.0 which was the latest release as at (4 Sep)
• Core 10.4
• Article node type enabled for scheduling and moderation, using default Editorial workflow
• Page node type, enabled for scheduling but not for moderation
• Created a page, set scheduled publish date for the future. When cron ran it was published as expected
• Re-scheduled the page, also scheduled an article to be published in the same cron run. Both got published correctly

I also repeated the above with the latest versions (Scheduler 2.2.1, SCMI 3.0.4) and got the same results.

Maybe you have another module which is interfering with the process? If you can reproduce the problem with a clean Drupal instalation please give the steps, and we can take it from there.

Thanks but many of the comments were not addressed, even if there was a later change. I've been through the entire list of chnages, and opened some more threads. It would be great if those who opened the earlier ones could check their own and close them when solved. Then we can really use the open threads as the driver to finally get this issue fixed.

Hi dieterholvoet,
Sorry for the last response. This slipped though the net.

If what you describe really does happen then it's a bug we need to fix. I will try to replicate it, but I thought we would have already tested this scenario.

Good to have that confirmed. I've now merged those two downstream MRs and reset the downstream branches here. Also as per this slack thread I've moved OPT_IN_TEST_MAX_PHP: 1 out of .downstream-base and into just the projects where we need it.

Downstream pipelines for GTD all work as expected. The d10-profile and d10-theme branches no longer run Max PHP. Setting to NR.

Thanks. Merged.

Thank you. Merged.

There are eight unresolved threads in the MR. So I don't think this should be set to RTBC until those are addressed and closed. In contrib MRs you can set push rules to prevent merging when there are open threads, but I'm not sure if that is done in the Core workflow. But in anycase those comments should be checked, and closed if done. But I don't they all can be closed yet.

Before we merge the downstream MRs and I remove those branch overrides, for our own information and knowledge, can you request write access on one of those issue branches, then re-try the downstream job. I presume that is all it needs.

I was typing the below, but you got there first.

This MR currently uses two downstream branches, to test the proposed additions to 'd10-theme' and 'd10-profile' so I will revert those back to running the default downstream branches before you commit this.

You're welcome. It seemed the best way to do it, and I wanted to get the situation resolved :-)

I have also tested these changes on a new d10-profile branch in GTD - see MR10 in 📌 Add a branch to test a 'profile' project type Active

When those two GTD merge requests are complete, I can reverted the downstream branches in this MR.

I have updated the doc page with the new derived defaults.

This covers everything now, so is ready for final review.

Here's a downstream pipeline from MR334 using this MR10 branch

• /modules/contrib still contains the correct 3rd-party modules
• /modules/custom no longer exists (correct)
• /profiles/custom is created and contains this project (correct)
• /themes/contrib is now created and contains the 3rd-party modules (correct)

These additions show that the fixes in MR344 are working as required.

Is there anything else to add here, before I merge this MR?

I have created a d10-profile branch and now we can expand the composer.json to load some modules and themes

Using MR10 with the current Gitlab Template, the pipeline shows that:

• /modules/contrib contains the 3rd-party modules (correct)
• /modules/custom contains this profile (wrong, should be profiles/custom)
• /themes contains the 3rd-party themes (wrong, should be themes/contrib)

jonathan1055 → created an issue.

Thanks. I have asked on the Bootstrap slack channel. In the meantime I have been using our new GTD d10-theme branch to test this, and specifically MR9 which adds some requirements of 3rd-party themes and a module. It all works well so far, with the projects being loaded in the correct directories.

I also realised that we have the $PROJECT_TYPE variable, which will be 'module', 'theme' or 'profile'. So instead of hard-coding "modules/custom" as the default if no override is given, we can now use DRUPAL_PROJECTS_PATH="${PROJECT_TYPE}s/custom" so that themes and profiles automatically get the correct default.

Here's a downstream branch where I have removed the override variable for "themes/custom".
https://git.drupalcode.org/issue/gitlab_templates_downstream-3511568/-/p...
The correct default is determined - as shown when browsing the artifacts

I've made the same change to main-d7 even though there is no project type to be read from the .info files. We now cater for an override variables: PROJECT_TYPE which we did not before, and default it to 'module' if none. This keeps things in line and easier to maintain.

If you are OK with these changes then I will update the doc to show the new more relevant defaults.

Closing this as no response after two months.

and here is a run using this MR as the downstream branch from MR334
https://git.drupalcode.org/issue/gitlab_templates_downstream-3511568/-/j...

1. 'modules/contrib' still has jquery_ui
2. 'modules/custom' no longer exists, as it is not used and 'themes/custom' contains this GTD theme (fixed)
3. 'themes/contrib' has 'bootstrap' and 'stable' (fixed)

I think this shows that the change to expand_composer_json is working as intended. It works for themes, so there is no reason it wont for profiles. (but we will get a profile branch in the project in due course)

Thanks for that. I added bootstrap ^3 and stable ^2 into the composer.json requirements. Bootstrap is good because it also needs the jquery_ui module, so this tests the download locations for both themes and modules. Here is the composer job artifacts - web directory run without MR334.

1. 'modules/contrib' has jquery_ui (good)
2. 'modules/contrib' has this gtd theme (incorrect, should be in themes)
3. 'themes' has 'bootstrap' and 'stable' (incorrect, needs 'contrib' sub-folder)

2 and 3 demonstrate the problem we are solving in MR334.

Manual test of BEFORE_SCRIPT_ACTIONS: cspell-use-madeupword phpcs-fail worked as expected
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...

I also committed DRUPAL_PROJECTS_PATH: themes/custom as a permanent variable in the .gitlab-ci.yml

I have looked at a few themes and I don't think they use a composer.json. I could not find a way to use the search but limit it to themes.

I have pushed a new branch named d10-theme
https://git.drupalcode.org/project/gitlab_templates_downstream/-/tree/d1...

The pipeline ran
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...

Can we add anything to it, to make it do things that a theme does, which are different from a module?

All questions resolved, but it would be good to test the change in expand_composer_json.php? Do we know any projects that would be affected by that theme and profile change?

I had this on my list of things for GTD, but it has now come to the fore given the latest changes in MR334

jonathan1055 → created an issue.

I have pushed those two changes, but have asked some questions in the MR.

On ✨ Consider using contrib instead of custom for module path Needs work I had no idea that the existing MR238 was so similar to this new one. That original is quite old, has multiple conflicts, so was worth me starting afresh, but I had not actually studied that one before.

There are two specific things that are not done in MR334
(a) adding 'contrib' into the path for themes and profiles in expand_composer
(b) changing the php variable name from pathToModules to pathToProject

Do you want either of those changes incorporated here?

OK, fair enough. I suppose those pipeline users would be annoyed if they had to manually trigger it. But it would have been good to have it as manual right from the start.

Yes I will add that onto the documentation list.

Thanks. Merged.

I think having the 'upgrade status' job default to being manual would be a good thing to do in the real gitlab templates. There was a slack conversation about the origin of that job being from the update bot, and it was never intended to be run continually in pipelines. If we change the default to 'manual' I can add a couple of lines in a doc to show how to override it and make it automatic if the developer really wants that.

What do you think? I will start the issue if you think it's a good idea.

Timing :-)

I have made the doc change - you can preview it here
https://git.drupalcode.org/issue/gitlab_templates-3506040/-/blob/3506040...

OK fine with me, I've reverted that.
RTBC

I pushed the change to ignore .git* and it made a bit of a difference to the number of files in the log

web/**/.git*/**/*: excluded 478 files              
.git*: excluded 3 files                            
.git*/**/*: excluded 26 files                      
vendor/**/.git*: excluded 53 files                 
vendor/**/.git*/**/*: excluded 85 files            
web/**/.git*: excluded 435 files    

The download size unzipped was only reduced by a few MB and there are still .github folders downloaded, so I don't exactly understand how that exclude: is meant to work. I thought I did, but clearly not.

This is RTBC if you want to get on and do it. The last commit can stay in, as it does not appear to do any harm?

The alternative to not passing this big artifacts between jobs is to run the composer install commands inside each job, but that'll be a big change.

So I think that answers my question "... can specify a smaller subset when downloading, or is it one and the same thing?"
The composer artifact is deleted in 1 week, which is good. All the others are 6 months.

I have pushed the change to specify a name for the composer and upgrade-status artifacts. Here is the re-run of Contribution Records MR3 - artifact name is ok.

I also tested the GTD MR7 (which runs d9-basic on Drupal 9 and 10 with Upgrade Status to check D11) manually via UI specifiying this MR to test against. The two composer jobs correctly have the $CI_PIPELINE_ID-$CI_JOB_NAME_SLUG added, making it possible to download them both without a name clash. The Upgrade Status job likewise has the correct artifact name. Here is that pipeline

I did notice in the composer artifacts, there are several vendor projects which have .github files. They are not large, of course, but could they also be ignored? Would simply changing .git to .git* in all the filter rows achieve that? It might be worth just seeing how much that reduces the file number and overall size? Other than that question, this would be RTBC.

Good. I have removed D11 compatibility from composer.json, deleted the upgrade-status-fail before_script, and also set the Upgrade Status job to be manual. This way, we run it when needed, but the normal pipeline ends green. If we always ran the job, even when not testing anything in it, the pipeline would always end amber, and just means we have to take a deeper look to check why. I think it's good if the GTD jobs normally end green.

So this is ready for review.

I've just realised that this needs documenting. I'll do that tomorrow.

All debug and downstream overrides removed. Ready for final review.

Excellent, thank you. In fact, the changes in that PHPCS mr just merged make use of the recently added $DRUPAL_PROJECT_FOLDER variable, which is now derived using $DRUPAL_PROJECTS_PATH. So there is nothing to do in the phpcs jobs at all :-)

The rebased mr downstream jobs for the modified path for phpcs in the downstream pipelines now end green instead of red as before.

I will remove the debug now.

in the phpcs job, so we are really close to getting this.

Yes, that change is trivial to do, I was just leaving it until ✨ Add basepath parameter to PHP Code Sniffer Active is merged, to avoid a conflict. Given that it is RTBC I thought it would not be too long. Also I know there are penty of debug lines to remove.

Workbench Moderation tests are now restored and running in 2.x and 8.x-1.x

jonathan1055 → changed the visibility of the branch 3495229-workbench-moderation-tests-8.x-1.x to hidden.

jonathan1055 → changed the visibility of the branch 3495229-restore-wbm-tests-8.x-1.x to hidden.

jonathan1055 → changed the visibility of the branch 2.x to hidden.

jonathan1055 → changed the visibility of the branch 3495229-restore-wbm-tests to hidden.

jonathan1055 → changed the visibility of the branch 3495229-restore-wbm-tests to hidden.

I've updated the summary with active links to the remainder of the test cases. This is all done except the phpcs job, so is ready for review, or you can wait until the phpcs job is done.

Using DRUPAL_PROJECTS_PATH: modules/anything/goes the downstream job passes

Ignore the red phpcs failure, I will make those changes when ✨ Add basepath parameter to PHP Code Sniffer Active is merged, to save having to re-do it after a conflict.

I have made the changes for the upgrade status 'sed'. I tried various things to avoid needing to do it in the first place, such as changing directory to the projects own top-level and running it there, which made no difference. I also tried changing the parameter --root=$_WEB_ROOT to --root=$DRUPAL_PROJECT_FOLDER but the result was still then same, with the long path writen to the output, backslashed, to give modules\/contrib\/mymodule-nnnnn\/mymodule.info.yml exactly as is now. I also tried to create a variable from $DRUPAL_PROJECTS_PATH adding in the backslashes, for use in the 'sed'but that was getting very cumbersome. It worked but needed six backslahes in the raw string, to allow the sed to work. Then I had the simple idea of doing two sed commands, the first just replaces all \/ with a single / . Then the second sed can use the straight value of $DRUPAL_PROJECTS_PATH and replace it with blank. This worked perfectly, and as a bonus it also fixes the backslashed url in the message See https:\/\/drupal.org\/node\/3070687

I've tested this with the modified d9-basic downstream branch, to get some output as required. The composer job nicely shows that this GTD branch is being used, and the upgrade status job has the required debug showing the two changes to the output file.

The above test is with the default value 'modules/custom' so next I will test it with a modified value for $DRUPAL_PROJECTS_PATH

jonathan1055 → changed the visibility of the branch 2.x to hidden.

I tested this MR with the Gitlab Templates MR on 🐛 Ignore more git files Active and it works exactly as intended.

jonathan1055 → changed the visibility of the branch mr326-3504083-workflow-reference to hidden.

jonathan1055 → changed the visibility of the branch mr331-3505585-variable-for-project-folder to hidden.

The other thing I noted is that the composer artifacts definition does not have any 'name:' key so the downloaded file is just called artifacts.zip. Same with 'upgrade status. But all the other jobs have
name: artifacts-$CI_PIPELINE_ID-$CI_JOB_NAME_SLUG
Can we change that to match? If so, shall we do it in this MR?