-
Notifications
You must be signed in to change notification settings - Fork 545
ParaTest: leverage WrapperRunner for quicker tests #1115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I see fatal erros in tests I recently added. It looks like these should use separate php namespaces (they overlap across files atm)
Yep, I also saw these numbers. Looks really nice
I think the fix should look like 2e2c62c
I've tried many iterations like that, but still it seems a require is being done twice :\
oh,.. then we should drop this line I guess
Already tried that too, unsuccessfully :\
hmmm all green on my end? - see only a different problem in "old PHPUnit" jobs
Not reliable: WrapperRunner is not deterministic, the two colliding tests can randomly get executed in the same proc or in different procs.
Try that PR locally with the aforementioned command, you'll see it failing:
$ vendor/bin/phpunit --filter '/(value-of-enum|testValueOfEnum)/' --debug
PHP Fatal error: Cannot declare class ValueOfEnumPhpdoc\Country, because the name is already in use in ./phpstan-src/tests/PHPStan/Rules/PhpDoc/data/value-of-enum.php on line 7
Try that PR locally with the aforementioned command, you'll see it failing:
I can reproduce the error locally. with the changes of #1119 the test-suite runs cleanly on my machine.
unrelated question: I am wondering why paratest uses just 6 workers on my laptop, even though it can run 12 threads (6 cores, 12 SMT threads)
I'm blown away by this! On my local machine running the tests just went down from 18s to 11s!
I'm gonna go ahead and merge it, thank you :)
Hi, could you please work out why it doesn't finish on PHP 7.2 on Windows? I reworked how the patch is applied (d99708f) but that shouldn't cause an issue. Thank you.
It always times out after 60 minutes: https://github.com/phpstan/phpstan-src/actions/runs/2048944923
I'm trying if the original works or not: #1130
Didn't help, decided to do this for now: a95ce30
It always times out after 60 minutes
Yup, I saw that, I was planning to tackle it this week 👍
Alright, you can start by reverting the commit that removes testing on Windows 😊 Thanks.
But don't sink too much time into it, it's not worth it 😊 Thanks.
Ok then.
Worth mentioning that I'm the ParaTest maintainer: ping me if you need further help on this topic.
An idea - we could bump PHPUnit to 8.5 if there was a compatible ParaTest version that would work on PHP 7.2. Don't know if it's solve anything though.
maybe we could just invoked paratest without --runner WrapperRunner on php72 ?
I've got the same idea but since I've already got an appetite for a faster pipeline like this, I rather removed testing on Windows on 7.2.
An idea - we could bump PHPUnit to 8.5 if there was a compatible ParaTest version that would work on PHP 7.2. Don't know if it's solve anything though.
When I became the main maintainer, ParaTest already had all the good ideas and stuff wrote, but with very poor CC, bad overall design and zero static analysis. I had to make sharp turns to unburden maintenance costs, so backporting would be very costly.
7.2 seems to go pretty well on Ubuntu: if you are ok to not test against Windows on PHP 7.2, you would ease my life a lot.
Starting from ParaTest 6, I can guarantee pretty decent support even in the future though, for every platform PHPUnit supports too.
Yes, the current status is fine, thank you :) It's my choice to support older PHP versions, I don't want to forward that burden to anyone.
Hi, opened this PR as draft to see if the Windows builds get a significant improvement.