Skip to content
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

Measure Code Coverage #2434

Merged
merged 31 commits into from
May 31, 2019
Merged

Measure Code Coverage #2434

merged 31 commits into from
May 31, 2019

Conversation

swissspidy
Copy link
Collaborator

@swissspidy swissspidy commented May 27, 2019

While working on #2408 I realized there was currently no easy way to measure PHP code coverage for the plugin.

With this PR, code coverage analysis is automatically run on Travis CI and then uploaded to Coveralls. This is the only service that wp-dev-lib supports out of the box. I've previously only used Codecov, but I guess they are pretty much the same.

Also, this PR disables Xdebug when no coverage analysis is needed, for a nice little extra boost.

Wanna get some code coverage analysis today? Fear not! It's as easy as following these steps:

  1. Make sure Xdebug is installed and active on your computer. Otherwise use pecl install xdebug
  2. Run phpunit --coverage-clover build/logs/clover.xml. This might take a while.
  3. In IntellJ, open Analyze -> Show Code Coverage Data and select the newly created XML file.
  4. Get visualization like this:

Screenshot 2019-05-27 at 17 42 39

@googlebot googlebot added the cla: yes Signed the Google CLA label May 27, 2019
@swissspidy
Copy link
Collaborator Author

After disabling Xdebug when not needed, the Travis jobs are now super fast: https://travis-ci.org/ampproject/amp-wp/builds/537875524 ⚡️

@swissspidy swissspidy changed the title [WIP] Measure Code Coverage Measure Code Coverage May 28, 2019
@swissspidy swissspidy added this to the v1.2 milestone May 28, 2019
@swissspidy swissspidy requested a review from westonruter May 28, 2019 20:54
@westonruter
Copy link
Member

@swissspidy The code coverage job is failing: https://travis-ci.org/ampproject/amp-wp/jobs/537889379

image

Read environment variables
Dump submitting json file: /tmp/wordpress/src/wp-content/plugins/amp/build/logs/coveralls-upload.json
File size: 2,012.67 kB
Submitting to https://coveralls.io/api/v1/jobs
Client error occurred. status: 422 Unprocessable Entity
Couldn't find a repository matching this job.
elapsed time: 1.488 sec memory: 14.00 MB

@swissspidy
Copy link
Collaborator Author

@westonruter That is to be expected, as the the project hasn’t been set up on Coveralls yet. See todo above:

Verify whether Coveralls can actually be added (GitHub permissions)

I don‘t have the permissions to add the Coveralls GitHub app to this repository. Without that, Coveralls cannot set things up.

@swissspidy
Copy link
Collaborator Author

swissspidy commented May 29, 2019

@swissspidy swissspidy changed the title Measure Code Coverage [WIP] Measure Code Coverage May 29, 2019
@swissspidy
Copy link
Collaborator Author

Just pushed some config changes that should make it work with Codecov. Nice bonus is that Codecov apparently makes it super easy to upload reports for both PHP and JS coverage.

I also requested permissions in order to get access to this repo with Codecov:

Screenshot 2019-05-29 at 11 12 17

Probably necessary because I am only a collaborator on this repo. Maybe you can directly add it without problems, @westonruter.

@swissspidy
Copy link
Collaborator Author

The last build failed because of PHPUnit errors:

