pycobertura
pycobertura copied to clipboard
add possibility to specify the columns to show
Add possibility to hide columns.
Additional tests needed for the case where columns are hidden. There is a problem with two tests that files are not found, inexplicable to me, would be great if you could have a look at it @aconrad
There is a problem with two tests that files are not found, inexplicable to me, would be great if you could have a look at it @aconrad
The two failing tests are:
-
test_diff__better_coverage_has_exit_status_of_zero
-
test_diff__worse_coverage_exit_status
Both tests pass the option --no-source
which means that the source code shouldn't be rendered when generating the report, only the summary table. When --no-source
is passed, it sets the flag DeltaReporter.show_source
to False
. When looking at the FileNotFound error, it's trying to open the file tests/dummy/__init__.py
which indeed doesn't exist and yet, it is obviously a source file (.py
). So for some reason, show_source
is not honored.
On line 342 is where it fails while trying to open that file:
340 class TextReporterDelta(DeltaReporter):
341 def generate(self):
342 -> lines = self.get_summary_lines()
343
344 if self.show_source:
345 missed_lines_colored = [
346 self.color_number(line) for line in lines["Missing"]
347 ]
~~... but self.show_source
is checked after, on line 344~~ (EDIT: this is misleading. While true, it doesn't imply that get_summary_lines()
shouldn't check for show_source
).
So self.get_summary_lines()
is trying to read the source file that doesn't exist and raises FileNotFound
.
Let me know if the explanation above helps. If not, I can take a closer look and see where exactly this is causing a problem.
@aconrad Could you re-review? Certainly README update and CHANGELOG update are still needed, maybe also some more tests. What do you think?
Another question popped up: Should we really make a hide columns
feature or rather a show columns
feature. The latter might allow then to specify a ordering of the columns like default
or the order in which the columns are represented in the list
or via an integer index
. This idea came up due to this issue here: https://github.com/aconrad/pycobertura/issues/161. I do not think coloring a whole row is reasonable, but if we would decide to color the Cover
column you might want to have that column as a first or second column.
Should we really make a
hide columns
feature or rather ashow columns
feature. The latter might allow then to specify a ordering of the columns likedefault
orthe order in which the columns are represented in the list
or viaan integer index
.
I generally tend to agree that "show" might be better than "hide" from a conceptual standpoint. It might also make the code more straightforward to read (dealing with "negatives" is usually trickier). I hadn't thought about the idea of using an array of columns to order the report's output, that could be useful.
If a "show" array of columns isn't provided (e.g. None
), would you think of defaulting to the regular column output?
This idea came up due to this issue here: https://github.com/aconrad/pycobertura/issues/161. I do not think coloring a whole row is reasonable, but if we would decide to color the Cover column you might want to have that column as a first or second column.
I'm still on the fence about whether we should color the output of show
, per my comment.
Hi @aconrad What is the state of this PR? What I still realize is that the command line passing does not work:
pycobertura show coverage.xml --only-show-columns ["Filename"]
<...>
KeyError: '['
but
(py37) $ python -i
>>> import os
>>> import pytest
>>> from click.testing import CliRunner
>>> from pycobertura.cli import show, diff, ExitCodes
>>> runner = CliRunner()
>>> runner.invoke(show, ['tests/dummy.original.xml', '--only-show-columns',["Filename","Miss"]],catch_exceptions=False)
<Result okay>
>>> runner.invoke(show, ['tests/dummy.original.xml', '--only-show-columns',["Filename","Miss"]],catch_exceptions=False).output
'Filename Miss\n----------------- ------\ndummy/__init__.py 0\ndummy/dummy.py 2\nTOTAL 2\n'
works. So the tests will be green, but on the command line it does not work. This is a bit frustrating. Do you have any input on this on what we can do here?
Other that click
may not support lists well and we might should pass it as a comma-separated string, which I initially proposed. Or we move to argparse
, which is Python's native argument parser.
I mean there are some workarounds here, but they all seem a least a bit "hacky": https://stackoverflow.com/questions/47631914/how-to-pass-several-list-of-arguments-to-click-option.
Hi @aconrad What is the state of this PR? What I still realize is that the command line passing does not work:
pycobertura show coverage.xml --only-show-columns ["Filename"] <...> KeyError: '['
but
(py37) $ python -i >>> import os >>> import pytest >>> from click.testing import CliRunner >>> from pycobertura.cli import show, diff, ExitCodes >>> runner = CliRunner() >>> runner.invoke(show, ['tests/dummy.original.xml', '--only-show-columns',["Filename","Miss"]],catch_exceptions=False) <Result okay> >>> runner.invoke(show, ['tests/dummy.original.xml', '--only-show-columns',["Filename","Miss"]],catch_exceptions=False).output 'Filename Miss\n----------------- ------\ndummy/__init__.py 0\ndummy/dummy.py 2\nTOTAL 2\n'
works. So the tests will be green, but on the command line it does not work. This is a bit frustrating. Do you have any input on this on what we can do here? Other that
click
may not support lists well and we might should pass it as a comma-separated string, which I initially proposed. Or we move toargparse
, which is Python's native argument parser.I mean there are some workarounds here, but they all seem a least a bit "hacky": https://stackoverflow.com/questions/47631914/how-to-pass-several-list-of-arguments-to-click-option.
Hi @aconrad What is the state of this PR? What I still realize is that the command line passing does not work:
pycobertura show coverage.xml --only-show-columns ["Filename"] <...> KeyError: '['
but
(py37) $ python -i >>> import os >>> import pytest >>> from click.testing import CliRunner >>> from pycobertura.cli import show, diff, ExitCodes >>> runner = CliRunner() >>> runner.invoke(show, ['tests/dummy.original.xml', '--only-show-columns',["Filename","Miss"]],catch_exceptions=False) <Result okay> >>> runner.invoke(show, ['tests/dummy.original.xml', '--only-show-columns',["Filename","Miss"]],catch_exceptions=False).output 'Filename Miss\n----------------- ------\ndummy/__init__.py 0\ndummy/dummy.py 2\nTOTAL 2\n'
works. So the tests will be green, but on the command line it does not work. This is a bit frustrating. Do you have any input on this on what we can do here? Other that
click
may not support lists well and we might should pass it as a comma-separated string, which I initially proposed. Or we move toargparse
, which is Python's native argument parser.I mean there are some workarounds here, but they all seem a least a bit "hacky": https://stackoverflow.com/questions/47631914/how-to-pass-several-list-of-arguments-to-click-option.
@aconrad Issues are resolved (have to admit used ChatGPT partly for this), so should be good to merge. The only thing to notice here, we have to quote the list when passing on the command line.
@aconrad Want to bring your attention to this PR again, is there something I can do here to make it easier for yout to review: rebasing, reducing the number of changes?