DX: Use ParaUnit to speed up tests by Wirone · Pull Request #6883 · PHP-CS-Fixer/PHP-CS-Fixer

Wirone · 2023-04-04T22:35:18Z

Currently test suites in Github Actions take ages. This PR's goal is reducing time needed for running test suite(s).

Initially started with ParaTest, but after encountering problems with test loader I've changed to ParaUnit.

	Process Lint	Fast Lint
PHPUnit	00:26:54 😩	00:00:56
ParaUnit	00:04:48	00:00:28 ❤️

Wirone · 2023-06-29T16:55:26Z

FYI: Test execution comparison, run locally on MacBook Pro M1, without smoke tests (466 test classes, 33572 tests):

	Process Lint	Fast Lint
PHPUnit	00:26:54 😩	00:00:56
ParaUnit	00:04:48	00:00:28 ❤️

ParaUnit is 2x faster than regular PHPUnit using Fast Linter (FAST_LINT_TEST_CASES=1), and over 5x faster using Process Linter. 5 minutes of whole suite is actually something that can be run locally without much problem, but 27 minutes? Naaahhhhh.

Wirone · 2023-07-03T08:23:26Z

FYI: about 462 files with NO TESTS EXECUTED, example with --debug:

PROCESS STARTED: /app/tests/RuleSet/Sets/PERRiskySetTest.php
'php' '/app/vendor/phpunit/phpunit/phpunit' '--configuration=/app/phpunit.xml' '--extensions=Paraunit\Parser\JSON\TestHook\BeforeTest,Paraunit\Parser\JSON\TestHook\Error,Paraunit\Parser\JSON\TestHook\Failure,Paraunit\Parser\JSON\TestHook\Incomplete,Paraunit\Parser\JSON\TestHook\Risky,Paraunit\Parser\JSON\TestHook\Skipped,Paraunit\Parser\JSON\TestHook\Successful,Paraunit\Parser\JSON\TestHook\Warning' '--group=auto-review' '/app/tests/RuleSet/Sets/PERRiskySetTest.php'


PROCESS TERMINATED: /app/tests/PregTest.php
 - with class name: N/A

PROCESS FULL OUTPUT:

PHPUnit 9.6.9 by Sebastian Bergmann and contributors.

No tests executed!

This test is not a part of auto-review group. So this is just difference between how PHPUnit and ParaUnit prepares test set - former filters actual set before running, while the latter runs everything but for some classes gets filtered result from the upstream.

Let's sum it up:

471 test classes processed
462 files with NO TESTS EXECUTED
9 test classes with @group auto-review

Everything is correct 🙂. It's not optimal, since 98% of test classes in Auto Review job are run for nothing, but the result is OK. Same with tests mentioned here - these 2 classes are from group covers-nothing which is excluded from code coverage.

keradus · 2023-07-03T08:52:34Z

+
+      - name: Collect code coverage
+        if: matrix.calculate-code-coverage == 'yes'
+        run: vendor/bin/paraunit coverage --testsuite coverage --exclude-group covers-nothing --clover build/logs/clover.xml


did you tried to compare results of coverage generated by PHPUnit and by paraunit? what's the difference?

No, I did not verify it, but I'll do it 👍.

In terms of coverage it looks like everything works as it should:

Coverage: 93.15% (23631/25368) for DX: Progress output refactor #6848

Coverage: 94.47% (23942/25344) for this MR

i have the feeling that number should not be different.

The number is different because the code is having global state, e.g. https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/blob/v3.21.1/src/Tokenizer/CT.php#L99-L104 - running it with PHPUnit make that if covered (or not covered) all the time with the same result, ParaUnit make it non-deterministic.

If you run PHPUnit like ./vendor/bin/phpunit --order-by=random you will get slightly different coverage for each run.

keradus · 2023-07-03T08:53:55Z

I find this very confusing. Maybe we could at least document it in codebase (not only in PR) ?

numbers diff is impressive 👍🏻

