On Tue, 25 Apr 2017 at 17:00 Terry Reedy <tjre...@udel.edu> wrote: > On 4/25/2017 11:00 AM, Barry Warsaw wrote: > > On Apr 24, 2017, at 09:32 PM, Ethan Furman wrote: > >>On 04/21/2017 03:29 PM, Victor Stinner wrote: > > (In the context of having a patch blocked by the blind Codecov robot ...) > > >>> I dislike code coverage because there is a temptation to write > artificial > >>> tests whereas the code is tested indirectly or the code is not > important > >>> enough to *require* tests. > > While I use code coverage to improve automated unittesting, I am opposed > to turning a usable but limited and sometime faulty tool into a blind > robotic master that blocks improvements. The prospect of this being > done has discouraged me from learning the new system. (More on 'faulty > tool' later.) >
It should be stated that code coverage is not a blocking status check for merging from our perspective (the **only** required check is that Travis pass with it's test run). The only reason Victor ran into it at a blocker is because he tried to do a merge over his phone where GitHub apparently prevents merging if any failing check exists, required or not (I assume this is because reviewing code on a small screen raises the chances of overlooking a failing check that one actually cared about). I'm on vacation until tomorrow so I've been off of GitHub since last Thursday, but it's possible there's already a PR out there to turn off the status checks. If there is still no such PR I'm still fine with turning off the status checks for Codecov. > > The temptation to write artificial tests to satisfy an artificial goal > is real. Doing so can eat valuable time better used for something else. > For instance: > > def meth(self, arg): > mod.inst.meth(arg, True, ob=self, kw='cut') > > Mocking mod.class.meth, calling meth, and checking that the mock is > called will satisfy the robot, but does not contribute much to the goal > of providing a language that people can use to solve problems. > My assumption is that there will be a test that meth() does the right thing, mock or not. If we provide an API there should be some test for it; using a mock or something else to do the test is an implementation detail. > > Victor, can you explain 'tested indirectly' and perhaps give an example? > > As used here,'whereas' is incorrect English and a bit confusing. I > believe Victor meant something more like 'even when'. > > For the last clause, I believe he meant "the code change is not > complicated enough to *require* automated unit test coverage for the > changed line(s)". If I change a comment or add missing spaces, I don't > think I should be forced to write a missing test to make the improvement. > Agreed. > > A less trivial example: on IDLE's menu, Options => Configure IDLE opens > a dialog with a font size widget that when clicked opens a list of font > sizes. I recently added 4 larger sizes to the tuple in > idlelib.configdialog.ConfigDialog.LoadFontCfg, as requested, I think, by > at least 2 people. I tested manually by clicking until the list was > displayed. As I remember, I did not immediately worry about automated > testing, let alone line coverage, and I do not think I should have had > to to get the change into 3.6.0. > > That line may or may not by covered by the current minimal test that > simply creates a ConfigDialog instance. But this gets back to what I > think is Viktor's point. This minimal test 'covers' 46% of the file, > but it only tests that 46% of the lines run without raising. This is > useful, but does not test that the lines are really correct. (For GUI > display code, human eyeballing is required.) This would remain true > even if all the other code were moved to a new module, making the > coverage of configdialog the magical ***100%***. > > >> If it's not important enough to require tests >> it's not important > enough to be in Python. ;) > > Modules in the test package are mostly not tested. ;) > :) But they are at least executed which is what we're really measuring here and I think all Ethan and I are advocating for. E.g. I don't expect test_importlib to be directly responsible for exercising all code in importlib, just that Python's entire test suite exercise importlib as much as possible as a whole. > > If 'test' means 'line coverage test for new or changed lines', then as a > practical matter, I disagree, as explained above. So, in effect, did > the people who committed untested lines. > > In the wider sense of 'test', there is no real argument. Each statement > written should be mentally tested both when written and when reviewed. > Code should be manually tested, preferably by someone in addition to the > author. Automated testing is more than nice, but not everything. Ditto > for unit testing. > The problem I have with just doing manual testing for things that can easily be covered by a unit test -- directly or indirectly -- is there are technically 85 people who can change CPython today. That means there's potentially 85 different people who can screw up the code ;) . Making sure code is exercised somehow by tests at least minimizes how badly someone like me might mess something thanks to me not realizing I broke the code. > > Some practical issues with coverage and CodeCov: > > 1. A Python module is comprised of statements but coverage module counts > physical lines. This is good for development, but not for gating. The > number of physical lines comprising a statement can change without > changing or with only trivially changing the compiled run code. So if > coverage is not 100%, it can vary without a real change in statement > coverage. > As I said earlier, no one here is gating PRs on code coverage. > > 2. Some statements are only intended to run on certain systems, making > 100% coverage impossible unless one carefully puts all system-specific > code in "if system == 'xyz'" statements and uses system-specific > .coveragerc files to exclude code for 'other' systems. > True. We could have a discussion as to whether we want to use Coverage.py's pragmas -- e.g. `# pragma: no cover` -- to flag code as known to be untested. Depending on how far we wanted to take this we could also set up pragmas that flag when a test is OS-specific (I found https://bitbucket.org/ned/coveragepy/issues/563/platform-specific-configuration while looking for a solution and I'm sure we could discuss things with Ned Batchelder if we needed some functionality in coverage.py for our needs). > > 3. Some tests required extended resources. Statements that are only > covered by such tests will be seen as uncovered when coverage is run on > a system lacking the resources. As far as I know, all non-Windows > buildbots and CodeCov are run on systems lacking the 'gui' resource. So > patches to gui code will be seen as uncovered. > I view 100% coverage as aspirational, not attainable. But if we want an attainable goal, what should we aim for? We're at 83.44% now so we could say that 80% is something we never want to drop below and be done with it. We could up it to 85% or 90% in recognizing that there is more testing to do but that we will never cover all Python code (all of this is configurable in Codecov, hence why I'm asking). > > 4. As I explained in a post on the core-workflow list, IDLE needs the > following added to the 'exclude_lines' item of .coveragerc. > .*# htest # > if not _utest: > The mechanism behind these would also be useful for testing any other > modules, scripts, or demos that use tkinter GUIs. > > There seems to be other issues too. > > > "Untested code is broken code" :) > > Most of CPython, including IDLE, has been pretty thoroughly tested. And > we have heaps of bug reports to show for it. What's more important is > that even code that is tested, by whatever means, may still bugs. Hence > However, obscure bugs are still found. And even correct code can be > corrupted (repressed) by attempt fix and improve. > > -- > Terry Jan Reedy > _______________________________________________ > python-committers mailing list > python-committers@python.org > https://mail.python.org/mailman/listinfo/python-committers > Code of Conduct: https://www.python.org/psf/codeofconduct/ >
_______________________________________________ python-committers mailing list python-committers@python.org https://mail.python.org/mailman/listinfo/python-committers Code of Conduct: https://www.python.org/psf/codeofconduct/
