Move traits from core/lib/Drupal/Core/Test to core/tests

Created on 28 August 2023, over 1 year ago
Updated 13 February 2024, 10 months ago

Problem/Motivation

Some code for PHPUnit tests is in core/tests, but some is also lib/Drupal/Core/Test. This is confusing DX.

From Slack:

things currently in lib/Drupal/Core/Test fall into 3 groups: 1. things that should be there, because theyโ€™re used by the SUT, 2. things that could be moved to core/tests, 3. Things that the test runner needs which should be moved somewhere else

For example FunctionalTestSetupTrait and RefreshVariablesTrait look like they could both be moved to core/tests, but a lot of other stuff looks like it can't.

Steps to reproduce

Proposed resolution

Move classes and traits to core/tests.

  • \Drupal\Core\Test\AssertMailTrait
  • \Drupal\Core\Test\FunctionalTestSetupTrait
  • \Drupal\Core\Test\RefreshVariablesTrait
  • \Drupal\Core\Test\TestSetupTrait

Remaining tasks

1. Identify which classes and traits should be moved to core/tests.
2. Create a MR to move them

User interface changes

API changes

Data model changes

Release notes snippet

๐Ÿ“Œ Task
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
PHPUnitย  โ†’

Last updated about 9 hours ago

Created by

๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom joachim

Live updates comments and jobs are added and updated live.
  • Needs change record

    A change record needs to be drafted before an issue is committed. Note: Change records used to be called change notifications.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @joachim
  • First commit to issue fork.
  • Status changed to Needs work 10 months ago
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia mstrelan

    Added some formatting to the IS. Opened an MR to get the ball rolling.

    1. Moved Drupal\Core\Test\FunctionalTestSetupTrait to \Drupal\Tests\FunctionalTestSetupTrait. Arguably it could go in Drupal\Tests\Traits but it's not clear when to use that and when not to. It could also go in Drupal\FunctionalTests but it's also used by \Drupal\TestSite\Commands\TestSiteInstallCommand so I don't know if a more generic namespace is better.
    2. Moved Drupal\Core\Test\RefreshVariablesTrait to Drupal\Tests\RefreshVariablesTrait. Again, it could go in Drupal\Tests\Traits but it seems logical to put it next to UiHelperTrait.
  • Pipeline finished with Failed
    10 months ago
    Total: 197s
    #88266
  • Pipeline finished with Failed
    10 months ago
    Total: 202s
    #88276
  • Pipeline finished with Failed
    10 months ago
    Total: 478s
    #88281
  • Pipeline finished with Success
    10 months ago
    Total: 476s
    #88296
  • Pipeline finished with Failed
    10 months ago
    #88311
  • Pipeline finished with Success
    10 months ago
    Total: 487s
    #88323
  • Status changed to Needs review 10 months ago
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia mstrelan

    I think it's just these three that can move.

    • \Drupal\Core\Test\AssertMailTrait
    • \Drupal\Core\Test\FunctionalTestSetupTrait
    • \Drupal\Core\Test\RefreshVariablesTrait
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia mstrelan

    Probably \Drupal\Core\Test\TestSetupTrait too

  • Pipeline finished with Failed
    10 months ago
    Total: 632s
    #88386
  • Pipeline finished with Success
    10 months ago
    Total: 703s
    #88393
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom catch

    Directly in Drupal\Tests seems OK, we already have lots of traits in there as you point out.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States mile23 Seattle, WA

    I wonder how many contrib tests will break because there aren't deprecated sub-traits remaining... Also DTT might have to do a re-organize.

    Arguably it could go in Drupal\Tests\Traits but it's not clear when to use that and when not to.

    The Traits namespace is there so we can limit the number of autoloaded test files per framework. Since some frameworks share traits, we put them in a separate directory so we can autoload them without autoloading everything else. This only applies to modules, AFAICR. https://git.drupalcode.org/project/drupal/-/blob/11.x/core/tests/bootstr... See also TestSuiteBase and its subclasses. https://git.drupalcode.org/project/drupal/-/blob/11.x/core/tests/TestSui...

    It might help us to do the same thing for non-module core, but I doubt it.

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia mstrelan

    Added a deprecated version of AssertMailTrait, if that looks right I'll add a CR and deprecate the others too. Do we need deprecation tests then too?

  • Pipeline finished with Failed
    10 months ago
    Total: 188s
    #89198
  • Pipeline finished with Success
    10 months ago
    Total: 2358s
    #89202
  • Status changed to Needs work 10 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    Believe we need a CR for the deprecations vs pointing to the ticket

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia mstrelan

    Seeking feedback on #9 but will probably just go ahead with the others.

Production build 0.71.5 2024