-
-
Notifications
You must be signed in to change notification settings - Fork 515
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
Update integration tests: #2503
Conversation
- Update PHPUnit 🎉 thanks to https://make.wordpress.org/core/2021/09/27/changes-to-the-wordpress-core-php-test-suite/ - Update/fix tests - Test Timber on PHP 8.0 🎉
91f8184
to
fc77cfe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, Nico, this is amazing! 🙌🚀 I read that post and thought that is going to be quite some work. And you did it! Thank you so much for putting in the effort.
The only thing that came to my mind is that the latest
keyword in https://github.com/timber/timber/blob/2.x/bin/install-wp-tests.sh
might not work either. But it doesn’t look like they changed it in https://github.com/wp-cli/scaffold-command/blob/master/templates/install-wp-tests.sh either, yet.
And I wonder whether we need to update https://github.com/timber/timber/blob/2.x/docs/v2/guides/testing.md. Maybe @jarednova could have a look at that.
Both comments could be handled in separate pull requests and shouldn’t hold us back from merging this in so you can continue working, Nico :).
It was not that much, the only thing that took me while to understand is that the polyfilled test suite is only included in branches, not tags...
This is only temporary, the time that scaffold tests are adapted to the new test suite. We could also modify But I think this is fine for now, I'll update these tests when a new WP version comes out or when scaffolded are updates.
I'm pretty sure they should be :) I don't install VVV anymore and it works fine if you have everything setup. And maybe in the future, switch to https://github.com/Yoast/wp-test-utils which eases tests a lot. And split real unit tests from integration tests to speed thing up. But one thing at a time :) |
- Mark PHP 8.0 as non experimental - Add PHP 8.1 as non experimental
- Fix PHP 8.0 tests
c6026ac
to
cf353ff
Compare
- Fix PHP 8.1 tests
69a9832
to
18bf358
Compare
Since PHP 8.1 is out soon... I added some fixes so everything passes tests. Just a tiny question, I had trouble with image resizing when Imagick is available and crop is set to 'default'. What does 'default' means when cropping? I found nothing about it: https://github.com/timber/timber/blob/master/lib/Image/Operation/Resize.php#L119-L149 So I supposed |
So this appears to be a leftover highly opinionated artifact of what I thought a good "default" crop should be circa 2015. The key lines are here ... https://github.com/timber/timber/blob/master/lib/Image/Operation/Resize.php#L116-L117 ... where it centers on the X axis and then 1/6 from the top on the Y axis. A perfectly sensible crop (this would be good for cropping to where faces generally are in photos) — but I can see why we'd want to make that controllable at the least. I'm going to say that for now, rearchitecting crops should be a 2.x future task. I'll review this over the next couple days, formal PHP 8 support is great!!! |
@nlemoine great work! as @gchtr said: Thanks for linking to the WP.org blog post, that answered all the big Qs I had in my review. As @gchtr notes, this is a good time to get to those |
@jarednova I'm still on a mid-2012 macbook pro (non retina version)... Because this is the last one where you can still easily replace/upgrade your hard drive (I replaced the optical drive with a 2To SSD drive), memory and battery. It's running pretty good but it's getting white hairs and I try to avoid VM when I can. So basically, my setup is only The ideal way to run tests is act but it's not supporting services yet. Once it does, running tests will be quite easy and more importantly, we won't have to handle local env and Github Actions env. |
Oh wow, you're going old school! Thanks for the links to act — it's great to have a way to execute those actions locally. I can't tell you how many hours I've spent pushing Travis commits just to see how it would use the build script. |
Timber now officially supports PHP 8.0/8.1 🎉
(I mean if @jarednova is ok with this 😅)
PR summary:
Side notes:
yoast/phpunit-polyfills
handle the PHPUnit version to use as advised in link abovecomposer req phpunit/phpunit:^7.5 -W --dev
but don't commit changes made tocomposer.json
5.8
, notlatest
, not5.7.1
):bash bin/install-wp-tests.sh wordpress_test <db_user> <db_pass> <db_host> 5.8