Problem/Motivation

Since PHPUnit 10, process isolation must be specified on a single class level or as a global CLI/config switch.

This would cause issues because Drupal needs to run Unit tests NOT in isolation for performance reasons.

For that purpose, workarounds were introduced to programmatically set process isolation in the test constructor.

However, in PHPUnit 12 that is no longer possible as the test constructor can no longer be overridden, and trying to intervene in the setUp() method is already too late for process isolation.

The #[RunTestsInSeparateProcesses] attribute works but needs to be set on each concrete test class, it can not be specified on abstract base classes.

Proposed resolution

In this issue, introduce a check that throws a deprecation if the #[RunTestsInSeparateProcesses] attribute is not specified on the concrete Functional/FunctionalJavascript test class, and fix leftovers.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#14 3550335-nr-bot_u2_cmwrg.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3550335

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

mondrake created an issue. See original summary.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: Check that #[RunTestsInSeparateProcesses] attribute is added to all Functional tests » Ensure that #[RunTestsInSeparateProcesses] attribute is added to all Functional tests

mondrake opened merge request !13413

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Active » Needs review
mondrake’s picture

mondrake
Location 🇮🇹
commented
Parent issue: #3546029: Ensure that #[RunTestsInSeparateProcesses] attribute is added to all Kernel tests » #3547849: Add #[RunTestsInSeparateProcesses] attribute to all Functional/FunctionalJavascript tests
Related issues: +#3546029: Ensure that #[RunTestsInSeparateProcesses] attribute is added to all Kernel tests
smustgrave’s picture

smustgrave commented

Do we have another example of doing such a quick deprecation?

mondrake’s picture

mondrake
Location 🇮🇹
commented

I do not know, but see #3546029-4: Ensure that #[RunTestsInSeparateProcesses] attribute is added to all Kernel tests and following comments.

smustgrave’s picture

smustgrave commented

Left a comment but guess this one needs release manager sign off too.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Parent issue: #3547849: Add #[RunTestsInSeparateProcesses] attribute to all Functional/FunctionalJavascript tests » #3445240: [meta] Add #[RunTestsInSeparateProcesses] attribute to all Kernel and Functional tests
Related issues: +#3547849: Add #[RunTestsInSeparateProcesses] attribute to all Functional/FunctionalJavascript tests
mondrake’s picture

mondrake
Location 🇮🇹
commented
Assigned: Unassigned » mondrake
Status: Needs review » Needs work
mondrake’s picture

mondrake
Location 🇮🇹
commented
Assigned: mondrake » Unassigned
Status: Needs work » Needs review

See #3546029-25: Ensure that #[RunTestsInSeparateProcesses] attribute is added to all Kernel tests.

smustgrave’s picture

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

Marking per the decision made in #3546029: Ensure that #[RunTestsInSeparateProcesses] attribute is added to all Kernel tests

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3550335-nr-bot_u2_cmwrg.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Reviewed & tested by the community

Hey, bot...

catch made their first commit to this issue’s fork.

  • catch committed f3d5a176 on 11.x 
    Issue #3550335 by mondrake: Ensure that #[RunTestsInSeparateProcesses]...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, thanks!

Made a small update to the wording of the deprecation message prior to commit.

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

catch closed merge request !13413

Status: Fixed » Closed (fixed)

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

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

FYI #3556315: Current approach to ensuring RunTestsInSeparateProcesses is added does not work