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

Deprecate includeUncoveredFiles configuration option #5951

Closed
nostadt opened this issue Sep 18, 2024 · 5 comments
Closed

Deprecate includeUncoveredFiles configuration option #5951

nostadt opened this issue Sep 18, 2024 · 5 comments
Assignees
Labels
feature/code-coverage Issues related to code coverage (but not php-code-coverage) feature/configuration/xml feature/test-runner CLI test runner type/backward-compatibility Something will be/is intentionally broken
Milestone

Comments

@nostadt
Copy link

nostadt commented Sep 18, 2024

Hi Sebastian,

before I start, I want to give my appreciation for the time and effort you put into this project.

In the code coverage section (v11.3) of the documentation it is mentioned that it is highly recommended to keep the default (true) for includeUncoveredFiles. Otherwise the report is not complete and honest. That made me wonder, why can I disable it at all? Perhaps it can be removed, with the goal to reduce complexity.

Looking forward to hearing from you,

Alexander

@sebastianbergmann
Copy link
Owner

That made me wonder, why can I disable it at all? Perhaps it can be removed, with the goal to reduce complexity.

Good question and excellent point. Thank you for raising it!

Before php-code-coverage used static analysis (using PHP-Parser) to (also) analyse uncovered files, uncovered files were actually loaded using require. This resulted in any code outside of code units such as classes or functions to be executed. This in turn could cause problems and was the reason why it was made optional whether uncovered files should be processed or not.

So, yes, the includeUncoveredFiles option could have been removed a while ago.

@sebastianbergmann sebastianbergmann self-assigned this Sep 18, 2024
@sebastianbergmann sebastianbergmann added type/backward-compatibility Something will be/is intentionally broken feature/test-runner CLI test runner feature/code-coverage Issues related to code coverage (but not php-code-coverage) feature/configuration/xml labels Sep 18, 2024
@sebastianbergmann sebastianbergmann changed the title Question: Can the configuration setting "includeUncoveredFiles" be removed? Deprecate includeUncoveredFiles configuration option Sep 18, 2024
@sebastianbergmann sebastianbergmann added this to the PHPUnit 11.5 milestone Sep 18, 2024
@nostadt
Copy link
Author

nostadt commented Sep 18, 2024

You're welcome. Thank you for the response and happy to hear that it can be removed nowadays.

@mgleska
Copy link
Contributor

mgleska commented Oct 19, 2024

There is one important reason to leave includeUncoveredFiles as it was before - daily work performance.

In my daily work I often run coverage test for only one test file. To check if my recent update of test file covers all lines and branches in tested class.
With includeUncoveredFiles=false, on coverage result index page I get only tested class and a few related files.
With includeUncoveredFiles=true, on coverage result index page I get coverage result for whole system - mostly with negative coverage score.

For very simple system it looks like this:
simple
vs
long

For real (big) system second list will be much longer.
It is easy to see, that on longer list we get some (a lot of) unwanted information, which is useless in this context and disturb quick perception of coverage result of tested class.

@sebastianbergmann Please consider restoring includeUncoveredFiles in config file or adding a CLI option for to enable this functionality.

@sebastianbergmann
Copy link
Owner

@theseer, @localheinz, @staabm, @Schrank, @sebastianheuer, @Tesla91, and I discussed this issue during the PHPUnit Code Sprint in Munich today. We would like to understand why you look at the overview page when you seem to only be interested in the details of a single source file (or a subdirectory of the project root directory).

@mgleska
Copy link
Contributor

mgleska commented Oct 19, 2024

@sebastianbergmann Because I am starting browsing of coverage results from index.html file.

Here is the content of coverage directory from my example (generated without includeUncoveredFiles=false):

/app/coverage # ls -p
Admin/                  CommonInfrastructure/   Kernel.php_branch.html  Printer/                _js/
Api/                    Customer/               Kernel.php_path.html    _css/                   dashboard.html
Auth/                   Kernel.php.html         Order/                  _icons/                 index.html

index.html is a natural starting point.

Now I see, that I can go to CommonInfrastructure directory and open index.html from this location.
Before this discussion I never thought to do so.

OK. I can start browsing in specific directory.
But there is another issue - performance. From my small demo project I get following execution time:

includeUncoveredFiles="false" includeUncoveredFiles="true"
[00:00.070] [00:02.256]
The src directory contains less then 100 files.

Is there any other option to not waste my time, CPU power and energy?
How to not waste resources? I want to save the planet 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/code-coverage Issues related to code coverage (but not php-code-coverage) feature/configuration/xml feature/test-runner CLI test runner type/backward-compatibility Something will be/is intentionally broken
Projects
None yet
Development

No branches or pull requests

3 participants