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

Feature request : make pcov.directory load from coverage list in phpunit or a config file #86

Open
Stevemoretz opened this issue Apr 15, 2022 · 8 comments

Comments

@Stevemoretz
Copy link

So far this is the only thing which it's not that great to work with in your extension, adding dpcov.directory manually is a little troublesome you see for instance I use Intellij Idea which let's you run tests with its ui, adding this option every time manually not only is time consuming but it causes a lot of errors by different members of the team.

I'm not really familiar with how php extensions are written but if it is possible to read phpunit.xml config directly in your plugin, using the coverage array of includes could easily get the highest root and that could be set as default, that way we can have a better default directory, don't you think so? Xdebug doesn't have this problem as I tested so it's doing something like that.

Example :
#37
#34

and many more like one of mine :

    <coverage processUncoveredFiles="true" includeUncoveredFiles="true">
        <include>
            <directory suffix=".php">./app/Http/Controllers</directory>
            <directory suffix=".php">./../../../dir/plugins/store_management</directory>
        </include>
        <exclude>
        </exclude>
    </coverage>
@shyim
Copy link

shyim commented Aug 2, 2022

That looks like an issue for phpunit and not pcov. they can ini_set it what they want

@Stevemoretz
Copy link
Author

Sounds logical would you mind reporting this issue to them, so they might act upon it?

@PeterDKC
Copy link

That looks like an issue for phpunit and not pcov. they can ini_set it what they want

Actually this is not a fix for this issue. pcov actively refuses to honor anything except global .ini changes to this value. I have a scenario where I have 6 applications running on local dev that use a grab bag of source directories including multiple source directories, as well as source directories that do not fit any of these names. I have tried setting the pcov.directory in composer.json, as an ini_set in phpunit.xml, on the command line, every option I can think of. Two issues

  1. pcov absolutely positively 110% refuses to honor any of these if it finds a directory in its list of options first whether I want that directory included in the coverage results or not
  2. and more direly, pcov does not appear to have any ability to handle n+1 source directories in execution coverage from what I can determine, something which test suites and alternate coverage drivers handle happily and with next to zero effort on the part of the developer.

These issues make it not fit for purpose when handling more complicated application structures. Dozens and dozens of people have run across this issue across Github and StackOverflow, nobody at first glance has any clue why it isn't just observing what's been executed like every other code coverage driver available, and the message here seems to be, "Ah well, someone else's problem." "Ah well, you're not conforming to these preconceptions of app structure I've assumed are universal. Too bad." Pretty frustrating.

If I had any lick of C experience I'd give it a crack but unfortunately I wouldn't know where to start. For the time being we'll all have to do what loads of people have done which is use ./, which seems like a terrible idea at scale; or switch off to another driver that behaves somewhat rationally.

@bancer
Copy link

bancer commented Oct 25, 2022

Yes, it is very annoying that pcov does not take into account <coverage> paths from phpunit configuration file. The same problem in details is described here - https://stackoverflow.com/questions/68688566/why-is-phpunit-pcov-not-generating-code-coverage-stats.

@Stevemoretz
Copy link
Author

Stevemoretz commented Oct 27, 2022

Even if this can be only fixed through phpunit, you should ask them to support it (as I mentioned earlier) with providing the steps, I haven't tested it myself but @PeterDKC mentions ini_set doesn't even work.

If your library depends on a tool to work, you need to do the compatibility checks for it too. Send a PR, add a feature request, or simply just at least report the issue with steps to fix it...
You can't just leave it to fate hoping it would be fixed someday lol.

@PeterDKC
Copy link

PeterDKC commented Nov 1, 2022

ini_set doesn't even work

Just to be clear about this behavior:

  • global .ini sets it to src/
  • local .ini sets it to app/
  • code that needs covering is in app/ and src/
  • A bunch of 3rd party manually installed code that isn't examined by tests and does not need coverage is in lib/
  • pcov completely ignores the app/ and src/ directories and only looks at lib/ because that's the one it "saw" first.
  • even if you delete lib/ (which you either can't / shouldn't have to), src/ still will never be examined for coverage.

This behavior is incompatible with actual real life application structures, is a complete surprise, and is just really really frustrating to come across. It shouldn't work like this.

@tylernathanreed
Copy link

+1

I just ran into this issue when moving files out of my app folder to modularize it. Code coverage failed to report coverage in my new packages folder. When I told PHPUnit to switch over to Xdebug instead of pcov, it worked like a charm.

Given that Xdebug works out of the box, whereas pcov doesn't, I worry that others like myself are going to keep using Xdebug for its simplicity in terms of integration.

@tvdijen
Copy link

tvdijen commented May 4, 2023

Given that Xdebug works out of the box, whereas pcov doesn't, I worry that others like myself are going to keep using Xdebug for its simplicity in terms of integration.

And they should, since this package is clearly unmaintained. I don't think there's a proper alternative atm

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

6 participants