Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Improvement of @covers and Introduction of @uses #140

Closed
sebastianbergmann opened this Issue · 4 comments

4 participants

@sebastianbergmann

PHPUnit and PHP_CodeCoverage currently support the @covers annotation to denote which unit(s) of code a test wants to cover:

class BankAccount
{
    public function deposit(Money $money)
    {
        // ...
    }
}

class BankAccountTest extends PHPUnit_Framework_TestCase
{
    /**
     * @covers BankAccount::deposit
     */
    public function testMoneyCanBeDepositedInAccount()
    {
        // ...
    }
}

When the @covers annotation is used, any code coverage for code that is not specified using a @covers annotation but executed during the test is discarded. This code should (optionally) be marked as executed (but not as covered / tested) in the HTML code coverage report.

In Strict Mode, PHPUnit 4.0 (PHP_CodeCoverage 2.0) will fail a test that uses the @covers annotation and code is executed that is not specified using a @covers annotation. This leads to a problem when the unit of code under test uses a value object (which does not make sense to stub) as this object, too, would need to be listed using the @covers annotation in order for the test to succeed. Listing the value object using the @covers annotation, however, leads to false code coverage for the value object's code as it is not actually tested in this test.

To solve this problem, a new annotation, @uses, should be added:

class BankAccountTest extends PHPUnit_Framework_TestCase
{
    /**
     * @covers BankAccount::deposit
     * @uses   Money
     */
    public function testMoneyCanBeDepositedInAccount()
    {
        // ...
    }
}

Units of code that are listed using the @uses annotation are allowed to be executed during the test but no code coverage information will be collected for them.

@boenrobot

Wouldn't the name @uses conflict with PhpDocumentor's @uses tag? The @uses is supposed to only list directly called functions and methods (ones that occur within the documented method), while this proposed @uses also seems to imply indirectly called methods.

Perhaps use the name @executes instead?

@edorian
Collaborator

@boenrobot Since the annotations are only on a class/method level for the PHPUnit test cases: Does that really matter? I mean: I've never seen someone generate docs for their test cases (and no one uses @param or @return there? (or the @throws for that matter).

@boenrobot

@param is sometimes used when a test method uses a data provider - it lets IDEs type hint.

Though other than that - yeah, people rarely document their tests, and they don't need to anyway.

Still, it's somewhat confusing to have a dual purpose tag. If we go on that route, how about replacing @exptectedException with @throws? With the exception of "BC concerns", any argument "against" this can be applied to the "@uses vs. @executes (or whatever)" situation as well.

@cbandy

I think distinguishing between covered and executed is interesting, but requiring test annotations that describe things that aren't tested seems like the wrong approach to strictness.

Is it even possible to have "unintentionally covered code" when forceCoversAnnotation is enabled? A test listener can ensure that tests describe what they intend to cover:

class TestListener implements PHPUnit_Framework_TestListener {
  ...
  public function endTest(PHPUnit_Framework_TestCase $test, $time) {
    $annotations = $test->getAnnotations();
    if (empty($annotations['class']['covers'])
      && empty($annotations['class']['coversNothing'])
      && empty($annotations['method']['covers'])
      && empty($annotations['method']['coversNothing'])) {
      $test->getTestResultObject()->addFailure(...);
    }
  }
}

I believe these two existing features offer the same amount of proposed strictness with less noise in tests.

@mwjames mwjames referenced this issue in SemanticMediaWiki/SemanticMediaWiki
Closed

PHPUnit 4.x, @covers, and coverage report generation #280

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.