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

annotate: does not handle if False properly / consistently #659

Open
nedbat opened this issue May 4, 2018 · 6 comments
Open

annotate: does not handle if False properly / consistently #659

nedbat opened this issue May 4, 2018 · 6 comments
Labels
annotate bug Something isn't working

Comments

@nedbat
Copy link
Owner

nedbat commented May 4, 2018

Originally reported by Anonymous


coverage annotate considers lines after if False to be covered:

% python -m coverage run t.py && python -m coverage annotate t.py && python -m coverage report -m && cat t.py,cover
1
Name    Stmts   Miss  Cover   Missing
-------------------------------------
t.py        1      0   100%
> if False:
>     print('never')
> print(1)

I know that if False is a special case (optimization), but AnnotateReporter.annotate_file should handle it properly.

analysis.statements is [3] in this case, that's why the covered logic kicks in later only.

Since covered gets not updated for the if False, it will be considered to be uncovered in the following case, because the previous line sets covered=False:

1
Name    Stmts   Miss  Cover   Missing
-------------------------------------
t.py        4      1    75%   3
> a = 0
> if a:
!     print(0)
! if False:
!     print('never')
> print(1)

@nedbat
Copy link
Owner Author

nedbat commented May 4, 2018

Original comment by Daniel Hahler (Bitbucket: blueyed, GitHub: blueyed)


The report is from me, wasn't logged in.

@nedbat
Copy link
Owner Author

nedbat commented May 4, 2018

Do people really use annotate? It seems really clumsy compared to the HTML report. You are right, this is a bug, but is it worth fixing?

@nedbat nedbat added major bug Something isn't working annotate labels Jun 23, 2018
@blueyed
Copy link
Contributor

blueyed commented Jun 30, 2018

Do people really use annotate?

I find it useful to inspect a single file quickly, but that is currently often related to developing covimerage (which uses coveragepy internally). Therefore not a ocmmon use case.

It's unfortunate that it is a bit fragile then, especially when using it for debugging.

I've found another issue now, which might be related, where a line not being executable gets marked as uncovered. This simple check might also help here.

@blueyed
Copy link
Contributor

blueyed commented Jun 30, 2018

I think factoring out the handling of fr.source_token_lines out of HtmlReporter.html_file and using it also for annotate then would make sense.
It could be generalized to return a list of lines with attributes, suitable for both reporters.

Then I could also imagine to have a new command annotate_file that would output to stdout and use terminal colors (and bold) if printing to a tty.

@blueyed blueyed mentioned this issue Jun 30, 2018
1 task
@blueyed
Copy link
Contributor

blueyed commented Jun 30, 2018

FWIW: the html reporter displays only line 3 as executed (correctly), and 1-2 as not executed (known limitation).

I think it is better to keep the output of annotate and html in sync, so I've created #677.

@nedbat
Copy link
Owner Author

nedbat commented Jul 1, 2018

This is an interesting idea. Instead of "annotate_file", I'd call the command "show". One thing to note: coverage has no prerequisites. It would be tempting to use an existing library for terminal colors, etc, but I would rather not. Also, do we end up in the quagmire of "themes" for the coloring? Somehow it's never been requested for the HTML reports, but I know what color the background will be in the HTML reports. Terminals might not be so compliant.

@nedbat nedbat removed the 4.5 label Aug 17, 2018
@nedbat nedbat removed the major label Jan 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
annotate bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants