Problem/Motivation

Follow-up #3151094: Replace use of whitelist/blacklist in \Drupal\Core\Template classes and their tests, pointed by @alexpott.

In fact we should file a follow-up to move this annotation to the class because there are so many statics involved with the Settings.

@runInSeparateProcess
Indicates that a test should be run in a separate PHP process.

@runTestsInSeparateProcesses
Indicates that all tests in a test class should be run in a separate PHP process.

See https://phpunit.readthedocs.io/en/8.5/annotations.html

Proposed resolution

Use @runTestsInSeparateProcesses on the class instead of @runInSeparateProcess on each individual test method.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 3164161-2.runTestsInSeparateProcesses.patch1.15 KBdww
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jungle created an issue. See original summary.

dww’s picture

dww
we/he/they
CreditAttribution: dww as a volunteer commented
Issue summary: View changes
Status: Active » Needs review
Related issues: +#3151094: Replace use of whitelist/blacklist in \Drupal\Core\Template classes and their tests
FileSize
3164161-2.runTestsInSeparateProcesses.patch1.15 KB

Updating the proposed resolution in the summary:

Use @runTestsInSeparateProcesses on the class instead of @runInSeparateProcess on each individual test method.

Seems to be working as intended:

Before

% ./vendor/bin/phpunit --debug -v -c core/phpunit.xml core/tests/Drupal/Tests/Core/Site/SettingsTest.php
...
Time: 7.16 seconds, Memory: 4.00 MB

OK (17 tests, 28 assertions)

After

% ./vendor/bin/phpunit --debug -v -c core/phpunit.xml core/tests/Drupal/Tests/Core/Site/SettingsTest.php
...
Time: 11.85 seconds, Memory: 4.00 MB

OK (17 tests, 28 assertions)

The ~4-5 second increase is consistent, even after a few runs, and has got to be from running all the other tests in separate processes, too.

;)

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

Rationale makes sense, patch looks good.

dww’s picture

dww
we/he/they
CreditAttribution: dww as a volunteer commented
Title: Move the @runInSeparateProcess annotation to the class:SettingsTest » Move the @runInSeparateProcess annotation to the SettingsTest class so it covers all test methods

Cool, thanks for the RTBC, @longwave!

Slightly better title for the commit message.

Crediting @jungle for opening this and initial research, and @longwave for the review.

Cheers,
-Derek

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed edd88ad and pushed to 9.1.x. Thanks!

  • alexpott committed edd88ad on 9.1.x 
    Issue #3164161 by dww, jungle, longwave: Move the @runInSeparateProcess...

Status: Fixed » Closed (fixed)

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