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

Support @covers annotation #46

Open
TiMESPLiNTER opened this issue Apr 19, 2021 · 6 comments
Open

Support @covers annotation #46

TiMESPLiNTER opened this issue Apr 19, 2021 · 6 comments

Comments

@TiMESPLiNTER
Copy link

TiMESPLiNTER commented Apr 19, 2021

It would be great to have support for the @covers annotation like PHPUnit has. Ensuring you only cover code with your test that's actually meant to be covered.

I'm fully aware that this might be plenty of work. What do you think? Should this be a feature of this package?

@jaylinski
Copy link
Member

Can you give us an example why this would be useful to you and what problem this would solve? Preferably with a short code-example.

Ref: https://phpunit.readthedocs.io/en/9.5/annotations.html#covers

@TiMESPLiNTER
Copy link
Author

TiMESPLiNTER commented Apr 19, 2021

I don't say this is code you should write but just to show what's potentially the problem without @covers:

final class Something
{
   public function returnSomething(): string
   {
       return 'something';
   }
}

final class TestSubject
{
    private Something $something;

    public function __construct(Something $something)
    {
        $this->something = $something;
    }


    public function doIt(): void
    {
        return $this->something->returnSomething();
    }
}


// PHPspec test
final class TestSubjectSpec extends ObjectBehavior
{
    public function let(): void
    {
        $this->beConstructedWith(new Something());
    }

    public function it_returns_something(): void
    {
        $this->doIt()->shouldBe('something');
    }
}

if I now don't add @covers TestSubject it will also mark lines of the Something class as covered because they get executed. Although my intention wasn't to test that class but only the class TestSubject. The covered lines in Something are kind of an unwanted side-effect.

@jaylinski
Copy link
Member

I think that's hard to implement. We are using phpunit/php-code-coverage in order to generate coverage, but the annotations-parsing seems to happen in phpunit/phpunit: https://github.com/sebastianbergmann/phpunit/blob/dc2d6c4e2fdd9d53606d2a578de0ba65a162ed5b/src/Framework/TestRunner.php#L232-L250

So I think we would have to either hack the phpspec testrunner or use the phpunit testrunner, which seems like a complex task... but maybe I'm wrong.

@TiMESPLiNTER
Copy link
Author

TiMESPLiNTER commented Apr 20, 2021

Out of curiosity I just copied the code section you linked above into the CodeCoverageListener class. And it actually works (of course with code from the phpunit package.) But if we just copy over the code into this package from PHPUnit it would work.

Beside the obvious \PHPUnit\Util\Test class, the package https://github.com/sebastianbergmann/code-unit would also be required.

Here the changed code with which I got the feature working:

namespace FriendsOfPhpSpec\PhpSpec\CodeCoverage\Listener;

class CodeCoverageListener implements EventSubscriberInterface
{
    // ... stripped code
    
    public function afterExample(ExampleEvent $event): void
    {
        if ($this->skipCoverage) {
            return;
        }

        $testFunctionReflection = $event->getExample()->getFunctionReflection();
        $testClassReflection = $event->getSpecification()->getClassReflection();

        try {
            $linesToBeCovered = \PHPUnit\Util\Test::getLinesToBeCovered(
                $testClassReflection->getName(),
                $testFunctionReflection->getName()
            );

            $linesToBeUsed = \PHPUnit\Util\Test::getLinesToBeUsed(
                $testClassReflection->getName(),
                $testFunctionReflection->getName()
            );
        } catch (InvalidCoversTargetException $cce) {
            $result->addWarning(
                $test,
                new Warning(
                    $cce->getMessage()
                ),
                $time
            );
        }

        $this->coverage->stop(true, $linesToBeCovered, $linesToBeUsed);
    }

    // ...
}

@jaylinski
Copy link
Member

jaylinski commented Apr 20, 2021

which seems like a complex task...

I guess I was wrong. 😅

I think it's not worth to include two additional packages just for this feature. But I think we could make this package compatible with @covers. What I mean with this: add the required code as progressive enhancement and suggest the user to install two additional packages (via https://getcomposer.org/doc/04-schema.md#suggest) in order to get annotations-support.

@TiMESPLiNTER
Copy link
Author

TiMESPLiNTER commented Apr 21, 2021

I'll do a PR with a suggestion today so we can discuss it.

See #47

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants