PestPHP

[julien-boudry]

Apart from that, have you heard of https://pestphp.com/? It seems to be a really nice testing framework, capable of running the standard PHPUnit tests. However, the tests are much nicer to read and write -- at least in my opinion.

Hi @toddy15

This is interesting.
But at the moment, I don't think it would bring any added value to the existing tests which are well mastered. And above all, they are accessible to all, as a standard, which is not the case with PestPHP tests.

At this stage, I prefer to keep everything in PHPUnit.

[julien-boudry]

Hi @toddy15

I have a better look at Pest.
It's elegant, but with a lot of magic, and strange syntax for IDE or analyzer...
But I am not sure that this syntax is better for the very fat Condorcet tests, they have more than 1000 assertions for "only" 310 tests, this elegant syntax can be less intuitive than the traditional syntax with tests functions, class, and use.

But more problematic: Pest does not support separate processes (pestphp/pest#270), and that is used by some tests, they are useful for restoring static properties as default after tests, garbage collector tests, or output buffers.

For these reasons, the initial promise that Pest would run directly from a PHPUnit installation, allowing a step-by-step transition, is false here. Since it raises many errors immediately, even though most tests work.

I must admit that the outputs are attractive. Or the group functionality. Or the direct generation of coverage.

[toddy15]

Hi @julien-boudry,

thanks for the follow-up. I didn't know that the tests use @runInSeparateProcess, but indeed a few tests are failing because of that with PestPHP. So that's currently not an option to switch to PestPHP.

Regards,
Tobias

[julien-boudry]

If someone wants to buy and PR the automated conversion, I promess to have a Look ;)
I'm in a phase of being trendy like a PHP artisan 🧔‍♂️

https://laravelshift.com/phpunit-to-pest-converter

[toddy15]

Hi Julien,

unfortunately, some tests still fail with Pest (but succeed with PHPUnit). The problem is the "run in separate process" feature of PHPUnit which is not supported by Pest. There was a PR trying to implement the feature, but it's not complete and not available in the latest release of Pest (pestphp/pest#285).

So I think that the conversion is still not possible, unless the failing tests are disabled for the time being -- until Pest supports them. From what I see, there are 9 tests which would need to be temporarily disabled.

Apart from that, do you know Rector (https://getrector.com/)? That is a very smart tool to help modernizing PHP code, and there is an ongoing effort to automatically convert PHPUnit tests into Pest tests (https://github.com/rectorphp/rector-pest). The tool would be free, so nobody has to buy the shift. :-)

[julien-boudry]

Hi @toddy15, nice to hear you :)

These 9 tests could maybe be reworked. And then could should without it.
The separate process was a radical and sure way to ensure that static variables must be initialized as their default value without more code. because these 9 tests, change it (like registering fake new votes methods).

