Add pcov coverage & experimental skip nova tests on PRs

mikebronner / laravel-model-caching

Eloquent model-caching made easy.

MIT License

2.24k stars 213 forks source link

Add pcov coverage & experimental skip nova tests on PRs #338

Closed dmason30 closed 4 years ago

dmason30 commented 4 years ago

I use this in all my projects and tests with coverage run much faster.

mikebronner commented 4 years ago

@dmason30 Any ideas why travis is failing on PR builds, but not on master? I have the environment variables set up in the repo settings, and they build fine on master.

dmason30 commented 4 years ago

@mikebronner https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions

The most important restriction for pull requests is about secure environment variables and encrypted data.

A pull request sent from a fork of the upstream repository could be manipulated to expose environment variables. The upstream repository’s maintainer would have no protection against this attack, as pull requests can be sent by anyone who forks the repository on GitHub.

They go on to suggest that tests requiring secure env variables should be skipped on a PR build. PHP Unit has the below argument:

https://phpunit.readthedocs.io/en/9.0/textui.html#command-line-options

  --exclude-group ...         Exclude tests from the specified group(s)

I can have a go at adding it to this PR if you want

mikebronner commented 4 years ago

Thanks for finding that :)

dmason30 commented 4 years ago

@mikebronner You want me to add exclude nova tests to travis for PRs only?

mikebronner commented 4 years ago

You think you can do that only for PR tests, and not for the main build? If so, that would be great. If not, let's leave it in for now.

coveralls commented 4 years ago

Coverage decreased (-0.01%) to 92.308% when pulling 84089deb98e0879ee9fc43087a45aaa0569af8e4 on dmason30:switch-to-pcov into a76e9f143905fca08690fdf2421b0132e8881af4 on GeneaLabs:master.

dmason30 commented 4 years ago

Ok so this took a few attempts to get right. Travis now on PRs removes the laravel/nova dependency - will be awkward in future with any Nova related PRs but I cant see another way round it..

I had to add the doctrine/dbal dev dependency for sqlite support otherwise the tests were erroring on this locally and on travis.

If you want the pcov without all the nova stuff feel free to reopen and merge https://github.com/GeneaLabs/laravel-model-caching/pull/339

mikebronner commented 4 years ago

Awesome, thanks for all the work on this!!!