search

wearerequired / traduttore

🗼 A WordPress plugin to improve the I18N workflow for your own projects based on @GlotPress.
https://wearerequired.github.io/traduttore/
72 stars 15 forks source link

Functional tests for WP-CLI commands #124

Closed swissspidy closed 3 years ago

swissspidy commented 5 years ago

Description

This PR sets up the necessary infrastructure for running Behat tests for the built-in WP-CLI commands.

So far this includes only a test for wp traduttore info, but not yet tests for the other commands.

For these, I think we need to figure out how to create a bunch of GlotPress projects with meta data etc. when running the tests. Ideally there would be some proper WP-CLI commands for that, but GlotPress doesn't currently provide any to do such CRUD operations. Perhaps we have to contribute these upstream...

That would make writing these tests way easier. An alternative was mentioned below:

You could just import a SQL dump or create a PHP file which is executed once and creates all the things.

Fixes #21.

How has this been tested?

By writing tests :-)

Types of changes New functional tests for WP-CLI commands.

Checklist:

swissspidy commented 5 years ago

The Travis setup currently fails and I'm not sure why:

$ composer dev-build
> rm -rf vendor/autoload.php
> composer dump-autoload
Generating autoload files
> mkdir -p $(pwd)/build
> wp dist-archive "$(pwd)" "$(pwd)/build/traduttore.zip"
Error:  zip warning: name not matched: traduttore
zip error: Nothing to do! (/home/travis/build/wearerequired/traduttore/build/traduttore.zip)

composer dump-autoload, wp dist-archive "$(pwd)" "$(pwd)/build/traduttore.zip" and wp dist-archive . "$(pwd)/build/traduttore.zip" all work locally.

swissspidy commented 5 years ago

In general, we need to figure out how to create a bunch of GlotPress projects with meta data etc. when running the tests. Ideally there would be some proper WP-CLI commands for that, but GlotPress doesn't currently provide any to do such CRUD operations. Perhaps we have to contribute these upstream...

@ocean90 Can you confirm that last part?

ocean90 commented 5 years ago

All the available CLI commands can be found in https://github.com/GlotPress/GlotPress-WP/tree/develop/gp-includes/cli. Yes, there's nothing for CRUD operations.

Why do you need CLI commands to create the sample data though? You could just import a SQL dump or create a PHP file which is executed once and creates all the things.

swissspidy commented 5 years ago

With all the Behat stuff being in a separate package it's not that straightforward to do so. Plus you'd want to test things like wp traduttore project build --all without any projects around and other edge case scenarios.

Also it would benefit everyone else using GlotPress and WP-CLI 🙂

swissspidy commented 5 years ago

There might be some progress on this in the future, as we're looking into adding such tests for the AMP plugin and evaluating how it can be made easier. https://github.com/ampproject/amp-wp/issues/3079

swissspidy commented 3 years ago

See also https://github.com/ampproject/amp-wp/issues/1815#issuecomment-523073046 and https://github.com/ampproject/amp-wp/issues/3076

Using a phar for Behat sounds more straightforward for now. https://github.com/civicrm/composer-downloads-plugin would easily allow for that. Works very well for including things like PHPStan with a phar too.

schlessera commented 3 years ago

This has been done now for the AMP plugin here: https://github.com/ampproject/amp-wp/pull/6070 (with code coverage still not 100% done).

swissspidy commented 3 years ago

@schlessera Amazing work! 🎉 🚀

swissspidy commented 3 years ago

Updating this PR was really straightforward. The tests passed with the first attempt. Now we'd just have to write tests for the more complex commands :-)

Now just have to figure out how to play nicely with the PHPStan WP-CLI stubs. Removing them completely doesn't work.

swissspidy commented 3 years ago

Now just have to figure out how to play nicely with the PHPStan WP-CLI stubs. Removing them completely doesn't work.

@ocean90 @schlessera any ideas on how to make PHPStan happy here?

ocean90 commented 3 years ago