I will have a look at this Rector package, thanks! I don't know which of these tools is the best, and 9$ it's not a serious issue.
If (it's a supposition) I migrate the test to pest one day, it will be with the complete syntax and convention update, and not for keeping PHPUnit style test in one folder, pest tests in another.
So I need the best conversion (I don't have the courage to do it by manually)

[julien-boudry]

Ps: the pest-rector code, seem not appear to have been merged in the released Rector code contrary to what is stated on this repo.

[toddy15]

Yes, it makes sense to convert the syntax to Pest as well and use expect() instead of assert().

And the repository rector-pest is fairly new -- from what I can tell, the development started just two months ago in February. So you're right, although it states that it is part of rector, this is not true right now, it hasn't been merged. But I think it would be possible to merge it in a local installation of rector and use those conversion rules nonetheless.

I have currently very limited free time available, but I can try to take a look at the automatic conversion in the next days (or weeks).

The question is, if it would be smart to try to rework those nine tests beforehand, so that they no longer need to be run in separate processes?

[julien-boudry]

Yes, it would be a good start to rework these tests. I'll look into it soon.

Regarding Pest, I haven't really made the final decision:
It's a waste of time, as native PHPUnit works very well and offers more functionality. It avoids the magical things of Pest (which is not bad for tests: being very boring and without surprises), even if it is a bit more syntax heavy.

At the same time, Pest offers something sexy and new. There is little risk that it will be abandoned in the future. And wishing to get more interested in the Laravel ecosystem, it would be a kind of sandbox for me.

So don't bother trying to merge this rector package yet, it will end up doing it by itself in due time (and probably with an Update on the latest versions of Pest)

[julien-boudry]

@toddy15

61d5f29 remove the RunInSeparateProcess strategy in favor of BackupStaticProperties. But does Pest support it? ... I have found a cli option, that applies on all tests, but not as granular (and I don't want a required cli option for success tests).

[LiamM32]

Does this make it take significantly longer to run the tests?

[toddy15]

Hi @julien-boudry,

that's awesome -- now Pest can be used! I've just used the dev-4.5 branch and installed Pest. I get the same amount of tests and assertion like when using the standard PHPUnit. Everything is green and the result from Pest reads "OK (424 tests, 1665 assertions)".

So the switch can be made anytime now.

@LiamM32: No, the time and memory consumption is the same.

[julien-boudry]

In fact, tests are slower (about 500ms, ~25%). I don't know if it's the one thread or the backStaticProperties (I suspect this last one because 500ms is more than the very few tests concerned).

But it's not really a subject. We don't use yet any parrallelisme (we can, with Pest or PHpUnit).

[julien-boudry]

@LiamM32 Ps : this is not related to Pest, performances are pretty the same.
But related to the code change to be compatible with Pest.

But @toddy15 , some test works because they are expressed with the PHPUnit syntax (Pest is a wrapper on top of PHPUnit).
But Pest doesn't have the support of BackupStaticProperties inside its syntax. Or I didn't find it.

[julien-boudry]

Finally, I have found a way to prevent using both RunInSeparateProcess and BackUpStaticProperties except for one last test that still need it.
The performance improvement is huge, all test runs in 1.0s (2.1s on a small VM) with JIT activated.

In the middle age, this morning (4.5 after removing run RunInSeparateProcess but with a lot of BackUpStaticProperties) it was up to 2.5s.

On the v4.4 branch, its 1.8s on the same hardware.

[julien-boudry]

Bug #166

[toddy15]

Just for your information: the new release of Pest (v2.9.0) comes with a plugin which converts from PHPUnit tests to Pest tests.

How cool is that? 🚀

https://pestphp.com/docs/migrating-from-phpunit-guide

[julien-boudry]

Ok, that's cool. Not fixing bugs, fastly ignored from their issue tracker ;/

[julien-boudry]

PHPUnit seems to have a new command line display style equivalent to Pest..

[julien-boudry]

Look like another bug!

[toddy15]

That was quick. 😃

[julien-boudry]

Tests can not be --drift easily from the Experimental/Pest branch.

• Assertations conversion is better, not perfect (the assertNot... are not converted and others).

• Few tests file are converted, because of a crash of PhpParser library:

• Maybe it's related to the use of class constant and class properties, they are not well converted at all and need refactoring.

• Tests group are not converted yet (but should be very soon, it's already on the changelog but not in Packagist)

• Exception expectations are not converted to the Pest style. And probably it's not desirable because they need to be placed with precision.

• RequiresPhpExtension PHPUnit attribute is not supported by Pest? It introduces a regression for testing environment without Sqlite.

• Probably other things about BackUpProperties

• The conversion tools crash before converting a dataProvider, but I'm not really confident!

• Probably other surprises to check.

[julien-boudry]

Basically, if PHPUnit release a Pest-style expectation API management, to write things like expect($condorcetFormat->CandidatesParsedFromVotes)->toBeFalse();, that would be enough for me to ignore Pest, because don't like to much the rest (the magic side with this bind all over the place and closures mania).

[toddy15]

Yes, I can see what you mean. Still sad, because I like the Pest syntax.

[julien-boudry]

And why not? => sebastianbergmann/phpunit#5450

Yes, the expectations API in particular is really nice and I would like to use it. They are also good things with architecture tests or Dataset (useful for the simplest of them).

But when it comes to long and complex tests, I get the impression that the sequence of methods to call on top of test()/it() and the destructuring of the classes make for even more of a mess than the rigid organization by class of PHPUnit
The syntax is especially good for small tests and projects because it's quick to write, explicit, and nice to see; but it can be the opposite on big projects.

So, even though PHPUnit is a big, complex program, following its logic with confidence is pretty natural, even for complex tests.
Whereas with Pest, we're always wondering what obscure magic and binding will happen; we're encouraged to suspect the framework rather than our code (and sometimes with good reason). Pest has a javascript dark side.

[julien-boudry]

If Pest is required by composer.json, we can use the Pest expectation function expect() inside PHPUnit style tests, and run with PHP Unit.
I don't know if it's will be maintained, if it's a good idea... Because rollback could be painful.

[julien-boudry]

pestphp/pest#874
sebastianbergmann/phpunit#5450 (comment)

[julien-boudry]

Experimental/Pest branch now uses the Pest expectations API inside PHPUnit structure style, the best of two worlds!

The conversion is not finished. ~220 assertions (assertSame & assertEquals) resits to my multiples Regex and manuals corrections. It took me a very (very) long time.

I will probably not convert the Exception API, because the Pest way is less featured (doesn't support testing Exception Message) and it is clunky in comparison (need to encapsulate the throw exception inside a closure, the closure inside a function call).

@toddy15 I'm interested in your opinion (or help to finish converting assertion without too many manual actions).

[julien-boudry]

@toddy15 I'm thinking about merging the branch. Lack of some expectations, I don't find an easy way to convert them yet.

[julien-boudry]

https://github.com/pestphp/pest-plugin-drift/issues/33
https://github.com/pestphp/pest-plugin-drift/issues/29
https://github.com/pestphp/pest-plugin-drift/issues/28

[julien-boudry]

pestphp/pest-plugin-drift#38

[julien-boudry]

bb7f6dd Convert most of the tests files to the Pest only syntax

[julien-boudry]

https://github.com/julien-boudry/Condorcet/releases/tag/v4.7.0

The full migration is finished.

[toddy15]

Yay, congratulations! Thanks for your hard work.