From 786e24d815c5976990533241188d8ba4efdb1cbf Mon Sep 17 00:00:00 2001 From: Michael Selik Date: Tue, 31 Oct 2017 17:11:10 -0700 Subject: [PATCH 01/10] flatten if/elif/else for line annotation Rather than nesting an if/else inside the last else of the outer condition, the conditional can be flattened into four possibilities. --- Lib/trace.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Lib/trace.py b/Lib/trace.py index 48a1d1b6a91402..c43f9a9900e3b3 100755 --- a/Lib/trace.py +++ b/Lib/trace.py @@ -326,15 +326,14 @@ def write_results_file(self, path, lines, lnotab, lines_hit, encoding=None): n_lines += 1 elif rx_blank.match(line): outfile.write(" ") - else: + elif lineno in lnotab and not PRAGMA_NOCOVER in line: # lines preceded by no marks weren't hit # Highlight them if so indicated, unless the line contains # #pragma: NO COVER - if lineno in lnotab and not PRAGMA_NOCOVER in line: - outfile.write(">>>>>> ") - n_lines += 1 - else: - outfile.write(" ") + outfile.write(">>>>>> ") + n_lines += 1 + else: + outfile.write(" ") outfile.write(line.expandtabs(8)) return n_hits, n_lines From 05b0e73963526247cba5b79be7497d655452cabb Mon Sep 17 00:00:00 2001 From: Michael Selik Date: Tue, 31 Oct 2017 17:11:47 -0700 Subject: [PATCH 02/10] reorder condition for line annotation A blank line will not appear in ``lnotab`` as returned by ``_find_executable_linenos``. Alternately, ``lnotab`` might be set to an empty dict. Either way, the check for whether a line is blank can be done after checking for whether the ``lineno`` is in ``lnotab``. --- Lib/trace.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/trace.py b/Lib/trace.py index c43f9a9900e3b3..54f2495c9d2cd2 100755 --- a/Lib/trace.py +++ b/Lib/trace.py @@ -324,14 +324,14 @@ def write_results_file(self, path, lines, lnotab, lines_hit, encoding=None): outfile.write("%5d: " % lines_hit[lineno]) n_hits += 1 n_lines += 1 - elif rx_blank.match(line): - outfile.write(" ") elif lineno in lnotab and not PRAGMA_NOCOVER in line: # lines preceded by no marks weren't hit # Highlight them if so indicated, unless the line contains # #pragma: NO COVER outfile.write(">>>>>> ") n_lines += 1 + elif rx_blank.match(line): + outfile.write(" ") else: outfile.write(" ") outfile.write(line.expandtabs(8)) From ca648506752c270f4ed8079aa44623c53f645785 Mon Sep 17 00:00:00 2001 From: Michael Selik Date: Tue, 31 Oct 2017 17:12:27 -0700 Subject: [PATCH 03/10] delete redundant condition There's no need to check for an empty line. Either we hit the line and wrote the count, we didn't hit the line and wrote an annotation, or we want to print some spaces to indent the line. --- Lib/trace.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/trace.py b/Lib/trace.py index 54f2495c9d2cd2..14128c66f5f46f 100755 --- a/Lib/trace.py +++ b/Lib/trace.py @@ -330,8 +330,6 @@ def write_results_file(self, path, lines, lnotab, lines_hit, encoding=None): # #pragma: NO COVER outfile.write(">>>>>> ") n_lines += 1 - elif rx_blank.match(line): - outfile.write(" ") else: outfile.write(" ") outfile.write(line.expandtabs(8)) From a16eb52d95c6efeba3e541ab122a97806c46a08c Mon Sep 17 00:00:00 2001 From: Michael Selik Date: Tue, 31 Oct 2017 17:13:21 -0700 Subject: [PATCH 04/10] remove unused regex --- Lib/trace.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/Lib/trace.py b/Lib/trace.py index 14128c66f5f46f..39efbcc1c3ebf8 100755 --- a/Lib/trace.py +++ b/Lib/trace.py @@ -73,9 +73,6 @@ def _unsettrace(): PRAGMA_NOCOVER = "#pragma NO COVER" -# Simple rx to find lines with no code. -rx_blank = re.compile(r'^\s*(#.*)?$') - class _Ignore: def __init__(self, modules=None, dirs=None): self._mods = set() if not modules else set(modules) From 3af492558dbf4acbdeb68302abbbc129c3ce29d2 Mon Sep 17 00:00:00 2001 From: Michael Selik Date: Tue, 31 Oct 2017 17:33:59 -0700 Subject: [PATCH 05/10] explain (comment) lnotab parameter The lnotab parameter shouldn't be renamed, for compatibility, but it could use some further explanation. One natural reading of it would be "lines without tabs", but it's really a "line number table" -- a mapping of line numbers to possible executable lines. One nuance to that "table" is that ``_find_executable_linenos`` will ignore some code that the user might want to highlight as unreachable. For example, code inside a condition like ``if False:`` will be excluded. Further, it appears ``lnotab`` doesn't need to be a dict. A set would suffice and might reduce the memory requirements of the trace module. If the ``_find_lines`` function is used only by the trace module, as the underscore prefix suggests, it could be refactored to return a set of line numbers instead of a mapping of line numbers to lines. --- Lib/trace.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/trace.py b/Lib/trace.py index 39efbcc1c3ebf8..3d46d441f4088a 100755 --- a/Lib/trace.py +++ b/Lib/trace.py @@ -303,6 +303,7 @@ def write_results(self, show_missing=True, summary=False, coverdir=None): def write_results_file(self, path, lines, lnotab, lines_hit, encoding=None): """Return a coverage results file in path.""" + # ``lnotab`` is a dict of executable lines, or a line number "table" try: outfile = open(path, "w", encoding=encoding) @@ -322,8 +323,7 @@ def write_results_file(self, path, lines, lnotab, lines_hit, encoding=None): n_hits += 1 n_lines += 1 elif lineno in lnotab and not PRAGMA_NOCOVER in line: - # lines preceded by no marks weren't hit - # Highlight them if so indicated, unless the line contains + # Highlight never-executed lines, unless the line contains # #pragma: NO COVER outfile.write(">>>>>> ") n_lines += 1 From 2f7cd8276fc2f60c68885b7322aedf28abe6d5d5 Mon Sep 17 00:00:00 2001 From: Michael Selik Date: Tue, 31 Oct 2017 17:58:52 -0700 Subject: [PATCH 06/10] write cover files regardless of show_missing Whether the show_missing parameter is true or false, the cover file should be written. The ``lnotab`` parameter will be used simply to decide whether to annotate unreached lines with some extra characters. --- Lib/trace.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/Lib/trace.py b/Lib/trace.py index 3d46d441f4088a..6ca8a2f06aeb86 100755 --- a/Lib/trace.py +++ b/Lib/trace.py @@ -275,16 +275,15 @@ def write_results(self, show_missing=True, summary=False, coverdir=None): lnotab = _find_executable_linenos(filename) else: lnotab = {} - if lnotab: - source = linecache.getlines(filename) - coverpath = os.path.join(dir, modulename + ".cover") - with open(filename, 'rb') as fp: - encoding, _ = tokenize.detect_encoding(fp.readline) - n_hits, n_lines = self.write_results_file(coverpath, source, - lnotab, count, encoding) - if summary and n_lines: - percent = int(100 * n_hits / n_lines) - sums[modulename] = n_lines, percent, modulename, filename + source = linecache.getlines(filename) + coverpath = os.path.join(dir, modulename + ".cover") + with open(filename, 'rb') as fp: + encoding, _ = tokenize.detect_encoding(fp.readline) + n_hits, n_lines = self.write_results_file(coverpath, source, + lnotab, count, encoding) + if summary and n_lines: + percent = int(100 * n_hits / n_lines) + sums[modulename] = n_lines, percent, modulename, filename if summary and sums: From cdfed48bd6e007c5f8bfa7e95c14c4f4756a6495 Mon Sep 17 00:00:00 2001 From: Michael Selik Date: Tue, 31 Oct 2017 18:48:59 -0700 Subject: [PATCH 07/10] add news blurb --- Misc/NEWS.d/next/Library/2017-10-31.bpo-31908.g4xh8x.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2017-10-31.bpo-31908.g4xh8x.rst diff --git a/Misc/NEWS.d/next/Library/2017-10-31.bpo-31908.g4xh8x.rst b/Misc/NEWS.d/next/Library/2017-10-31.bpo-31908.g4xh8x.rst new file mode 100644 index 00000000000000..700bc690764f0e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-10-31.bpo-31908.g4xh8x.rst @@ -0,0 +1,3 @@ +Fix output of cover files for ``trace`` module command-line tool. +Previously emitted cover files only when ``--missing`` option was used. +Patch by Michael Selik. \ No newline at end of file From 50fec117317ba17f97edc79c21ba4e9e2d533683 Mon Sep 17 00:00:00 2001 From: Michael Selik Date: Mon, 23 Apr 2018 16:59:23 -0700 Subject: [PATCH 08/10] Add testcase for verifying CLI output The single test function checks that the coverfile is correctly written and contains appropriate line count and unreached code highlighting. The check for ``if []`` avoids interpreter optimization which might allow the interpreter to recognize the conditional statement as a no-op and eliminate it. For example, ``if False`` would not be counted, nor would any code in the following block be highlighted by the trace. --- Lib/test/test_trace.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/Lib/test/test_trace.py b/Lib/test/test_trace.py index 1d87aea5a3d346..486401de32387b 100644 --- a/Lib/test/test_trace.py +++ b/Lib/test/test_trace.py @@ -2,6 +2,7 @@ import sys from test.support import TESTFN, rmtree, unlink, captured_stdout from test.support.script_helper import assert_python_ok, assert_python_failure +import textwrap import unittest import trace @@ -365,6 +366,35 @@ def test_ignored(self): # Matched before. self.assertTrue(ignore.names(jn('bar', 'baz.py'), 'baz')) +# Created for Issue 31908 -- CLI utility not writing cover files +class TestCoverageCommandLineOutput(unittest.TestCase): + + codefile = 'tmp.py' + coverfile = 'tmp.cover' + + def setUp(self): + with open(self.codefile, 'w') as f: + f.write(textwrap.dedent('''\ + x = 42 + if []: + print('unreachable') + ''')) + + def tearDown(self): + unlink(self.codefile) + unlink(self.coverfile) + + def test_cover_files_written(self): + argv = '-m trace --count --missing'.split() + [self.codefile] + status, stdout, stderr = assert_python_ok(*argv) + self.assertTrue(os.path.exists(self.coverfile)) + with open(self.coverfile) as f: + self.assertEqual(f.read(), textwrap.dedent('''\ + 1: x = 42 + 1: if []: + >>>>>> print('unreachable') + ''')) + class TestCommandLine(unittest.TestCase): def test_failures(self): From c4524e44f62421f1aaf5e4321139efbd80beedb8 Mon Sep 17 00:00:00 2001 From: Michael Selik Date: Wed, 25 Apr 2018 10:30:37 -0700 Subject: [PATCH 09/10] Add test without --missing CLI option The --missing or -m option was hiding the bug. Testing the CLI with --count option and only that option reveals the bug. --- Lib/test/test_trace.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_trace.py b/Lib/test/test_trace.py index 486401de32387b..7fcc3e545604cf 100644 --- a/Lib/test/test_trace.py +++ b/Lib/test/test_trace.py @@ -375,16 +375,27 @@ class TestCoverageCommandLineOutput(unittest.TestCase): def setUp(self): with open(self.codefile, 'w') as f: f.write(textwrap.dedent('''\ - x = 42 - if []: - print('unreachable') + x = 42 + if []: + print('unreachable') ''')) def tearDown(self): unlink(self.codefile) unlink(self.coverfile) - def test_cover_files_written(self): + def test_cover_files_written_no_highlight(self): + argv = '-m trace --count'.split() + [self.codefile] + status, stdout, stderr = assert_python_ok(*argv) + self.assertTrue(os.path.exists(self.coverfile)) + with open(self.coverfile) as f: + self.assertEqual(f.read(), textwrap.dedent('''\ + 1: x = 42 + 1: if []: + print('unreachable') + ''')) + + def test_cover_files_written_with_highlight(self): argv = '-m trace --count --missing'.split() + [self.codefile] status, stdout, stderr = assert_python_ok(*argv) self.assertTrue(os.path.exists(self.coverfile)) @@ -393,7 +404,7 @@ def test_cover_files_written(self): 1: x = 42 1: if []: >>>>>> print('unreachable') - ''')) + ''')) class TestCommandLine(unittest.TestCase): From 560bc1fdecc6525ecfedb9166a775041df714b17 Mon Sep 17 00:00:00 2001 From: Michael Selik Date: Mon, 30 Apr 2018 22:50:15 -0400 Subject: [PATCH 10/10] fix test expected output indentation --- Lib/test/test_trace.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_trace.py b/Lib/test/test_trace.py index 7fcc3e545604cf..13afd36486b070 100644 --- a/Lib/test/test_trace.py +++ b/Lib/test/test_trace.py @@ -389,11 +389,11 @@ def test_cover_files_written_no_highlight(self): status, stdout, stderr = assert_python_ok(*argv) self.assertTrue(os.path.exists(self.coverfile)) with open(self.coverfile) as f: - self.assertEqual(f.read(), textwrap.dedent('''\ - 1: x = 42 - 1: if []: - print('unreachable') - ''')) + self.assertEqual(f.read(), + " 1: x = 42\n" + " 1: if []:\n" + " print('unreachable')\n" + ) def test_cover_files_written_with_highlight(self): argv = '-m trace --count --missing'.split() + [self.codefile]