Problem/Motivation

In #3402007: Fix strict type errors in test modules it was noticed that there are a handful of test modules that don't conform to usual location of MODULE/tests/modules/TEST_MODULE. In #3400334: Add declare(strict_types=1) to all test modules we're trying to target test modules with a wildcard path but it's not finding everything.

Steps to reproduce

Proposed resolution

Move test modules in to MODULE/tests/modules inside the parent module.

Remaining tasks

Decide if this is worthwhile. Is it too disruptive?

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3439800

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:

Comments

mstrelan created an issue. See original summary.

mstrelan’s picture

Issue summary: View changes
Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Looks like an excellent cleanup!

  • larowlan committed edb73610 on 10.3.x
    Issue #3439800 by mstrelan: Standardize location of test modules
    
    (...

  • larowlan committed 5a9ce8cf on 11.x
    Issue #3439800 by mstrelan: Standardize location of test modules
    
larowlan’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed to 11.x and backported to 10.3.x

Thanks

  • larowlan committed 43e3efe0 on 10.3.x
    Revert "Issue #3439800 by mstrelan: Standardize location of test modules...

  • larowlan committed 273d19b0 on 11.x
    Revert "Issue #3439800 by mstrelan: Standardize location of test modules...
larowlan’s picture

Status: Fixed » Reviewed & tested by the community

Discussed this with @alexpott
At the time of committing I did consider this might be disruptive, but alexpott felt it definitely was so on that basis rolling it back

Alex pointed out that at some point we will get to tests/* so the fact these aren't in modules won't matter so much

alexpott’s picture

I don't think we need to do this issue. I think we can just wait to catch the outliers when you change the include to be <include-pattern>*/tests/*</include-pattern>

I think we can go ahead with #3400334: Add declare(strict_types=1) to all test modules and just rename it to something to do with MODULE/tests/modules so we don't reject because it is not ALL test modules.

alexpott’s picture

Status: Reviewed & tested by the community » Needs review

I think the correct status for this is closed won't fix but going to move to needs review to build consensus

mstrelan’s picture

Sounds fine to me, I think we can target */tests/modules/* and */tests/*_test/* and catch the outliers later.

larowlan’s picture

Status: Needs review » Closed (won't fix)

Thanks

nicxvan’s picture

Status: Closed (won't fix) » Needs work

I'd like to reopen this since I had the same question and have had the same grepping issue in the past trying to see what test modules are doing.

Link to discussion in slack https://drupal.slack.com/archives/C079NQPQUEN/p1752356974306669 (Note if you copy this link, open slack and search for the link it will bring you to the discussion rather than linking to the browser).

It was reverted because it was thought to be disruptive, but I would like to reopen that discussion.
First these are test modules not tests, and most test modules do not change very often.

Most of the modules have not been changed in months, and the changes were by people in agreement that they should be moved:
@mstrelan, @acbramely, and myself.

Here is a list of the modules with test modules in non standard locations and the most recent change.

  • config 1 was edited two weeks ago by acbramley most of the rest are 4 months to 1 year
  • dynamic_page_cache 10 months ago
  • file most 4 to 10 months ago two edited 3 days ago
  • filter 1 and 10 months ago
  • language 4 months to 1 year ago
  • menu_link_content 5 months to 1 year ago
  • navigation 3 days ago
  • node 7 months ago
  • options 5 months to 1 year
  • serialization 4 months ago

Notable exceptions
Most seem related to code sniffing updates, but config in particular has a module or two that do update more frequently.
config
file
navigation

All in all, almost all changes to these are bulk updates like conversions or sniffing that would benefit from a standard location.
A one time disruption makes sense.
The other users I see are quietone and borisson_ I could ping them as well.

Another bit of evidence is the majority of these modules the last change was the revert commit in this very issue.

nicxvan’s picture

Version: 10.3.x-dev » 11.x-dev
Status: Needs work » Active
nicxvan’s picture

Title: Standardize location of test modules » [pp-1] Standardize location of test modules
Status: Active » Postponed (maintainer needs more info)
quietone’s picture

Title: [pp-1] Standardize location of test modules » Standardize location of test modules
Status: Postponed (maintainer needs more info) » Active

I wondered what happened to this issue. This change would make it easier to complete some, but not all, of the core coding standards issues. I also think that core should be consistent in structure. We all know how that can benefit searching and organizing work.

quietone’s picture

Title: Standardize location of test modules » [pp-1] Standardize location of test modules
Status: Active » Postponed (maintainer needs more info)

Restoring meta data.

Why is this postponed on #3067979: Exclude test files from release packages?

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.