When a user requests a code review, the review is responsible for making sure that the code is tested. While the quality of the tests is a subjective matter, their presences is not; either they are there or they are not there. If they are not there, it is on the developer to explain why or why not.
Not every line of code is testable. Not every test is intelligent. But, at a minimum, a test should ensure that the code in a patch is run at least once, without an unexpected exception.
For Keystone and related projects, we have a tox job called cover that we can run on a git repo at a given revision. For example, I can code review (even without git review) by pulling down a revision using the checkout link in gerrit, and then running tox:
git fetch git://git.openstack.org/openstack/keystoneauth refs/changes/15/583215/2 && git checkout FETCH_HEAD git checkout -b netloc-and-version tox -e cover |
I can look at the patch using show –stat to see what files were changed:
$ git show --stat commit 2ac26b5e1ccdb155a4828e3e2d030b55fb8863b2 Author: wangxiyuan <wangxiyuan@huawei.com> Date: Tue Jul 17 19:43:21 2018 +0800 Add netloc and version check for version discovery If the url netloc in the catalog and service's response are not the same, we should choose the catalog's and add the version info to it if needed. Change-Id: If78d368bd505156a5416bb9cbfaf988204925c79 Closes-bug: #1733052 keystoneauth1/discover.py | 16 +++++++++++++++- keystoneauth1/tests/unit/identity/test_identity_common.py | 2 +- |
and I want to skip looking at any files in keystoneauth1/tests as those are not production code. So we have 16 lines of new code. What are they?
Modifying someone elses’ code, I got to
git show | gawk 'match($0,"^@@ -([0-9]+),[0-9]+ [+]([0-9]+),[0-9]+ @@",a){left=a[1];right=a[2];next};\ /^\+\+\+/{print;next};\ {line=substr($0,2)};\ /^-/{left++; next};\ /^[+]/{print right++;next};\ {left++; right++}' |
Which gives me:
+++ b/keystoneauth1/discover.py 420 421 422 423 424 425 426 427 428 429 430 431 432 433 437 +++ b/keystoneauth1/tests/unit/identity/test_identity_common.py 332 |
Looking in a the cover directory, I can see if a line is uncovered by its class:
class="stm mis" |
For example:
$ grep n432\" cover/keystoneauth1_discover_py.html | grep "class=\"stm mis\"" <p id="n432" class="stm mis"><a href="#n432">432</a></p> |
For the lines above, I can use a seq to check them, since they are in order (with none missing)
for LN in `seq 420 437` ; do grep n$LN\" cover/keystoneauth1_discover_py.html ; done |
Which produces:
<p id="n420" class="pln"><a href="#n420">420</a></p> <p id="n421" class="pln"><a href="#n421">421</a></p> <p id="n422" class="pln"><a href="#n422">422</a></p> <p id="n423" class="pln"><a href="#n423">423</a></p> <p id="n424" class="pln"><a href="#n424">424</a></p> <p id="n425" class="pln"><a href="#n425">425</a></p> <p id="n426" class="stm run hide_run"><a href="#n426">426</a></p> <p id="n427" class="stm run hide_run"><a href="#n427">427</a></p> <p id="n428" class="stm run hide_run"><a href="#n428">428</a></p> <p id="n429" class="stm par run hide_run"><a href="#n429">429</a></p> <p id="n430" class="stm run hide_run"><a href="#n430">430</a></p> <p id="n431" class="pln"><a href="#n431">431</a></p> <p id="n432" class="stm mis"><a href="#n432">432</a></p> <p id="n433" class="pln"><a href="#n433">433</a></p> <p id="n434" class="stm run hide_run"><a href="#n434">434</a></p> <p id="n435" class="pln"><a href="#n435">435</a></p> <p id="n436" class="pln"><a href="#n436">436</a></p> <p id="n437" class="pln"><a href="#n437">437</a></p> |
I drop the grep “class=\”stm mis\”” to make sure I get something, then add it back in, and get no output.
There’s a nice Python library that can do this:
https://pypi.org/project/diff_cover/
There’s a tool for this already:
https://github.com/Bachmann1234/diff-cover
It’s quite easy to integrate with tox and pytest, and you can do stuff like making the test fail if coverage of the diff is less than a given percentage and so on. Here’s a tox.ini from one of my projects for example:
https://pagure.io/fedora-qa/relvalconsumer/blob/master/f/tox.ini