Problem/Motivation

In #3417805: Use Symfony Process to spawn subprocesses in PhpUnitTestRunner we added a new timeout for tests. I think this was incorrect for the following reasons:

  • We have no idea how long contrib and custom tests take (on whichever CI run on or a local machine).
  • Long debug sessions now are impossible
  • The process time last for the whole test - if a test class contains lots of tests this will not allow you to run the whole test locally.

Steps to reproduce

Proposed resolution

  • If we really want to enforce some timeout on tests then it should apply on the test level method.
  • It should be introduced first as a notice so people can adjust things.
  • It should be configurable.

But personally I think we should not do this. CI have timeouts and locally you can kill the test. This is not an improvement.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3420765

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

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
alexpott’s picture

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

I've made this major because this will lead to contrib and custom tests failing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Related issues: +#3417805: Use Symfony Process to spawn subprocesses in PhpUnitTestRunner

mondrake opened merge request !6550

mondrake’s picture

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

Seems OK for now. A possible follow up is to make the timeout dependent on an env variable.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: +PHPUnit 10
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs work

Turns out the default is 60 seconds, which makes all our tests fail now.

Symfony\Component\Process\Exception\ProcessTimedOutException: The process "'/builds/issue/drupal-3420765/vendor/phpunit/phpunit/phpunit' '--log-junit' '/builds/issue/drupal-3420765/sites/default/files/simpletest/phpunit-100.xml' '/builds/issue/drupal-3420765/core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php'" exceeded the timeout of 60 seconds. in /builds/issue/drupal-3420765/vendor/symfony/process/Process.php:1156
mondrake’s picture

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

$process->setTimeout(NULL); should do, for it does not set any time limit to the process execution.

https://github.com/symfony/symfony/blob/7.0/src/Symfony/Component/Proces...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs review » Reviewed & tested by the community

Thanks @mondrake for fixing it to have an unlimited timeout - yeah 60 seconds was just not going to cut it :D

longwave’s picture

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

Committed 75f85d8 and pushed to 11.x. Thanks!

  • longwave committed 75f85d81 on 11.x 
    Issue #3420765 by alexpott, mondrake: Remove new test timeout

longwave closed merge request !6550

mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: Remove new test timeout » Remove PhpUnitTestRunner process execution timeout

Status: Fixed » Closed (fixed)

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