Problem/Motivation

Currently there is no good place for test modules for core tests.
There are currently modules in core/tests/Drupal/Tests/Core/Extension/modules/, but this causes confusion with nested PSR-4 directories.

For now I want to use this for #3381785: Add kernel tests for hook system and #3379488: Add kernel tests for event system.

Steps to reproduce

n/a

Proposed resolution

Establish core/tests/$type as the canonical place:

  • core/tests/modules/ for modules.
  • core/tests/themes/ for themes.
  • core/tests/profiles/ for profiles?

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3381933

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

donquixote created an issue. See original summary.

donquixote opened merge request !4607

donquixote’s picture

donquixote commented
Status: Active » Needs review

I think this needs some kind of test, but not sure yet how.
We can add a proof-of-concept module and have a test to discover it.
Will this be enough? Is it necessary?

donquixote’s picture

donquixote commented

Instead of a dedicated test, we could use the two issues I mentioned as proof-of-concept.
If these work, we know that the change is ok.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

I love the idea! If committers agree think we should open a follow up to move the existing modules to the new directory.

donquixote’s picture

donquixote commented
Issue summary: View changes

move the existing modules to the new directory.

Just as a note:
Currently the modules in core/tests/Drupal/Tests/Core/Extension/modules/, in core/tests/Drupal/Tests/Core/Database/fixtures/core/modules/ and in core/tests/fixtures/ are not found by ExtensionDiscovery.
They are used for unit tests where we have to manually add these paths.
Moving them to a place where they are discovered _could_ have side effects that we need to watch out for.
I don't see any name clashes with test modules in core/modules/**/tests/, so I guess it will be fine.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Needs subsystem maintainer review, +Needs framework manager review

I think this addition should have either testing topic maintainer or FM review (tagged both, but really just one or the other would do). Thanks!

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: -Needs framework manager review +Needs change record, +Needs followup

This looks reasonable to me, and is consistent with how we store test modules for core modules.

I have one concern regarding the weight, specifically that we have core/modules at weight 0, ie explicitly the lowest in the priority. This is for example to allow a contrib version of a core module (e.g. CKEditor4, RDF) to take precedence over core when both exist (such as towards the end of Drupal 9).

Adding this new directory at weight 6 means it would take precedence over the other locations. I think instead it should probably be given weight -1 and added before the core directory. So that it gets the lowest precedence.

But in terms of the concept, I think this makes sense.

Is there an existing module in core/tests/Drupal/Tests/Core/Extension/modules/ we can move as part of this issue to provide implicit test coverage?

This will also need a change record and a followup to move any other existing modules.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

nicxvan’s picture

nicxvan commented
Component: base system » extension system
mstrelan’s picture

mstrelan commented

Related #3534377: Consider moving test modules out of system module, unless they relate to system.module