sympy / sympy-bot-old

SymPy pull request helper
http://reviews.sympy.org/
Other
24 stars 16 forks source link

Add testing of doctest coverage with --doc-coverage #161

Closed asmeurer closed 11 years ago

asmeurer commented 11 years ago

To run, use --doc-coverage. --doc-coverage automatically enables --build-docs, because the docs must be built to get the Sphinx coverage.

The output so far looks something like this:

SymPy Bot Summary: :white_check_mark: Passed after merging asmeurer/travis-sphinx (2e1817c7a1fb39a0c798d557ad48e54a0cabe5c1) into master (0ec0ba5a953f8c52d971f9bd1aff9c6089b98766). :white_check_mark: Sphinx 1.2b1: pass :red_circle: Doc Coverage: (Failed) 36% (1983 of 5425) of functions have doctests 40% (2221 of 2221) of functions are imported into Sphinx

Still TODO:

asmeurer commented 11 years ago

Any idea why the doctest coverage script doesn't flush with every newline?

asmeurer commented 11 years ago

OK, I added comparison against master (the formatting is still not nice, but I will fix that). Here is an example for https://github.com/sympy/sympy/pull/1993.

SymPy Bot Summary: :white_check_mark: Passed after merging pernici/nthroot (828e3cc6d35f894794a87ef5e922a6412fd57bb4) into master (0ec0ba5a953f8c52d971f9bd1aff9c6089b98766). :white_check_mark: Sphinx 1.2b1: pass :red_circle: Doc Coverage: (Failed) 36% (1985 of 5427) of functions have doctests 40% (2221 of 2221) of functions are imported into Sphinx For comparison, in master, the coverage is 36% (1983 of 5425) of functions have doctests 40% (2221 of 2221) of functions are imported into Sphinx

asmeurer commented 11 years ago

OK, images added. It now looks like

SymPy Bot Summary: :white_check_mark: Passed after merging pernici/nthroot (828e3cc6d35f894794a87ef5e922a6412fd57bb4) into master (00a0a1e9d27c3f57ed4acd96c20b4c7374078099). :white_check_mark: Sphinx 1.2b1: pass :red_circle: Doc Coverage: (fail) 36.60% of functions have doctests (compared to 36.58% in master) 40.89% of functions are imported into Sphinx (compared to 40.91% in master)

Or if the coverage remains unchanged

SymPy Bot Summary: :white_check_mark: Passed after merging pernici/nthroot (828e3cc6d35f894794a87ef5e922a6412fd57bb4) into master (00a0a1e9d27c3f57ed4acd96c20b4c7374078099). :white_check_mark: Sphinx 1.2b1: pass :red_circle: Doc Coverage: (fail) 36.58% of functions have doctests (compared to 36.58% in master) 40.91% of functions are imported into Sphinx (compared to 40.91% in master)

asmeurer commented 11 years ago

OK, this is now ready for testing, I think. I'm running sympy-bot review mergeable --doc-coverage.

asmeurer commented 11 years ago

One more change before I test this everywhere. Now the summary says if the coverage overall increased, decreased, or remained unchanged:

SymPy Bot Summary: :white_check_mark: Passed after merging pernici/nthroot (828e3cc6d35f894794a87ef5e922a6412fd57bb4) into master (00a0a1e9d27c3f57ed4acd96c20b4c7374078099). :white_check_mark: Sphinx 1.2b1: pass Doc Coverage: decreased 36.60% of functions have doctests (compared to 36.58% in master) 40.95% of functions are imported into Sphinx (compared to 40.97% in master)

SymPy Bot Summary: :white_check_mark: Passed after merging asmeurer/travis-sphinx (2e1817c7a1fb39a0c798d557ad48e54a0cabe5c1) into master (00a0a1e9d27c3f57ed4acd96c20b4c7374078099). :white_check_mark: Sphinx 1.2b1: pass Doc Coverage: unchanged 36.58% of functions have doctests (compared to 36.58% in master) 40.97% of functions are imported into Sphinx (compared to 40.97% in master)

asmeurer commented 11 years ago

I'm wondering if we should change -D from --build-docs to --doc-coverage, so that the default behavior is to also check the coverage.

asmeurer commented 11 years ago

I finished funning the tests. Only one pull request resulted in an increase in both kinds of coverage. Probably about half resulted in unchanged coverage (they did not add or delete functions), and of those that did change, probably about half resulted in an increase in doctest coverage and half in a decrease. Only one pull request resulted in an increase in Sphinx coverage (I could be wrong about that though because there was a bug that made it not notice small changes). So our current coverage percentages should not be surprising.

Some comments on this:

Due to this, I think the increase or decrease arrows are good metrics.

The one pull request that resulted in an increase in Sphinx coverage did so by deleting a function. The coverage script is not run when Sphinx fails, so that could also be part of the issue (whenever something changes Sphinx, there is a high chance it will spuriously fail in my bot).

I think that the doc-coverage code is well tested. I would still like to test running the bot without the bot coverage to make sure that works, but all in all, I'm done with my work here.

flacjacket commented 11 years ago

This looks neat, and it looks good to me. I'll test it out when I'm back home.

asmeurer commented 11 years ago

OK, this has sat here forever, and I've been using it for quite some time without any other comments (positive or negative). I'm going to merge it.