PHP Fatal error:  Uncaught Error: Class 'AMP_Base_Sanitizer' not found in /tmp/wordpress/src/wp-content/plugins/amp/tests/test-class-amp-theme-support.php:1907
Stack trace:
#0 phar:///home/travis/phpunit-bin/5.7/phpunit/phpunit/Util/Fileloader.php(52): include_once()
#1 phar:///home/travis/phpunit-bin/5.7/phpunit/phpunit/Util/Fileloader.php(36): PHPUnit_Util_Fileloader::load('/tmp/wordpress/...')
#2 phar:///home/travis/phpunit-bin/5.7/phpunit/phpunit/Framework/TestSuite.php(316): PHPUnit_Util_Fileloader::checkAndLoad('/tmp/wordpress/...')
#3 phar:///home/travis/phpunit-bin/5.7/phpunit/phpunit/Framework/TestSuite.php(393): PHPUnit_Framework_TestSuite->addTestFile('/tmp/wordpress/...')
#4 phar:///home/travis/phpunit-bin/5.7/phpunit/phpunit/Util/Configuration.php(946): PHPUnit_Framework_TestSuite->addTestFiles(Array)
#5 phar:///home/travis/phpunit-bin/5.7/phpunit/phpunit/Util/Configuration.php(843): PHPUnit_Util_Configuration->getTestSuite(Object(DOMElement), NULL)
#6 phar:///home/travis/phpunit-bin/5.7/phpunit/phpunit/TextUI/Com in /tmp/wordpress/src/wp-content/plugins/amp/tests/test-class-amp-theme-support.php on line 1907
Fatal error: Uncaught Error: Class 'AMP_Base_Sanitizer' not found in /tmp/wordpress/src/wp-content/plugins/amp/tests/test-class-amp-theme-support.php on line 1907
Error: Class 'AMP_Base_Sanitizer' not found in /tmp/wordpress/src/wp-content/plugins/amp/tests/test-class-amp-theme-support.php on line 1907

No idea why this happens. Seems like some autoloader issue, but I couldn't reproduce it locally.

See https://travis-ci.org/ampproject/amp-wp/jobs/538685863#L685-L710

@westonruter
Copy link
Member

PHP Fatal error: Uncaught Error: Class 'AMP_Base_Sanitizer' not found in /tmp/wordpress/src/wp-content/plugins/amp/tests/test-class-amp-theme-support.php:1907

@swissspidy This happens when amp.php short-curcuits due to the plugin not having been built, as I see you have discovered!

@westonruter
Copy link
Member

@swissspidy I've put in the request to get the GitHub app installed.

I suppose we need to update wp-dev-lib to enable generation of the badge in readme.md.

@swissspidy
Copy link
Collaborator Author

Yeah wp-dev-lib doesn‘t support codecov yet. Not urgent, as it‘s mostly important for ourselves.

Let‘s hope the request gets through soon. Maybe we can ping some people directly?

.travis.yml Outdated Show resolved Hide resolved
@swissspidy
Copy link
Collaborator Author

The issue with PHP coverage is a bit odd: https://travis-ci.org/ampproject/amp-wp/jobs/539185192#L979-L1007

PHPUnit says a coverage report was built, but Codecov can't find anything at build/logs/clover.xml.

@westonruter
Copy link
Member

westonruter commented May 30, 2019

PHPUnit says a coverage report was built, but Codecov can't find anything at build/logs/clover.xml.

I believe the problem is that the travis.after_script.sh hasn't been run. This is where code coverage is performed. So that needs to be sourced as well, before running bash <(curl -s https://codecov.io/bash) -cF php -f ...

@swissspidy
Copy link
Collaborator Author

Hmm that just uploads the report to Coveralls, no? We already (try to) upload it to Codecov in the script step, so that doesn‘t really apply to this setup.

The actual code coverage analysis is done by PHPUnit while running the tests.

amp.php Outdated Show resolved Hide resolved
@swissspidy swissspidy changed the title [WIP] Measure Code Coverage Measure Code Coverage May 31, 2019
@swissspidy
Copy link
Collaborator Author

swissspidy commented May 31, 2019

Finally! It works! 🥳

Check out the newest code coverage report here: https://codecov.io/gh/ampproject/amp-wp/tree/140e6b3abdd1c86cf10cdbee8c2fa3e19b85824d

It combines both PHP and JS coverage, which is nice.

@westonruter If you wanna review this again, that would be awesome 🙂

Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice persistent work.

@swissspidy swissspidy merged commit 4f8b94f into develop May 31, 2019
@swissspidy swissspidy deleted the add/coveralls branch May 31, 2019 19:07
@amedina
Copy link
Member

amedina commented May 31, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Signed the Google CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants