Problem/Motivation

In the test Drupal\Tests\Core\Test\TestDiscoveryTest::testGetTestsInProfiles() an instance of TestDiscovery is created with 3 parameters. The constructor of TestDiscovery only takes 2 parameters.

Proposed resolution

Remove the last parameter in the creation of the instance of TestDiscovery.

Remaining tasks

TBD

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

daffie created an issue. See original summary.

daffie’s picture

Status: Active » Needs review
FileSize
1.08 KB

The fix.

TR’s picture

This bug was introduced by #2863055: Move TestDiscovery out of simpletest module, minimize dependencies when TestDiscovery was moved out of the simpletest module and into core.

In simpletest, the third argument of the TestDiscovery constructor was the module handler. The module handler was used only for the purpose of implementing hook_simpletest_alter(). When that hook was removed by [#2939892], the module handler was no longer needed, so when TestDiscovery was moved to core that third constructor argument was deliberately removed. This was discussed in #2863055: Move TestDiscovery out of simpletest module, minimize dependencies.

Patch #2 is correct but incomplete. There is one additional place in TestDiscoveryTest where the module handler is passed to the constructor when creating a mock TestDiscovery. Here is a new patch that fixes that as well.

Note that #2863055: Move TestDiscovery out of simpletest module, minimize dependencies was made in D8.8, so this bug exists in both D10 and D9. The patch here does apply to D9 as well, with some offset.

Status: Needs review » Needs work

The last submitted patch, 3: 3257485-3-testdiscoverytest.patch, failed testing. View results

TR’s picture

Status: Needs work » Needs review
TR’s picture

daffie’s picture

Status: Needs review » Reviewed & tested by the community

For @TR, was my patch from comment #2: "Patch #2 is correct but incomplete".

The added changes in the patch from comment #3 look good to me.

For me it is RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: 3257485-3-testdiscoverytest.patch, failed testing. View results

TR’s picture

DrupalCI is showing random intermittent failures with layout_builder. These failures are unrelated to this patch. See #3262505: [random test failure] Random error in layout_builder FunctionalJavascript tests

I will continue to re-test to get it to run green, then set the status back to RTBC.

TR’s picture

Status: Needs work » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: 3257485-3-testdiscoverytest.patch, failed testing. View results

daffie’s picture

Status: Needs work » Reviewed & tested by the community

  • catch committed 136f70d on 10.0.x
    Issue #3257485 by TR, daffie: Constructor of Drupal\Core\Test\...
  • catch committed 4c5ba4c on 9.5.x
    Issue #3257485 by TR, daffie: Constructor of Drupal\Core\Test\...
catch’s picture

Version: 10.0.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.0.x and cherry-picked to 9.5.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.