Conversation
✅ WordPress Plugin Check Report
📊 ReportAll checks passed! No errors or warnings found. 🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check |
…that were causing tests to fail
There was a problem hiding this comment.
A couple of questions and a minor quibble inline.
PHP Unit is generating the files and folders below that will need to be added the ignore file.
tests/_output/html/
tests/_output/php-coverage.xml
I'm able to get the tests running using both Chassis and WP-Env. On WP-Env I'm getting the message Warning: No code coverage driver available so you may need to add something to the config file.
.github/workflows/phpunit.yml
Outdated
| include: | ||
| - php: '8.4' | ||
| wp: latest | ||
| coverage: false |
There was a problem hiding this comment.
This appears to be covered by the matrix, why is it needed?
There was a problem hiding this comment.
Originally I had coverage: true here, so the coverage report would be generated only on PHP 8.4 and WP latest. But our test coverage is currently so low that the report isn't super useful so I changed this to false and yeah, no need to have this now
| $response = file_get_contents( __DIR__ . '/image_analyze.json' ); | ||
| } elseif ( strpos( $url, 'http://e2e-test-image-processing.test/vision/v3.2/generateThumbnail' ) !== false ) { | ||
| $response = file_get_contents( __DIR__ . '../classifai/assets/img/icon256x256.png' ); | ||
| $response = file_get_contents( '/var/www/html/wp-content/plugins/classifai/assets/img/icon-256x256.png' ); |
There was a problem hiding this comment.
As an open source project, I think it would be good to keep this relative to __DIR__ to account for people using different environments to run the tests on.
It's not a hill I will die on though.
There was a problem hiding this comment.
The issue here is it seems this code never worked and we just never realized it. Now that our test environment runs with debug on, this started causing failures and I realized it was broken (and thus fixed it here). It seems the path traversal doesn't work (or I couldn't get it to work) so easy answer was this. But I can also just copy this image into our test directory and load it from there
There was a problem hiding this comment.
I didn't notice it yesterday but I now see the typo in the original code, it was __DIR__' . '../' rather than __DIR__ ' . '/../'
There was a problem hiding this comment.
So there were two typos, a missing slash (__DIR__ . '../classifai vs __DIR__ . '/../classifai) and a missing dash in the filename (icon256x256.png vs icon-256x256.png). I swear I fixed both of those yesterday and tests were still failing locally but trying again today things work so 🤷🏻
In any case, this has been updated now to use the correct image path: 20c50a1
(p.s. thanks for sticking with this one)
Added in 45bd2ce
So this can be fixed by running |
peterwilsoncc
left a comment
There was a problem hiding this comment.
So this can be fixed by running npm run env start -- --xdebug=coverage when we start our test environment. I'm currently not doing that as we're not using the coverage reports for anything (and this causes things to be slightly slower). But if you think having those coverage reports is useful, I can change this.
That makes sense to me.
This looks good to merge.
Description of the Change
Our PHPUnit workflow has started to fail and in investigating that, seems GitHub has updated their default version of Docker that is installed in their runners and that is conflicting with the tool we use to run our tests. That tool hasn't been updated since June 2024 so I'm not confident they'll push out a fix.
Also, there's a better way to run PHPUnit tests now that didn't exist/we didn't know about when we set this up (using
wp-env) so this is a good opportunity to clean up that setup.The following updates are made:
wp-envenvironment to run our PHPUnit tests (and run those on a wider matrix)Integration(previously wasClassifai)Note most of this was copied from the approach we take on the AI Experiments plugin.
How to test the Change
Ensure the PHPUnit GitHub Action runs and passes on this PR
Changelog Entry
Credits
Props @dkotter
Checklist: