Switch to using run-tests.sh by default

+1 For this change! It really makes a huge difference on modules with a huge set of tests. For a fact, @Berdir was the person who made me aware of the GitLab CI variable to enable concurrent test runs. Been adding that variable to quite some projects now.

Agree that it is the sensible default. The only reason why it's not is because this was added after the plain PHPUnit. For this reason, I don't think that we should just switch it, at least without a proper handling of BC.

Initial MR created. Need to work on BC now. We need to detect if _PHPUNIT_EXTRA is used and if it is, then see if the project has overwritten _PHPUNIT_CONCURRENT. If it hasn't that, means that we need to throw a warning and either fail, or set _PHPUNIT_CONCURRENT to 0 and throw a warning.

Projects using _PHPUNIT_EXTRA: https://git.drupalcode.org/search?group_id=2&repository_ref=main&scope=b...

I suggest creating a _RUN_TESTS_EXTRA variable, empty by default.
Then, I think this could be a way to go.

Almost no template will have this explicitly set, as it is the default so far.
- If _PHPUNIT_EXTRA is not empty, then we need to warn the users that they need to set _PHP_CONCURRENT: 0 if they want to continue using it. We can actually set the value in the .pre job and then display a warning.

- If _RUN_TESTS_EXTRA is empty, and there is value in _PHPUNIT_EXTRA (and _PHP_CONCURRENT is set to 1), we populate it like this: _RUN_TESTS_EXTRA: $_PHPUNIT_EXTRA and display a warning message for users to change to the new format.

This means that the templates have not overridden _PHP_CONCURRENT nor _PHPUNIT_EXTRA. These are the challenging ones, as something that was working might suddenly stop working.

I guess the only way to go about it is to show a warning message about it saying that it has changed. We could potentially run concurrent first, and if these fail, then non-concurrent, and then show the warning to users, but this would mean a double run (money++) and then maintainers might decide not to change it.

I think that the warning at the beginning of the job, right before running the tests, might be enough.

From @berdir via slack: https://drupal.slack.com/archives/CGKLP028K/p1718272288626879

@fjgarlin in case you need more reasons to push the change to using concurrency by default, https://git.drupalcode.org/project/hal/-/merge_requests/7/pipelines is a good example. test execution time goes from 30min to <3min after adding that

If we work on a 2.x effort, we should probably unify and only have one way to run the tests. I guess like core, with "run-tests.sh", but we'd need to coordinate with them to see if this is also the long term goal for running the tests.

Switching this made a huge difference in Experience Builder total run time.
📌 Improve CI pipeline runtime Active

The ideas in #6 are definitely worth trying out. The new _RUN_TESTS_EXTRA variable would be good in any case, so that different options can be set for concurrent and non-concurrent, making it easier to switch (I already have some custom code to change the matrix value if runing concurrent=1, and having a specific "extra" variable for run-tests.sh would simplify that.

The problem will be in the logic about making decisions on what to give a warning about if phpunit_extra is not empty. We may also need to override and set allow_failure:true if we think that we've changed something that could cause the pipeline to fail.

Overall it is worth investigating, and setting out the detailed scenarios to test. I will rebase the MR, as it is 180 commits behind and has a conflict.

I will suggest something along the lines of paratest may be better option for the performance gain and that we phase out the use of run-tests.sh in a v2 in the templates, (or add its as a 3rd option that is preferred).

Especially for those of us who use code coverage that is our primary reason for not using the run-tests.sh job.

I've heard of some issues with paratest in the past though I do not believe I hit any game stoppers in my testing. IIRC I did run across some phpunit options not supported however I believe they all had workarounds. I need to restore working on that project again.

Long term in my eyes run-tests.sh is non-viable, as it is harder to push fixes for.

We've explored paratest in the past, and the gains (and compatibility) weren't that great compared to using run-tests.sh.

I agree that using phpunit is as standard and agnostic as it gets for PHPUnit tests, but the gains of using run-tests.sh are really good as the script is optimised to run Drupal tests.

Long term usage of the script will probably be determined by what core does. If it keeps on being the primary tool to run CI, then it makes sense that we use it too.

This issue is really exploratory for 1.x, we won't move anything unless we have a solid solution, but if it means reducing CI costs by +50% in all contrib (for the PHPUnit jobs), then it's worth exploring.

Thanks for linking the history of this. For completeness 📌 Improve concurrency: replace experimental package with core script Fixed is where we switched from paratest to run-tests.sh

We looked at paratest for core but there are some hard blockers, like lack of support for parallel CI jobs. In the meantime we're slowly modernising run-tests.sh to try to make it more maintainable. 📌 Deprecate TestDiscovery test file scanning, use PHPUnit API instead Active should help to further improve pipeline times by ordering the tests with the most test methods (taking into account data providers) first in a run.