Maybe @szepeviktor has an idea? https://github.com/wearerequired/traduttore/pull/124/checks?check_run_id=2522558897#step:9:6

szepeviktor commented 3 years ago

Maybe szepeviktor has an idea?

You make me think!

Please send the usual €1 000 000. Thank you.

szepeviktor commented 3 years ago

Cooking something over at #209

szepeviktor commented 3 years ago

@swissspidy Here is another fix to this branch #211

szepeviktor commented 3 years ago

Where does GHA get its config? This does not include "Run tests with coverage" but it is apparently executed. https://github.com/wearerequired/traduttore/actions/runs/903475711/workflow Please advise.

codecov[bot] commented 3 years ago

Codecov Report

Merging #124 (30de83a) into master (1dad289) will not change coverage. The diff coverage is 91.66%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #124   +/-   ##
=========================================
  Coverage     81.59%   81.59%           
  Complexity      369      369           
=========================================
  Files            24       24           
  Lines           913      913           
=========================================
  Hits            745      745           
  Misses          168      168
Flag Coverage Δ
feature ∅ <ø> (?)
php 81.59% <91.66%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
inc/Configuration.php 100.00% <ø> (ø)
inc/Updater.php 93.75% <ø> (ø)
inc/ZipProvider.php 98.76% <ø> (ø)
inc/Plugin.php 46.49% <50.00%> (ø)
inc/Export.php 98.68% <100.00%> (ø)
inc/Project.php 100.00% <100.00%> (ø)
inc/ProjectLocator.php 100.00% <100.00%> (ø)
inc/Runner.php 100.00% <100.00%> (ø)
inc/TranslationApiRoute.php 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1dad289...30de83a. Read the comment docs.

swissspidy commented 3 years ago

So for Behat coverage maybe-generate-wp-cli-coverage.php needs to updated for the right version of the phpunit/php-code-coverage package.

Also, seems a bit wasteful to run the Behat tests against all these configurations.

ocean90 commented 3 years ago

Is the code coverage working? Wondering since there's no change on https://app.codecov.io/gh/wearerequired/traduttore/compare/124.

szepeviktor commented 3 years ago

Is the code coverage working?

The upload step in CI says

Reports have been successfully queued for processing at codecov.io/github/wearerequired/traduttore/commit/0a906c10bb9465558fc77a0cbadac3dfcb8f7706

And Codecov knows about our latest commit https://github.com/wearerequired/traduttore/pull/124/checks?check_run_id=3027581945

swissspidy commented 3 years ago

Is the code coverage working?

The upload step in CI says

Reports have been successfully queued for processing at codecov.io/github/wearerequired/traduttore/commit/0a906c10bb9465558fc77a0cbadac3dfcb8f7706

And Codecov knows about our latest commit #124 (checks)

Plus, locally the clover files are generated just fine and I can see the code coverage for InfoCommand and the like as expected.

Screenshot 2021-07-09 at 12 16 11

There must be something wrong with the paths in there or something.

I'll try to compare the output with the one from PHPUnit.

swissspidy commented 3 years ago

So strange 😕

Setting pcov.directory and pcov.exclude did definitely help and the coverage files now look more or less the same as when using Xdebug.

The only big difference I found between the coverage file from PHPUnit and the one from Behat is the namespace.

The former has namespace="Required\Traduttore" and <package name="Required\Traduttore"> everywhere, the one from Behat has namespace="global".

ocean90 commented 3 years ago

fwiw: @schlessera did disable the codecov upload in https://github.com/ampproject/amp-wp/pull/6070/commits/07a844f4f2ee4a7bfc6e7e00af3a883d9c60d342. The comment for the reason isn't clear to me though.

swissspidy commented 3 years ago

fwiw: @schlessera did disable the codecov upload in ampproject/amp-wp@07a844f. The comment for the reason isn't clear to me though.

Interesting! I don't see why merging wouldn't work though, as we usually merge multiple reports in the web-stories-wp repo and it works just fine there.