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

@codeCoverageIgnore on a docblock should prevent driver start/stop #3697

Closed
andrewnicols opened this issue May 21, 2019 · 2 comments
Closed
Labels
type/enhancement A new idea that should be implemented

Comments

@andrewnicols
Copy link
Contributor

Q A
php-code-coverage version 6.1.4
PHP version 7.2.18
Driver PHPDBG
Xdebug version (if used) x.y.z
Installation Method Composer
Usage Method PHPUnit
PHPUnit version (if used) 7.5.9

At the moment if you specify @codeCoverageIgnore on an individual test function or testcase class then that entire test or group of tests is not included in coverage generation. However, this only filters the data which is collected, rather than preventing the collection in the first place.

Whilst the collection is still required in some situations, specifically where a single line is ignored (// @codeCoverageIgnore), or a range of lines via the @codeCoverageIgnoreStart and @codeCoverageIgnoreEnd tags, where the entire class or an entire function is ignored then I feel it would be prudent to not start the driver at all.

This becomes a problem when testing large and complex datasets as the fact that the driver is logging at all causes high memory consumption leading to out of memory. To give a more concrete example, we have one dataset related to machine learning where generation of the data consumes 76GB of memory via opcache alone. At the moment our only solutions are to either add a @group tag to the relevant test and ignore it from phpunit when we wish to generate coverage at all, or to add a hack before and after the test to stop/restart the opcache logging if it is enabled.

I'd like to propose that php-code-coverage either:

  • does not start/stop the driver if the @codeCoverageIgnore tag is applied to an entire testcase class, or test function within that testcase class; or
  • a new tag is added to support the same functionality.

I'm aware that xdebug now supports some filters, but unfortunately phpdbg does not do so yet.

If you feel that this would be a beneficial change I'm more than happy to help in its development.

@sebastianbergmann sebastianbergmann transferred this issue from sebastianbergmann/php-code-coverage May 21, 2019
@sebastianbergmann sebastianbergmann added the type/enhancement A new idea that should be implemented label May 21, 2019
@sebastianbergmann
Copy link
Owner

@codeCoverageIgnore etc. are not used in test code but rather in the code that is being tested.

What you want can be achieved with the @coversNothing annotation on a test case class or test method. Any test that has this will not contribute to code coverage and no code coverage data will be collected while it is being executed.

@andrewnicols
Copy link
Contributor Author

Thanks @sebastianbergmann,

It looks like @coversNothing does what I need it to do on the class, but it does not extend to the test method yet.

From what I see, phpunit calls start() on the driver if PHPUnit\Framework\TestResult::isAnyCoverageRequired() returns true. Right now, the logic there has:

  • Any method @covers annotations returns true
  • Any class @coversNothing returns false
  • Else return true

As a result, @coversNothing will still call start() on the coverage driver for test methods defining @coversNothing. You're correct in that it doesn't start the driver for a class annotation.
For methods it is started, but is later ignored during the filter stage.

I also notice that the test for that function (tests/unit/Framework/TestResultTest.php::testCanSkipCoverage) seems to be inverted.
Unless I'm missing something, it is calling TestResult::isAnyCoverageRequired($test) and expecting the result to be the opposite - i.e. the variable is named $canSkipCoverage and the test seems to concur with the naming. The test is expecting CoverageNothingTest to return true to coverage being required, whil

andrewnicols added a commit to andrewnicols/phpunit that referenced this issue May 21, 2019
andrewnicols added a commit to andrewnicols/phpunit that referenced this issue May 21, 2019
andrewnicols added a commit to andrewnicols/phpunit that referenced this issue May 21, 2019
andrewnicols added a commit to andrewnicols/phpunit that referenced this issue May 24, 2019
andrewnicols added a commit to andrewnicols/phpunit that referenced this issue May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement A new idea that should be implemented
Projects
None yet
Development

No branches or pull requests

2 participants