Skip to content

Add exact line coverage report #1704

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

Closed
wants to merge 1 commit into from

Conversation

rwbarton
Copy link
Contributor

The output is intended to be fed into coverage.py, the Python code
coverage tool.

The output is intended to be fed into coverage.py, the Python code
coverage tool.
@rwbarton
Copy link
Contributor Author

This is a slightly cleaned-up version of what I put together for Zulip. For example, see https://coveralls.io/jobs/15343075.


def indentation_level(self, line_number: int) -> Optional[int]:
"""Return the indentation of a line of the source (specified by
zero-indexed line number). Returns None for blank lines or comments."""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to handle certain idioms correctly. Example:

def f():
    x = (
1)  # actually not indented logically
    y = """
foo"""  # again not indented logically
    z = \
"bar"  # again not indented

Not sure how serious these issues are or if it's worth fixing them now, but at least the triple-quoted string case is pretty frequent in some code I work on (though I dislike this "idiom"). We could perhaps use a Python lexer to get more reliable indent level information.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's what the TODO comment above is referring to. Of these cases, the triple-quoted string concerns me most because there is a valid use for not indenting the continuation lines of the string literal. The other two cases are probably uncommon in practice because they're ugly and hopefully rejected by linters.

It wouldn't be too hard to handle these cases correctly, but if anyone knows of an existing Python lexer that we can conveniently use from mypy, I'm all ears.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could consider making typed_ast give you the end line/col number as well as the starting one (because that might be useful later anyway).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking it might be useful for reporting a missing return statement warning, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a quick look at having typed_ast give the ending position, at least for function definitions. It doesn't look that hard, but then I realized I don't even know how to build typed_ast and I don't really want to get into that right now.

Another approach would be to just take the largest (starting) line of a node within a function as the last line of the function. This would be too conservative in some cases (function ends with a multi-line string literal, or a close parenthesis on its own line), but it would be pretty close to correct, and for coveralls (as used by Zulip) I think it wouldn't even matter since coveralls considers continuation lines to be uninteresting anyways. Unfortunately it's not totally trivial to do that either; I might add a default_visit_node method to TraverserVisitor that is called at the start of every method...

@timabbott
Copy link

Just a small note, based on the analysis in https://github.com/zulip/zulip/pull/1558/files, I think that probably we should make this support a COVERAGE_FILE environment variable to match other tools.

@timabbott
Copy link

(Also, would love to see this merged so that Zulip can stop using a fork of mypy for just this feature...)

@rwbarton
Copy link
Contributor Author

I just learned about the existence of lib2to3; does using its parser sound like a reasonable approach?

@gvanrossum
Copy link
Member

gvanrossum commented Aug 10, 2016 via email

@rwbarton
Copy link
Contributor Author

I mean just for the purpose of this line coverage report: re-parse the input file using lib2to3 just to find out exactly where functions end. I'd be happy to try to implement that as long as lib2to3 is not obviously incapable of doing it (I haven't looked into the details at all) or too unstable to use as a dependency of mypy.

@gvanrossum
Copy link
Member

gvanrossum commented Aug 10, 2016 via email

@gvanrossum
Copy link
Member

gvanrossum commented Aug 28, 2016

Closed by #2057.

@gvanrossum gvanrossum closed this Aug 28, 2016
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

Successfully merging this pull request may close these issues.

5 participants