| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Rather than nesting an if/else inside the last else of the outer condition, the conditional can be flattened into four possibilities.
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``.
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.
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.
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.
|
This is waiting on me (or someone) to write some tests to help prevent future regressions. |
Sorry, something went wrong.
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.
|
@berkerpeksag I added a test case, treating the CLI as a black box. |
Sorry, something went wrong.
There was a problem hiding this comment.
The test is passed if revert changes in trace.py. Please ensure that it fails with unpatched code.
Sorry, something went wrong.
| def setUp(self): | ||
| with open(self.codefile, 'w') as f: | ||
| f.write(textwrap.dedent('''\ | ||
| x = 42 |
There was a problem hiding this comment.
Please increase the indentation of these lines for readability.
Sorry, something went wrong.
The --missing or -m option was hiding the bug. Testing the CLI with --count option and only that option reveals the bug.
|
Whoops. I forgot that the --missing flag hides the bug. I added a test that uses only --count and no other CLI options. This fails without the patch. Also, indented the triple-quote. |
Sorry, something went wrong.
|
Tests are failed because dedent() removes too much. It may by simpler to not use dedent(): self.assertEqual(f.read(), '''\
1: x = 42
1: if []:
print('unreachable')
''')or self.assertEqual(f.read(),
" 1: x = 42\n"
" 1: if []:\n"
" print('unreachable')\n"
) |
Sorry, something went wrong.
|
I chose the 2nd of those two suggestions. While the explicit newlines are awkward, I think the mismatched indentation of the closing brace was the worse evil. My apologies for not checking the test pass/fail. It correctly passes the tests now in the branch. |
Sorry, something went wrong.
|
Thanks @selik for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7. |
Sorry, something went wrong.
…ol. (pythonGH-4205) Previously emitted cover files only when --missing option was used. (cherry picked from commit 47ab154) Co-authored-by: Michael Selik <mike@selik.org>
|
GH-6666 is a backport of this pull request to the 3.7 branch. |
Sorry, something went wrong.
…ol. (pythonGH-4205) Previously emitted cover files only when --missing option was used. (cherry picked from commit 47ab154) Co-authored-by: Michael Selik <mike@selik.org>
|
GH-6667 is a backport of this pull request to the 3.6 branch. |
Sorry, something went wrong.
|
Thanks @selik for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6. |
Sorry, something went wrong.
…ol. (pythonGH-4205) Previously emitted cover files only when --missing option was used. (cherry picked from commit 47ab154) Co-authored-by: Michael Selik <mike@selik.org>
|
GH-6668 is a backport of this pull request to the 3.6 branch. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
The lnotab variable -- a dict of executable lines -- is a helper for the show_missing parameter. When show_missing is false, we don't need to calculate that dict. However, emitting the cover files had been made conditional on that dict being non-empty in Commit f026dae. This reverts part of the commit to ensure the cover files are emitted regardless of the show_missing argument.
Additionally, the write_results_file is refactored to clarify the purpose of lnotab.
https://bugs.python.org/issue31908