Reduce run-tests.sh complexity in spawning subprocesses

Appears to have merge conflicts

If you are another contributor eager to jump in, please allow the previous poster(s) at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!

Recommended way for testing?

left a comment on the MR but leaving in review for additional eyes,

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.

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.

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.

96 commits behind, tried to rebase, but needed a merge. It was all clean, no conflicts.

Updated IS with notes on the related issues.

Appeared to break phpstan?

Can we just default to --sqlite :memory: now, but keep the switch in case anyone wants to inspect the database later?

Not sure how to test this one :( but the pipeline is all green, let me see if I can get someone.

Sorry which logs do you mean? And not sure how to test contrib

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.

For Contrib testing, I will try to use this MR as a patch in a Gitlab Templates CI pipeline. This does not replace the suggestion in #44, it's an extra. I am interested in getting this done, to unblock #3549601: Allow --directory and @group to work together in run-tests.sh

Following up my comment in #47 I have now tested this MR in a Contrib pipeline, which has quite a few phpunit tests, split over 6 parallel matrix streams. The patching worked on run-tests.sh ok (ignore the fact that it failed on a few of the test files modifed here).

https://git.drupalcode.org/issue/scheduler-3356800/-/jobs/8141884#L201

The tests ran, exactly as before. The log output is identical except for some formatting around

Drupal test run
--------------------------------------------------------------
Drupal Version.......: 11.3.2

The new long line shows that it is running the modified run-tests.sh, because that is one of the changes in the patch. It is nice to see visible proof, to confirm that.

I know that running time was not the primary aim of this MR, but on a sample of one pipeline before and one after, five of the six parallel test groups were faster in the newer version, ranging from 5% to 14% quicker over tests taking 2 mins to 11 mins. One test was slower but this was 30s to 35s so not actually significant. This is only a sample of one, so not statistically definitive, but it is good. At least it seems that the modified script is producing the same outcomes as before

Just in case anyone else would like to try the same as I did on their contrib project, here is the phpunit: before_script customisation I used.

I did my best to study the code since I've never looked at it before. I found a few things and commented on them in the MR. I'm mostly setting the status to Needs Work because I think I found the potential for a fatal error with that $testClass property.

I diffed most of the tests from this MR versus another simple MR that I worked on today. There was effectively no difference between them, which I think is the point. For the most part, all the tests ran (or at least were reported) in the same groups in the exact same order. The Unit test (Core) summary sections showed the classes in different orders, but the details results sections were the ordered the same. I don't know if that's typical or not.

Anyway, it looks to me like all the tests are still running exactly as they should be after the change. I'm going to RTBC it.

In the MR you made the comment that PHPCS is not being run on run-tests.sh and I think the reason is that the core phpcs.xml.dist file does not include sh in the list of extensions, probably because in general .sh files are bash scripts not php. But it seems there was an attempt to include specific .sh files in the check. I don't know if this ever worked, but it certainly does nothing now, at least locally for me, because the list in "extensions" overrides any exceptions here.

Maybe it is worth opening a separate issue for this? If you agree I will do that. The check can be done in a second separate call to phpcs with a custom list of extensions, if there is no appetite to add 'sh' into the extensions list in the file.

I have opened #3568959: PHPCS ignores the attempt to check run-tests.sh
Also adding the meta issue as parent here.

Looks really good - net reduction in code, moves more logic out of run-tests.sh, unblocks more things later too.

Committed/pushed to main and 11.x, thanks!

Thank you @catch.
Merging this has unblocked #3536964: run-tests.sh - segregate command line parsing and use Symfony Console classes