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.)
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.
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.
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. ;)
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.
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.
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.
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.
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.
