The phpcs job now passes green again.
One file could be fixed easily by getting the empty lines correct.
The two tests that had an early 'return' because they fail - this is better done by setting them to skipped. We also now see this info in the phpunit log, making it clear.
Ready for review.
First commit to remove the three phpcs:ignorefile
to demonstrate the warnings
https://git.drupalcode.org/issue/webform-3550939/-/jobs/6810972
Now that this is merged it has unblocked 📌 Fix PHPUnit tests that fail in Core 11.3-dev Active
Now that MR742 on 📌 LogicException: The hook update_dependencies does not support attributes Active has been merged, this work can be unpostponed. We now see the details of the three failing tests:
There were 3 failures:
1) Drupal\Tests\webform_scheduled_email\Functional\WebformScheduledEmailTest::testWebformScheduledEmail
Behat\Mink\Exception\ExpectationException: The string "Webform submission from: Test: Handler: Test scheduled email</em> sent to <em class="placeholder">simpletest@example.com</em> from <em class="placeholder">Drupal</em> [<em class="placeholder">simpletest@example.com</em>" was not found anywhere in the HTML response of the current page.
2) Drupal\Tests\webform\Functional\Element\WebformElementDateTimeTest::testDateTime
Behat\Mink\Exception\ExpectationException: The string "datetime_default: '2009-08-18T16:00:00+1000'" was not found anywhere in the HTML response of the current page.
3) Drupal\Tests\webform\Functional\WebformEntityTranslationTest::testSettingsTranslate
Behat\Mink\Exception\ExpectationException: The string "<span lang="en"><p>Submitted on [webform_submission:created]</p>
<p>Submitted by: [webform_submission:user]</p>
<p>Submitted values are:</p>
[webform_submission:values]
</span>" was not found anywhere in the HTML response of the current page.
Note that I have no knowledge of how webform works, and I am not actively fixing these tests, just preparing this issue so that they can be worked on by someone else.
Just making a note here that the processing and logic for reading the *.info.yml files and for setting PROJECT_NAME
and PROJECT_TYPE
can be re-factored here, following the changes made on
✨
Add recipes path handling
Active
. The sequence could be adjusted to look for a recipe.yml
file first, and if found then there will not be any *.info.yml so that processing can be skipped. Currently the *.info.yml files are searched for first.
Yes that's a good idea. I am fairly confident that the changes here to cater for recipes are the correct approach, and @jcandan agrees (see #13). So I will tidy up this MR and get it ready for merging. Given that each downstream pipeline is manual there is no harm in having the phpunit jobs fail at the moment. I have done some testing locally and I know what the problems are, which will be worked on in 📌 Add a Recipe branch Active
There is just a question about using $DRUPAL_PROJECT_FOLDER
as the input to symlink_project.php, and I have opened a couple of threads on the MR to discuss it.
Thanks. I have modified the documentation page with changes to the Project Name and Project Type paragraphs. Also added a SCRIPT_VERBOSE
hidden variable as per your suggestion.
I have included the new d11-recipe downstream branch, and that flushed out a problem in eslint where we were using $_WEB
in a path. I have now fixed that, and also removed the other usages of the combined string $CI_PROJECT_DIR/$_WEB_ROOT/$DRUPAL_PROJECTS_PATH
and replaced with $DRUPAL_PROJECT_FOLDER/..
which is consistent for all projects.
The downstream Recipes phpunit tests do not pass, but that is expected, as they are not ready yet. I think we need to decide on 📌 Add a Recipe branch Active what flavours of phputnit testing we want to cover, then merge that MR, before completing this one.
96 commits behind, tried to rebase, but needed a merge. It was all clean, no conflicts.
Updated IS with notes on the related issues.
jonathan1055 → made their first commit to this issue’s fork.
I've not done a full review, but the first thing that struck me were the many changes from $args('the-string')
to $input->getOption('the-string')
. In fact there are 73 of these, and I suggest it would be more readable if the $args array was retained, and simply built by the necessary calls to $input->getOption() but doing each only once. This would also reduce the compexity of the MR, and make it possible to print the entire $args array when doing debug. But having looked now at more of the changes, maybe my idea is not so important and stick with what you have done. But the diff is massive and needs a lot of checking. I can try to use this in a Contrib pipeline to test, but I expect the Core tests are doing enough to verify it already.
Thank you. That's a better way to do it. Much cleaner to read. I will test in a real contrib pipeline, but as you said before this could also have a build test. I don't know where the tests for run-tests.sh
are, but if you point me there I could look at expanding to add coverage for this. Or if you can do it easily and quickly then carry on and do so.
The 'phpunit next minor' correctly tries to run just the three tests, but we get no details due to the pre-existing problem
https://git.drupalcode.org/issue/webform-3550117/-/jobs/6764595
Therefore postponing this, as it requires 📌 LogicException: The hook update_dependencies does not support attributes Active
OK I'll open a new MR so I do not pollute your work, then
I have a static patch of the work done so far, so I can use that in other jobs, and for reference, so I'm fine with you using this MR, then we can see the further changes you do from this point. Core has enough redundant MRs already :-) But if you want to start again that's OK too.
Thanks for clarifying.
I've opened 📌 Fix PHPUnit tests that fail in Core 11.3-dev Active and removed those changes from here.
this is definitely wrong and essentially just a revert to how it was
Just checking, when you say "this is wrong" do you mean the current commited code is wrong? or that the fix suggested here is wrong? I think you mean the former, but just want to make sure.
Yes I could move the Gitlab CI changes to a separate MR in this issue or into separate issue. But the error in webform is only evident when testing against 11.3-dev, (aka 11.x or "next minor"). So the gitlab CI change to opt in to 'next minor' is needed to demonstrate the fix. The other CI change (moving _LENIENT_ALLOW_LIST to the top-level global variable) is not just convenient for this change, and is needed for the job to run at 11.x, but it also allows easier running of a pipeline via the UI. If you try a UI pipeline and do not specify _LENIENT_ALLOW_LIST = select2, styleguide
then all the composer jobs fail, because the blank value from the form takes precdence over the job-level coded value in the .gitlab-ci.yml. I could make a change here to repeat the job-level chnage for 'next minor' but that is still not the best way, which is to set the value at the global level.
The three tests that I have marked with @group @group webform_fail_at_11_3
I will move to a separate issue, as that is unrelated.
Thanks. I've replied in the threads.
Here is a test on the Webform module, submitted via UI just trying to run with _PHPUNIT_TESTGROUPS = webform_access
https://git.drupalcode.org/issue/webform-3547627/-/pipelines/616970 - it fails as expected becuase of errors in the Feed module test files
Now the same UI pipeline, but using Gitlab Tempates MR410, which applies the patches from here and add --directory
in the command
https://git.drupalcode.org/issue/webform-3547627/-/pipelines/616992 - phpunit runs just the small subset of tests and passes.
I have pushed a one-line fix for the empty CI_PARALLEL_NODE_INDEX
and that has fixed all of the tests :-)
This is ready for review and feedback. I have also used this patch in contrib jobs that were failing due to 3rd-party Test Discovery errors, and now they pass. I'll add links to them here.
Thanks @mondrake for the link. I've followed that issue.
To allow testing in Gitlab pipelines here is a static patch file for core 11.2 as there have been some changes in 11.x (11.3) which clash when trying to apply using https://git.drupalcode.org/project/drupal/-/merge_requests/13363.diff
I have opened ✨ Allow --directory and @group to work together in run-tests.sh Active and will now use the .diff url from that MR to apply a patch here, and show that the fix does what we need.
Interesting with the PHPUnit Build failed job, the command is
sudo -u www-data -E -H php ./core/scripts/run-tests.sh --phpunit-configuration $PHPUNIT_CONFIGURATION_FILE_PATH --debug-discovery --color --keep-results --types "$TESTSUITE" --concurrency "$CONCURRENCY" --repeat "1" --sqlite "./sites/default/files/tests.sqlite" --dburl $SIMPLETEST_DB --url $SIMPLETEST_BASE_URL --verbose --non-html --all --ci-parallel-node-index $CI_PARALLEL_NODE_INDEX --ci-parallel-node-total $CI_PARALLEL_NODE_TOTAL
but the test failed due to an unexpected value for test_names
[test_names] => Array
(
[0] => 1
)
This would normally be either empty, or a test group or a class or module, but not "1". Then we also see
[ci-parallel-node-index] => --ci-parallel-node-total
[ci-parallel-node-total] => 1
so this implies to me that $CI_PARALLEL_NODE_INDEX
is unintentionally empty, thus shuffling the value along so it reads the string "--ci-parallel-node-total" as the value. This means we get left with a "1" which is taken to be a test group (positional parameter) and that causes the $group array to be filters for the test "1", so we end up with no tests found.
I have pushed an initial commit to the MR. I'm utilising the existing --debug-discovery
argument to give further debug output to show what it happening. Some of this debug could stay, other bits will be deleted.
I will use the diff url from this MR to download and apply the patch, to deminstrate it in contrib testing.
Here's another place where we are being hampered in development by 3rd-party phpunit deprecations and errors.
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/67...
The "sam" module is brought in by one of the other requirements, I have no idea what it is for, but it means we cannot run any tests at Next Minor or Max PHP due the error. Having the ability to use --directory
would remove these errors (which we do not care about at all) and allow better use of our time.
I have created a d11-recipe branch, initially as a copy of d10-plus, and then modified so you can see how it changes from a module to a recipe. The phpunit tests need work, but the pipeline runs and now we can use this as a new downstream branch.
Thanks for the advice. I'm happy with d11-recipe, given that we are introducing this new branch when D11 is the "current" core version. I will push an initial branch, then probably create a MR here too, to adjust the tests as we learn from the changes in MR408
I've pushed two commits. The first one checks for the existence of a recipe.yml file, to allow us to detect that the project is a recipe.
The second commit adds a --verbose
argument in symlink_project.php and displays some values for debug. Previously this script assumed that all projects were located under /web
but this is no longer the case. There may be a better way to pass in the path, rather than derive it in two places. The full path is known in environment variables already, before the script is called. But I suppose we still have to cater for the rare chance that the script is used in a custom job? It would be good if we could assume our scripts are internal and only used in the places specified in the template jobs.
Here's a test with PROJECT_TYPE: 'my-recipe-type'
, which obviously causes all the phpunit jobs to fail, but it shows that setting this variable in .gitlab-ci.yml takes precedence, which is the required outcome
https://gitlab.com/jonathan1055/demo_drupal_gitlab_templates_recipe/-/jo...
For the next test, I have removed PROJECT_TYPE:
from the template, and we see that the correct value 'recipe' has been derived due to the fact that a recipe.yml file exists in the project top-level.
https://gitlab.com/jonathan1055/demo_drupal_gitlab_templates_recipe/-/jo...
I have removed the temporary changes and this is ready for review. In addition to fixing the update_dependencies hook, this MR also sets OPT_IN_TEST_NEXT_MINOR: 1
so that Webform will now always check against the upcoming minor version. I have also marked three tests with @group webform_fail_at_11_3
which will allow quick and easy re-running of that group, when working on the fixes for Core 11.3
The obvious branch name is d10-recipe
to follow the pattern of the existing branches. But when I commit this, we can't change it. Should it be d11-recipe
? or just recipe
? The current branch naming convention is not completely consistent:
- We have two 'd7' branches which run at D7 only, implying that the number in the branch name means 'the current version'.
- The 'd9-basic' branch runs with D10 as current, but used Previous Major to test at D9.
- All the 'd10' branches run with D11 as current, but also some have Previous Major to test at D10.
To demonstate the problem with adding --directory
when also trying to run separate @group
here is the downstream phpunit job - the three separate phpunit job should each run one group (gtd1, gtd2 or gtd3) but they all run all three groups.
I have done more research into recipes (for my own knowledge, as I did not know any of this). It will allow us to make better decisions on what needs to be changed.
How and where are recipes installed
Staring on https://www.drupal.org/docs/extending-drupal/drupal-recipes → and clicking on the "Contributing Recipes" button we get https://www.drupal.org/node/add → and under the title of General project we see
A general project is anything that is not a module, theme, or other more-specific project type that is managed on Drupal.org. This could include; JavaScript components, Drush extensions, recipes, and PHP libraries
So a recipe is definitely classed in a total different category from modules, themes and profiles.
Under Installing Recipes → we see
Composer will download Drupal recipes into the /vendor directory by default. An emerging convention for locating Drupal recipes is in the /recipes directory.
...
If you started your project on Drupal 11.2 or later, this is already done for you.
So even though it is not a confirmed standard, this does suggest that we should be using /recipies
at the root level, not any sub-folder of /web
Examples of recipes running PHPUnit tests
From the list of curated recipes → none of the 8 Site starter recipes → even has a .gitlab-ci.yml file, let alone any phpunit tests.
Of the 19 Functionality-specific recipes → only 2 have a .gitlab-ci.yml and run pipelines - these are normalized_responsive_images → which runs linting but no PHPUnit tests (and the last pipeline was run in Jan 2025), and llm_support which I mentioned above. This recipe does have a PHPUnit test and it runs OK without any customisations
In Packages that extend recipes → - none of the 5 have a .gitlab-ci.yml
How the recipes:
property is used
So I wanted to know why the llm_support phpunit test runs OK wihout any pipeline modification, whereas the recipe from @jcandan does need the changes. I've done lots of testing on my fork of the demo project and it comes down to which other recipes are required by the recipe being tested.
Core recipes, stored in /core/recipes
, have:
recipes:
- article_content_type
- comment_base
This implies to me that a single name means "in the same directory"
llm_support has
recipes:
- core/recipes/administrator_role
- core/recipes/content_editor_role
These are the only ones, and the existing phpunit test passes, so they must be detected and checked as valid somehow. The recipe being tested is loaded into web/recipes/custom/{name}
Our demo project has
recipes:
- core/recipes/core_recommended_maintenance
- core/recipes/core_recommended_performance
- drupal_cms_admin_ui
meaning that it needs two core recipes and also a 3rd-party recipe not provided by core. This is what causes the demo job to fail when llm_support runs OK (I used @group demo_d to replicate the llm_support test).
How are recipes searched for and located?
Reading core/lib/Drupal/Core/Recipe/RecipeConfigurator.php shows that if the recipe name includes a / then \Drupal::root()
is prepended before the name. If there is no / then $include_path
is used instead. Now, $include_path
is defined in core/lib/Drupal/Core/Recipe/Recipe.php using $file, which is the path to the recipe.yml
file.
$include_path = dirname($file, 2)
gives the folder above the {name} of the recipe.
So my conclusion from this is that for gitlab pipelines, if the recipe being tested also depends on any 3rd-party recipe (not just core recipes) then the recipe has to be in the same folder as those 3rd-party recipes. This will be $CI_PROJECT_DIR/recipes/{name}
which is where recipes are being loaded, even without any changes I have made in MR408, for pipelines using Drupal Core 11.2 onwards. [side-note: We will also want to check what happens using 'Previous Major' or 'Previous Minor']
Detection that a project is a recipe
We can use the fact that all recipes must have a recipe.yml
top-level file to detect when a project is a recipe. This will avoid the maintainer having to add PROJECT_TYPE: 'recipe'
in their .gitlab-ci.yml
Yes the Composer job create-environment-variables section reads the project's *.info.yml files, but currently we only read the sub-modules files if we don't find a top-level .info.yml, but that could easily be changed.
Sorry for the delay, and for leaving this issue hanging, been off for a few days. I started the MR work, and initially I added an installer-path of $webRoot . '/recipes/contrib/{$name}'
. But after looking at core-recommended composer.json following your comment in #12 I changed it to just 'recipes/{$name}'. I made my own fork of your demo project to test some scenarios, and created a merge request there, which tests MR408.
But then I realised I needed to check out what the best practices are for how recipes should be installed and tested. You say that the $DRUPAL_PROJECTS_PATH
should be ../recipes
which in the pipeline jobs resolves to /builds/project/{myproject}/recipes/{myproject} thus omitting $_WEB_ROOT
. I know recipies are fairly recent things and maybe not many have phpunit tests, but I found a
list of curated recipes →
. I'm not exactly sure how or by whom they are "curated" but it implies that these recipes at least follow some best practices and could be used as good examples to follow?
Specifically I have been trying to find out the recommended installation path for recipe projects, as it is not clear to me that it has to be in project_root/recipes
as opposed to project_root/web/recipes/contrib
. Take the llm_support recipe as an example. They have a .gitlab-ci.yml file and are running phpunit tests, and all works fine without any customisation. See https://git.drupalcode.org/project/llm_support/-/pipelines
This could be good. In addition to the other reasons you list, the fact that "Pipelines will run in the fork project." means that it is safer for maintainers to run a pages job in a MR pipeline that updates the project's documentation pages. At the moment, if this job is triggered on a MR pipeline that was created by a maintainer, then the MR doc changes are written (and therefore publically published) to the "real" live doc site, before the change is committed. This is obviously not ideal, so we have set the pages job not to run in MRs. But that also means we cannot test out the documentation updates.
We have ongoing discussion in 📌 Allow testing documentation pages via MRs Active with several ideas, but I think if the MR pipelines ran in issue fork regardless of the submitters maintainership status, it would solve that problem.
Sorry, I mis-understood the request. I thought you meant comments in issues, but now I see you mean comment about issues, in code files. That's still a bit niche, but I guess it could be useful. How would you report it? And if the comment should stay, would it need a phpcs:ignore
before it?
Comments on issues are really nothing to do with coder, surely? Coder and CodeSniffer check the linting, coding standards and code quality of source code. Did you file this issue in the wrong queue? Maybe the Drupal infrastructure or the Drupalorg D7 customisations?
All the GTD pipelines pass OK, so that's a good start.
Reading the regex pattern sed 's/[^a-zA-Z0-9]//g'
is the idea to allow all alpha-numeric and digit chars and delete everything else? That would have the effect of deleteing quotes (which is good) but it would also allow mod8 ule
to become module
and be valid, as you mentioned before. We need to decide if it is our job to clean up the string, or report the error and fail.
Couple of things that you could change:
- We already have
tr '[:upper:]' '[:lower:]'
to convert the string to lower-case. So you do not need theA-Z
in the regex pattern - I suggest there is no need to allow numerals in the type as that is not currently part of any type string. If digits are allowed in future we can expand the pattern
That's useful thanks. I have now clicked 'get push access' and I can trigger the downstream pipelines (for those projects that I am a maintainer of) so I will do that.
Also, what is the state of your MR where you had the .info.yml with the bad carriage return? Can you re-instate the wrong file, and then we'll test this MR on your project.
Nice work @valic,
For an initial check, are you able to trigger any of the downstream pipelines?
I can't do that as I did not start the main pipeline, but I am a maintainer of Gitlab Templates Downstream → . Let us know which ones you can trigger, as that will be useful info.
Yes, the 's' is needed. But we can keep this issue open for the eventual fix. ie to remove any trailing blankl or new line from project_type
This can't be done at the moment, it needs a slight enhancement to run-test.sh
. The problem is that if --directory
is specified then all tests in that directory are run, even if group name(s) are also passed in the final argument. This can be fixed, I have tried it locally, and I will raise an issue. I think
the Core queue →
is the place where tun-tests.sh issues should be filed? or would it be
infrastructure →
?
I have raised an issue in Gitlab Templates, and will work on that too.
After some investigation, I think we should be able to add the --directory
argument to run-test.sh
in cases. Currenrtly we do this only when the job needs to run all tests. When filtering on a group we then just pass the group. This works OK, but the Test Discovery process checks every test file for the potential @group, including all 3rd-party files in other module. If both parameters are supplied then the Test Discovery only looks in the directory given, and then further filters by the @group. This should be sufficient for contrib tests.
Could there be a rare case where a contrib module wants to run tests in another project? If so, we can have some kind of override to allow all directories to be checked, but this would be off by default.
Setting back to Needs Work, while I investigate the missing class error
PHP Fatal error: Uncaught Error: Class "Drupal\Tests\feeds\Kernel\FeedsKernelTestBase" not found
in /builds/issue/webform-3547627/web/modules/contrib/paragraphs/tests/src/Kernel/Feeds/Target/ParagraphsTest.php:14
This was set to RTBC three months ago, but I would like to suggest a slight extra, to keep the "Test" bullet-point consistent with the "Trait" and "Interface" wording. It needs Accordingly, non-tests should never have the suffix "Test"
I have put this in the issue summary.
That has fixed the small sample of failing tests
https://git.drupalcode.org/issue/webform-3547627/-/jobs/6620075
There are still lots of deprecations, but this is a good step forward.
But interestingly, restoring _PHPUNIT_CONCURRENT: 1
to use run-tests.sh
we get failures. Either there is a problem in the webform code, or paragraphs or run-tests.sh
https://git.drupalcode.org/issue/webform-3547627/-/jobs/6620132#L61
I have skipped some jobs, and selected just a few tests to run, to get quicker results. Interestingly, with the code as-is, we see the phpstan warning:
------ ----------------------------------------------------------------
Line src/EntityStorage/WebformEntityStorageTrait.php (in context of class
Drupal\webform\WebformSubmissionStorage)
------ ----------------------------------------------------------------
85 Return type mixed of method
Drupal\webform\WebformSubmissionStorage::__get() is not covariant
with return type mixed of method
Drupal\Core\Entity\ContentEntityStorageBase::__get().
------ ----------------------------------------------------------------
https://git.drupalcode.org/issue/webform-3547627/-/jobs/6619707#L102
This looks like it is related to the change you suggest.
That's great news, thanks for the info. Independently, and before I saw your result in #12, I wondered if running the pipeline phpunit tests with _PHPUNIT_CONCURRENT: 0
to use the plain phpunit binary might give any different information. The tests have been using _PHPUNIT_CONCURRENT: 1
which runs the core run-test.sh script. And yes it confirms what you discovered - we get hundreds of the error
PHPUnit\Framework\Exception: PHP Fatal error:
Declaration of Drupal\webform\EntityStorage\WebformEntityStorageTrait::__get($name) must be compatible with Drupal\Core\Entity\ContentEntityStorageBase::__get(string $name): mixed in /builds/issue/webform-3547627/src/EntityStorage/WebformEntityStorageTrait.php on line 85
See https://git.drupalcode.org/issue/webform-3547627/-/jobs/6614518#L685
I will make the change you suggest and see what we get.
I have moved webform_update_dependencies()
back to into includes/webform.install.update.inc
so that it is procedural again in this commit, undoing one little part of the changes from on
📌
Convert to OOP Hooks
Active
. This has removed all of errors
LogicException: The hook update_dependencies on class Drupal\webform\Hook\WebformInstallUpdateHooks does not support attributes and must remain procedural.
in the 'next minor' phpunit job and the log size has reduced from 14k lines to 10k. There are still other errors, but they were also present in the initial pipeline phpunit before any changes. So it's not fixed at 11.3 but is better than before.
@demeritcowboy can you re-try using the changes in this MR?
The attributes were added back in April on 📌 Convert to OOP Hooks Active . That issue is long and complex, and MR598 had modifications to 238 files. Many of the changes are correct, I am sure, but maybe this one was not? The pipelines back then were run with the latest 'current' core, which was 11.1. The problem seems to have been introduced in Core changes for 11.3 as the tests pass at 11.2 at the moment. That's why I have add the opt-in for "next minor" in MR742 in this issue.
Both modules have D11 releases. I think you can remove the _LENIENT_ALLOW_LIST
Yes they do. It's strange because the first pipeline (without _LENIENT_ALLOW_LIST
) failed with unresolved requirements:
https://git.drupalcode.org/issue/webform-3547627/-/jobs/6599563#L76
Problem 1
- Root composer.json requires drupal/core-recommended 11.x-dev -> satisfiable by drupal/core-recommended[11.x-dev].
- Root composer.json requires drupal/select2 1.x-dev -> satisfiable by drupal/select2[1.x-dev (alias of dev-1.x)].
- drupal/core[9.0.0-alpha1, ..., 9.0.0-alpha2] require symfony/psr-http-message-bridge ^1.2.0 -> satisfiable by symfony/psr-http-message-bridge[v1.2.0, v1.3.0].
- drupal/core[9.0.10, ..., 9.0.x-dev] require php ^7.3 -> your php version (8.3.25) does not satisfy that requirement.
- drupal/core-recommended 11.x-dev requires drupal/core 11.x-dev -> satisfiable by drupal/core[11.x-dev].
- drupal/select2 dev-1.x requires drupal/core ^9 || ^10 -> satisfiable by drupal/core[9.0.0-alpha1, ..., 9.5.x-dev, 10.0.0-alpha1, ..., 10.6.x-dev].
- symfony/psr-http-message-bridge[v1.2.0, ..., v1.3.0] require php ^7.1 -> your php version (8.3.25) does not satisfy that requirement.
- You can only install one version of a package, so only one of these can be installed: drupal/core[8.0.0-beta6, ..., 8.9.x-dev, 9.0.0-alpha1, ..., 9.5.x-dev, 10.0.0-alpha1, ..., 10.6.x-dev, 11.0.0-alpha1, ..., 11.x-dev].
- You can only install one version of a package, so only one of these can be installed: drupal/core[8.7.0-alpha1, ..., 8.9.x-dev, 9.0.0-alpha1, ..., 9.5.x-dev, 10.0.0-alpha1, ..., 10.6.x-dev, 11.0.0-alpha1, ..., 11.x-dev].
- drupal/select2 1.x-dev is an alias of drupal/select2 dev-1.x and thus requires it to be installed too.
Reading the full list, it seems the bit that caused the problem was:
- drupal/select2 dev-1.x requires drupal/core ^9 || ^10 -> satisfiable by drupal/core[9.0.0-alpha1, ..., 9.5.x-dev, 10.0.0-alpha1, ..., 10.6.x-dev].
we only see ^9 || ^10 there is no ^11 which is surprising.
I have created a webform issue 📌 LogicException: The hook update_dependencies does not support attributes Active and demonstrated the problem by runing a pipeline at Next Minor. The error is the same as we see here.
I have created MR742 to demonstrate the problem by running against 11.3-dev, using OPT_IN_TEST_NEXT_MINOR: 1
. The PHPUnit Next Minor job fails with the same error.
I've made the change to add $webRoot . '/recipes/contrib/{$name}'
as an additional installer-path
@jcandan Is it OK if I use your demo project https://gitlab.com/jcandan/demo_drupal_gitlab_templates_recipe to test MR408 ?
From #5
if you only specify the
PROJECT_TYPE: 'recipe'
CI variable, a dev would also need to update the Composerinstaller-paths
... otherwise, dependencies will be in ../recipes, ...would get"recipe does not exist"
.
So another way to solve this could be to add another value in the installer-paths array in scripts/expand_composer_json.php for the recipes folder. Then if the project does not have a *.info.yml but you set PROJECT_TYPE: recipe
then it should all flow through correctly?
I have removed the temporary lines and also one intermediate echo. I don't have any other pending changes, so this is ready for feedback and review.
Thank you. Merged and marking fixed. Do you think that one test dependency project is enough? It would be good to have a second one.
I've removed the "core" from the pattern as it is not needed, and now that we have dropped webform in
📌
Webform - LogicException: The hook update_dependencies does not support attributes
Active
the 'phpunit next minor' job is green
https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/65...
I have not removed 'webform' from the deprecation patterns, in case we bring that module back in future.
Merged and fixed.
Yes. I also was thinking about opening an issue in Webform, as this would probably not be compatible with 11.3 when it is released. I know that some projects do not like "chasing core" to remove deprecation, but this error does not seem like a deprecation. I would guess it would at least cause webforms phpunit tests to fail when 11.3 becomes 'current' but I will check that before raising an issue.
With webform removed the phpunit 'next minor' runs OK https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/65...
We get the expected 7 deprecations for search_api, which are beig fixed in MR35 on 📌 Deprecations in 3rd-party modules in Next Minor Drupal 11.3-dev Active
The immediate solution is to drop webform → from the dependency list. Then we can get a green passing pipeline and continue working as before.
We can add it back later, when the problem is fixed in webform and it works with 11.3-dev.
But, if you meant that it would get it from the
PROJECT_TYPE
, it seems this value comes from the*.info.yml
, which Recipes don't have.
Yes I did mean the calculated value, sorry, not the actual default value. I was not aware that recipes do not have a .info.yml (actually, I've not worked on any recipe projects and don't know much about them. Maybe it was an assumption of mine that all D8+ projects should have a *.info.yml)
We have an open issue and MR for documentation updates 📌 Documentation pages Active or you can make a new MR here.
Oh, that is excellent. I didn't know that you could have executable php in an include file
The second suggestion was the solution I thought of when creating this issue, and is also my preferred option. But I recalled the slack conversation
One option could be to just write/append the new patttern into the existing file (which would require determining where the ignoreErrors:
section is, or adding it if none.
However, the phpstan.neon file can also inlcude secondary files, like:
includes:
- phpstan-baseline.neon
- phpstan-baseline-extras.neon
So a second approach could be to add an inlcude:
line and have the extra messages in an asset file in the gitlab_templates project. This way could be clearer to understand and maintain.
ha! Found it! A quick look through the 11.x commit log and I found this commit on 12 September from 🐛 Deprecation warning in DefaultPluginManager::__construct() is too unspecific Active
I suspect that we no longer need the "core" part of the pattern now, so I will remove that and check. It did make me wonder at the time when I was doing the first MR, why we were seeing a deprecation that appeared to be a core problem and that there was no indication that it came from a 3rd-party contrib module. This now all makes sense!
Thanks for replying @karavkars.
As there has been no response from the creator of the issue for a month (since my last comment #8 on 16 Aug) and no response for 3 months (since my request for info in #7) I am closing this issue and the merge request.
Downstream d10-plus with _COMPOSER_YARN_INSTALL=1
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
All looks OK
Downstream d10-theme with _COMPOSER_YARN_INSTALL=0
The following surprised me https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/65...
Displaying the version reverted Yarn to 1.22.22. I obviously don't understand the architecture and what is being done here :-)
Full pipeline https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
OK I will work on that. I don't think its very helpful (indeed could even be more confusing) if the versions are shown in the Composer log when _COMPOSER_YARN_INSTALL=0
because showing that info implies that something has been executed, when in fact nothing has been done. This was my addition, so I will remove it. It may be helpful to instead show a message something like "_COMPOSER_YARN_INSTALL=0 therefore node_modules corepack and yarn install will be done automatically in the jobs that need it. See https://project.pages.drupalcode.org/gitlab_templates/jobs/composer/#whe... for more details"
The version info should always be shown in the later jobs that use it, regardless of the value of _COMPOSER_YARN_INSTALL
. The only question is whether corepack enable
should also be run, even if not installing, as that has an impact of the version of Yarn that is shown. But would I be right in thinking that if there is no yarn install
then the version of Yarn is irrelevent?
All jobs are green, specifically the PHPUnit at 'next minor'
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
Ready for review
Thanks for the hint. Just tried that, and we get the same deprecations with PHPUnit 11.5.36 (which I'm not surprised at, but worth eliminating from the equation)
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/j...
I might just have to accept that I don't know why it was all clear on commit on 6th Sept, but by 16th Sept we had new deprecations. The pattern to ignore these was already in the file for the Core deprecations of this format, so it was "active" back on 6th, but just not being reported for webform and search_api.
Thanks @duaelfr
Yes this issue was created in the early days of Gitlab Templates, probably before the pipeline variants were introduced. The variable requested here did actually get added.
The variables list are precisely the ones that the "OPT_IN" variants use. See the project documentaion page gitlab_templates/info/variants/ for more details.
The 13 new deprecations are all in search_api (7) and webform (6) are all of the form
"Not supporting attribute discovery in * is deprecated in drupal:11.2.0"
- The search_api version has not changed, still 1.38.0
- Neither has the webform version 6.3.0-beta4, these are both the same as in 6th Sept.
- The core/.deprecation-ignore.txt did have a recent change but it was not related to this pattern
- I have compared the logs, and the only salient differences are that the Gitlab Runner has been updated 18.3.0 -> 18.3.1 and PHPUnit 11.5.36 -> 11.5.39. Could this have caused the difference in behavior?
I don't think we can easily run phpunit with the earlier version, can we?
The final pipeline in MR33 ended green for 'phpunit next minor' which implied to me that all the deprecations were covered in the new .deprecations-ignore.txt file added here.
However, on running 'phpunit next minor' now, there are more deprecations not ignored and the job ends with the amber warning.
See this scheduled d10-plus branch test from last night
I don't know if these deprecations have been added since the last run on 6th September (probably not), or whether somehow they were there all along but not being reported in the job (more likely, but I don't know why). Either way, patterns for these deprecations can be added easily into the .deprecations-ignore.txt file.
I can see why this difference is happening. The Merge Request has been rebased so the 3523139-test-only-changes branch now includes the two commits made to the project's d10-plus branch (Sep 6 and Aug 21) - see gitlab_templates_downstream-3523139/-/commits/3523139-test-only-changes
But the d10-plus branch in the issue fork does not have these two commits, see gitlab_templates_downstream-3523139/-/commits/d10-plus. So the test-only changes job determines the common ancestor SHA to be 520125b and we get those extra changes in the list.
I ran the job again via UI adding our new variable _TEST_ONLY_DEBUG=1
and get more info in the log
This is aall fine, I don't think there is any other question to solve here. I could update the issue branch, but have not done so, because I want the links above to demonstrate the difference. I am going to remove the temporary code then this will be ready for merging.
Using GTD d10-theme branch with _COMPOSER_YARN_INSTALL=0
, therefore running Drupal 11 for the validation jobs -
The composer job shows yarn version 1.22.22
The eslint job and cspell job show 1.22.22 at the start, but after corepack enable</ code> the yarn version is immediately 4.9.1. So it is nothing to do with the <code>yarn add @gitlab-formatters
in stylelint, which is good in a way, as at least we have consistency over all the validation jobs, even if we don't yet understand why the version is reported differently. I have to admit that I know very little about node_modules and corepack and yarn, so all I'm doing here is providing the info and debug.
Our aim is to have better and clearer info in the logs, so maybe @tom konda who created the request has an idea about what could be going on?
To correct the unwanted installation I have removed the call to *get-node-modules
from the Composer job (as it can't be executed there if you don't want the check on job name). So I added the version display lines directly.
Regarding the changing yarn version, I have added some extra debug to investigate:
In this d10-plus composer job (_COMPOSER_YARN_INSTALL=1
) it seems to be just corepack enable
which causes the newer version as we can see it jumps from 1.22.22 to 4.9.1 right there.
In the linting jobs that do not run corepack enable
(because it is all done in Composer job) the version is reported as 1.22.22 and remains that way. This could be confusing for the original purpose of this issue. Is the version really 1.22.22? See this d10-plus eslint job
In d9-basic (_COMPOSER_YARN_INSTALL=0
) the composer job just shows 1.22.22 because no enable or install is done.
But confusingly, in the d9-basic eslint job, where the enable and install is done we still see version 1.22.22 at the end. Even in the d9-basic stylelint job it is 1.22.22. This could be because this pipeline is running the linting jobs at Drupal 10 not 11. I will run a test on d10-plus with _COMPOSER_YARN_INSTALL=0
to see what we get.
The default value for DRUPAL_PROJECTS_PATH
should already cater for 'recipe' projects - see
https://git.drupalcode.org/project/gitlab_templates/-/blob/main/includes...
I've restored back to before. It means that we have corepack enable && yarn install
in two places, which is what I wanted to avoid, but it's OK, no big deal.
d10-plus pipeline with default
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
I have now commited _COMPOSER_YARN_INSTALL: 0
to the d9-basic branch, and removed the temporoary line
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
This is actually now doing the wrong thing, because I used your suggestion of still calling the reference in the Composer job. So that means that the directory does not exist and it gets created even when not wanted. So I will have to change that again.
Also it is interesting to see that the inital version didslayed for yarn is 1.22.22
as shown in this cspell job, but after yarn install
it shows as 4.9.1
which can be seen in this stylelint job.
I have pushed a modification to allow the get-node-modules
reference to be called also in the Composer jobs. This means that the versions are displayed in the composer log too, and if we ever had to change how the corepack enable and yarn install are done, it would be in one place not two.
GTD d10-plus uses the default _COMPOSER_YARN_INSTALL: 1
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
GTD d9-basic has _COMPOSER_YARN_INSTALL: 0
and we see the different log output
https://git.drupalcode.org/project/gitlab_templates_downstream/-/pipelin...
Reeady for review and feedback. I think it would be good to have the _COMPOSER_YARN_INSTALL: 0
commited into one of the GTD branches permanently, so we cover that scenario always, therefore the chnage in .gitlab-ci.yml here is only temporary.
OK. I have some ideas about consolidating the two places where we do yarn install, and this seems like a good time to do it. It's the weekend, so this can wait till Monday :-) Just letting you know not to commit this MR right now, if that's OK?
Thanks for the quick response. I confirm that this is now fixed.
Good addition.
What happens if _COMPOSER_YARN_INSTALL
is 0 so that it's not installed already in Composer, and gets installed via the modified .get-node-modules
snippet. Do the new outputs still work? Just wondering if we have a downstream project with this setting. If not, it would be useful to do so. Or we can push a temporary change to set _COMPOSER_YARN_INSTALL: 0
in the downstream pipeline definition.
Yes, a name without the issue number might be better. It could be a separate branch in the project, or it could reside in that issue fork as now. A new separate branch in the project has an overhead of maintainance, we already have seven branches. It all depends on if we want a new separate downstream pipeline (as linked in #33 above) or whether we want to incorporate the 'test-only changes' job into the existing d10-plus pipeline. I think with using the _TEST_ONLY_TARGET_BRANCH
variable we may be able to run the job in the existing downstream pipeline (not an issue fork) and compare to another branch. But I'm not certain how that would detect the changes. Have to test it.
I think my preference would be not to have a new (8th) branch in the project just for this, but to have the changes in an issue fork which can be easily updated via the 'update fork' UI button. Actually, I was imagining that we'd just keep the existing GTD MR29 open and use that branch for the test. Then it would be easy to rebase the MR if/when d10-plus received updates.
Checking without the core patch that reverted a test discovery change:
https://git.drupalcode.org/project/scheduler/-/jobs/6514645 5:55 (run-tests.sh), 6:42 (whole job)
https://git.drupalcode.org/project/scheduler/-/jobs/6515061 4:49 (run-tests.sh), 5:34 (whole job)
https://git.drupalcode.org/project/scheduler/-/jobs/6515197 5:53 (run-tests.sh), 6:58 (whole job)
Here are the changes since 8.x-1.5
https://git.drupalcode.org/project/scheduler/-/compare/8.x-1.5...8.x-1.x...
I confirm this is the case on a full-screen laptop (not tried on small-screen phone).
The simple solution would just to not have that button/link when editing the issue summary, as they are likely to be separate tasks. Or have it placed at the end of the page along with the other links.
Milestone moment! We have our first "test-only changes" job being run in a downstream pipeline, triggered from this Gitlab Templates MR :-)
https://git.drupalcode.org/issue/gitlab_templates_downstream-3523139/-/p...
The $CI_PIPELINE_SOURCE
is neither 'web' nor 'merge_request_event', it is 'pipeline'.
For this I added a new GTD downstream pipeline which use the 3523139-test-only-changes
branch in the issue fork. This branch can remain in-situ, and becomes the permanent place where the 'test-only changes' job is run downstream. The content of that branch is not finalised yet - see
✨
Add coverage for test-only changes job
Active
, but at least we now have proved the concept of running 'test-only changes' in a downstream pipeline.
No changes to the Scheduler tests, but things run faster now anyway:
12 Sep
https://git.drupalcode.org/project/scheduler/-/jobs/6513772 4mins 48
https://git.drupalcode.org/project/scheduler/-/jobs/6513931 5 mins 38
https://git.drupalcode.org/project/scheduler/-/jobs/6514358 5 mins 33
This may be due to using a lower value for concurrency threads, see 📌 Lower default concurrency Active which was mered in July 2025.
Made title more specific and added related issue for cross-reference.
Updated title to show the default is still 32, as there was discussion about reducing it in this issue.
For BC, the biggest dependent project is Scheduler Content Moderation Integration → so we'd need to test thoroughly with that. I want to run a test on SCMI anyway using this MR254 before merging, just to make sure.
The test coverage we have in Scheduler is pretty thorough (I have made sure of that over the years) so I'm fairly confident that your changes here are good to go in. Yes a new release is worth it, for these improvements.
Actually, I've changed my view on #12 and maybe it would be better to do the simplification and removal of redundant code in a separate MR in a follow-up issue, and keep MR254 just for the caching improvements. Then it will be easier to see what is being changed/removed later, after this one is committed.
The single fix for this deprecation reduces the number of reported deprecations in the Scheduler group. Previously the 23 tests ranged from 16 to 12 deprecations. The DefaultTime
test had 16, but now has only 15. The message does not appear in any test.
All tests pass at D11, D10 and D9. The D8 single test failure is a known (and unrelated) problem.