keradus · 2023-07-03T08:56:05Z

        "symfony/stopwatch": "^5.4 || ^6.0"
    },
    "require-dev": {
+        "facile-it/paraunit": "^1.3 || ^2.0",


tbh, I never heard about it.

when thinking about parallel execution of PHPUnit, somehow I always had in mind https://github.com/paratestphp/paratest , and to my understanding maintainer of it, Slamdunk, is also more close to PHPUnit itself.

would you mind to share this and that about 2 solutions and arguments for decision?

Hi, Paraunit creator here. Paratest was in disuse when I created Paraunit, but Slamdunk took it over later and revived it. Now it has been used into Laravel, hence the growing popularity.

The main difference between the two tools is basically in the approach: Paratest is "hand on", uses @internal components of PHPUnit and has a tighter integration with it (hence some additional capabilities like being able to hook into specific output formats or being able to filter by groups); Paraunit is "hands off", just executes PHPUnit from the outside, hence the simpler integration but less functionality.

BTW I know Slamdunk and he even invited me to co-maintaint Paratest, but I have to admit I'm not so well versed in it.

Thanks for the details!

side note: yeah, it could be interesting to have the parallel executor available, instead of few possibilities. I could suggest one step further - consider to incorporate it to PHPUnit natively [I know Sebastian wanted to have some internal cleanups first, but if I recall right, lot of them already in place]

Wirone · 2023-07-03T09:18:23Z

I find this very confusing. Maybe we could at least document it in codebase (not only in PR) ?

@keradus let's finish #6839, then such change would be transparent to users because Composer script would switch from PHPUnit to ParaUnit and everything would work as we expected 🙂. This is advantage of that PR, which I'm fighting for for months 😆.

Symfony bumped minimal PHP version in 6.1 and to satisfy ParaTest we need to enforce it too.

See issue 749 in `paratestphp/paratest` (link removed in order to not pollute issue with PR references).

coveralls · 2023-07-20T23:51:13Z

Pull Request Test Coverage Report for Build 5474591896

0 of 0 changed or added relevant lines in 0 files are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage increased (+1.3%) to 94.521%

Files with Coverage Reduction	New Missed Lines	%
src/Console/Output/ErrorOutput.php	2	92.42%

Totals
Change from base Build 5474542201:	1.3%
Covered Lines:	24102
Relevant Lines:	25499

💛 - Coveralls

This reverts commit 7d34e69.

Wirone changed the title ~~PoC: Use ParaTest to speed up tests~~ DX: [PoC] Use ParaTest to speed up tests Apr 4, 2023

This comment was marked as outdated.

Sign in to view

Wirone added the topic/tests label Apr 4, 2023

Wirone requested review from keradus and kubawerlos April 4, 2023 23:26

keradus reviewed Apr 5, 2023

View reviewed changes

Comment thread .github/workflows/ci.yml

This comment was marked as outdated.

Sign in to view

This was referenced Apr 5, 2023

Upgrade to PHPUnit 10 #6884

Closed

Test classes are not found under provided paths paratestphp/paratest#749

Closed

DX: Use Fast Linter by default #6890

Closed

Wirone force-pushed the codito/paratest branch 3 times, most recently from b86866b to df6fc50 Compare June 29, 2023 12:08

This comment was marked as outdated.

Sign in to view

Wirone force-pushed the codito/paratest branch from df6fc50 to b3a2051 Compare June 29, 2023 15:24

Wirone changed the title ~~DX: [PoC] Use ParaTest to speed up tests~~ DX: Use ParaUnit to speed up tests Jun 29, 2023

Wirone mentioned this pull request Jun 30, 2023

DX: Fix FileRemovalTest (do not fail when running it standalone) #7104

Merged

Wirone force-pushed the codito/paratest branch from b3a2051 to 467d225 Compare July 2, 2023 08:08

This comment was marked as outdated.

Sign in to view

Wirone force-pushed the codito/paratest branch from 467d225 to 864bf22 Compare July 2, 2023 22:24

Wirone marked this pull request as ready for review July 2, 2023 22:42

Wirone self-assigned this Jul 2, 2023

Wirone marked this pull request as draft July 3, 2023 07:58

Wirone marked this pull request as ready for review July 3, 2023 08:23

keradus reviewed Jul 3, 2023

View reviewed changes

Comment thread .github/workflows/ci.yml Outdated

keradus reviewed Jul 3, 2023

View reviewed changes

Comment thread .github/workflows/ci.yml Outdated

keradus reviewed Jul 3, 2023

View reviewed changes

Wirone requested a review from keradus July 3, 2023 22:01

Wirone added 5 commits July 6, 2023 12:51

PoC: Use ParaTest to speed up tests

adaca94

PoC: Test compatibility with Symfony 6 using ^6.1 and PHP8.1

a445af9

Symfony bumped minimal PHP version in 6.1 and to satisfy ParaTest we need to enforce it too.

PoC: Run tests on PHP8.0 too

5bdba7f

PoC: Use ParaUnit instead of ParaTest

aa859fa

See issue 749 in `paratestphp/paratest` (link removed in order to not pollute issue with PR references).

Revert previous CI matrix

82b04ac

Wirone force-pushed the codito/paratest branch from e702de3 to 82b04ac Compare July 6, 2023 10:52

keradus approved these changes Jul 8, 2023

View reviewed changes

keradus merged commit 7d34e69 into PHP-CS-Fixer:master Jul 8, 2023

Wirone deleted the codito/paratest branch July 8, 2023 23:35

Wirone mentioned this pull request Jul 19, 2023

Missing comments in PRs lemurheavy/coveralls-public#1723

Closed

mvorisek pushed a commit to mvorisek/PHP-CS-Fixer that referenced this pull request Jul 28, 2023

Revert "DX: Use ParaUnit to speed up tests (PHP-CS-Fixer#6883)"

dd1581b

This reverts commit 7d34e69.

mvorisek pushed a commit to mvorisek/PHP-CS-Fixer that referenced this pull request Jul 28, 2023

Revert "DX: Use ParaUnit to speed up tests (PHP-CS-Fixer#6883)"

d79fa93

This reverts commit 7d34e69.

mvorisek pushed a commit to mvorisek/PHP-CS-Fixer that referenced this pull request Jul 28, 2023

Revert "DX: Use ParaUnit to speed up tests (PHP-CS-Fixer#6883)"

f2744ee

This reverts commit 7d34e69.

Wirone mentioned this pull request Aug 3, 2023

CI must dump diffs if strings do not match #7187

Closed

Wirone added the topic/ci Github Actions, tooling label Mar 22, 2024

Uh oh!

Conversation

Wirone commented Apr 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

Wirone commented Jun 29, 2023

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Wirone commented Jul 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

keradus Jul 3, 2023

Choose a reason for hiding this comment

Uh oh!

Wirone Jul 3, 2023

Choose a reason for hiding this comment

Uh oh!

Wirone Jul 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

keradus Jul 4, 2023

Choose a reason for hiding this comment

Uh oh!

kubawerlos Jul 8, 2023

Choose a reason for hiding this comment

Uh oh!

keradus commented Jul 3, 2023

Uh oh!

keradus Jul 3, 2023

Choose a reason for hiding this comment

Uh oh!

Jean85 Jul 3, 2023

Choose a reason for hiding this comment

Uh oh!

keradus Jul 4, 2023

Choose a reason for hiding this comment

Uh oh!

Wirone commented Jul 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Jul 20, 2023

Pull Request Test Coverage Report for Build 5474591896

💛 - Coveralls

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Wirone commented Apr 4, 2023 •

edited

Loading

Wirone commented Jul 3, 2023 •

edited

Loading

Wirone Jul 3, 2023 •

edited

Loading

Wirone commented Jul 3, 2023 •

edited

Loading