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

Implement XML-based reports #713

Closed
wants to merge 27 commits into from
Closed

Implement XML-based reports #713

wants to merge 27 commits into from

Conversation

o11c
Copy link
Contributor

@o11c o11c commented Jun 20, 2015

This implements a new reporting framework and uses it to implement XML reports using lxml. The old HTML report is still available using --old-html-report <dir>.

The new reports implement --xml-report <dir> and --xslt-html-report <dir>, and makes --html-report <dir> an alias for the latter. It is anticipated that --xslt-txt-report <dir> could be written, and also that a 3rd party might write their own XSLT file that combines mypy's report with coverage's report in a single HTML document.

The XML report embeds a reference to the XSLT document, so it can be viewed directly in most browsers. However, WebKit-based browsers (such as Google Chrome) have a bug that prevents this from working on file: URLs. For this and text-based browsers, the XSLT-based html report internally performs the transforms, and also rewrites the extension of the links from .xml to .html. An improved external CSS stlylesheet is used regardless of who does the XSLT step.

A single XML document is constructed in memory (potentially useful directly for unit tests, though this patch doesn't include any) if any of the new transformations are requested, and written or translated during output. If none of the new reports are requested, lxml does not need to be installed.

A very minimal stub for lxml.etree is included at top-level. It should probably go somewhere else, but there is no precedent for a non-standard-library stub.

While writing this, I noticed that it would be useful to use python3.4's enum (which is backported as enum34). This likely requires special support because it uses a metaclass to do weird stuff.

This PR includes the commits from #711 since it should be merged ASAP, and also some fixes to stubgen that could be refactored into a separate PR since I didn't end up using them because they ended up crashing mypy once I started specifying some of the arguments it couldn't detect.

@@ -0,0 +1,91 @@
# Hand-written stub for lxml.etree as used by mypy.report.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that your str annotations could (should?) be AnyStr.

@o11c
Copy link
Contributor Author

o11c commented Jun 23, 2015

Changed str to AnyStr. Didn't test every function, just one, but I assume lxml is consistent.

That reminds me, bytes(foo) should work when type(Foo) has a __bytes__ method, I currently do an unnecessary encode/decode cycle because of its lack.

Also implemented --txt-report (as proof-of-concept; it only outputs a summary, not per-file info) and refactored the argument logic a bit.

@o11c o11c mentioned this pull request Jun 23, 2015
@o11c
Copy link
Contributor Author

o11c commented Jun 23, 2015

travis says: lxml/etree.pyi:17: error: Incompatible types in assignment (expression has type "str", variable has type "bytes")

This is a bug in other parts of mypy, since this code should be okay: def write(method: AnyStr = 'xml): pass

@o11c o11c force-pushed the xml-report branch 2 times, most recently from 8f52f35 to 7baba51 Compare June 30, 2015 00:16
@o11c o11c mentioned this pull request Jun 30, 2015
@o11c
Copy link
Contributor Author

o11c commented Jun 30, 2015

@spkersten Okay, I've figured out that we don't wany AnyStr here, since that's a TypeVar. Instead we want a Union[str, bytes] which is completely different.

@o11c o11c force-pushed the xml-report branch 2 times, most recently from 79f6dc3 to e61e781 Compare June 30, 2015 05:16
@o11c
Copy link
Contributor Author

o11c commented Jun 30, 2015

Ah, I didn't realize there were two copies of typing that had to be changed. That's really bad.

@o11c o11c mentioned this pull request Jul 8, 2015
@o11c o11c force-pushed the xml-report branch 5 times, most recently from e84e4f2 to 124a4b5 Compare August 1, 2015 03:26
This was referenced Sep 24, 2015
@JukkaL
Copy link
Collaborator

JukkaL commented Oct 11, 2015

I'll look at this now that the test driver PR was merged.

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 11, 2015

I rebased and pushed the PR, after having tweaked the CSS a little further.

I find XSLT painful to use and most contributors won't be familiar with it (see https://www.google.com/trends/explore#q=xslt), so I'd like to get rid of it eventually, but we can live with it for a while (I created an issue for it: #909, but it's not really urgent).

@JukkaL JukkaL closed this Oct 11, 2015
@o11c o11c deleted the xml-report branch October 27, 2015 04:42
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.

4 participants