-
Notifications
You must be signed in to change notification settings - Fork 40
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
CI: Run PHPUnit tests #99
Conversation
@adamziel I just merged another workflow, adding PHPCS checks. |
Yay @aristath! It seems like something still isn't working with PHPUnit in CI, though :( |
@adamziel I fixed the issues. There is now some failing test that - as far as I understand - is a genuine error though 🤔 |
These tests pass on my machine, though, which is super weird. Do they pass on your? |
I have no idea! 😅 I'm unable to run phpunit locally right now, my environment is a bit messed-up |
I made some further updates, and all unit test runs are passing on CI except PHP 7.4. Will try locally with PHP 7.4. |
I'm able to reproduce the failures with PHP 7.4 locally. |
This appears to be due to different tokenization where the translator sees "i" as the table name instead of "information_schema". In PHP 8.3, logging tokens in the lexxer looks like:
But in PHP 7.4, logging the same tokens looks like:
I'm looking into why. If this is indeed an issue, it seems remarkable that we don't have more issues with earlier versions of PHP. |
It was a misplaced paren in the |
6fde36c
to
ef731b2
Compare
ef731b2
to
18c259b
Compare
This reverts commit 18c259b.
The tests are passing after choosing a fixed phpunit version and fixing a str_ends_with typo in the test bootstrap. (And after reintroducing some fixes from @aristath that I had naively reverted) Unfortunately, I have not found a way to test from PHP 7.0 through PHP 8.3. If no phpunit version is specified, there are test errors for PHP 7.0, 8.1, 8.2, and 8.3. If a phpunit version is specified, phpunit 8.x requires at least PHP 7.2, and phpunit 7.x only allows PHP 7.x. There is also the issue that phpunit 8.x requires a void return type on setup methods, but As a compromise, the tests can run on phpunit 8.x for PHP 7.2 through PHP 8.3. Note: With this test configuration, I was able to re-add the consider-X-as-exception test settings for now. |
There are now a ton of commits where I tried various things with the CI workflow in this PR, but the actual changes remain readable. |
Let's go ahead and merge this as the changes are test-related, and CI testing for PHP 7.2 and up is pretty good and better than no CI testing. |
7.2+ is good enough as WordPress phases out support for 7.0 and 7.1. Great work @brandonpayton, thank you! |
No description provided.