aconrad / pycobertura

A code coverage diff tool for Cobertura reports
MIT License
114 stars 39 forks source link

Ignore / only warn about missing files #86

Open blueyed opened 6 years ago

blueyed commented 6 years ago

It might be useful to ignore missing files, issuing a warning only.

This is relevant for when using git archive to create the source trees, which does not include untracked files, which are included in the report/coverage.xml though.

Currently it is failing like this:

Traceback (most recent call last):
  File "…/project/.venv/lib/python3.6/site-packages/pycobertura/utils.py", line 24, in cache_get
    return cache[key]
KeyError: ('project/app/tests/xxx_test_factoryboy.py',)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "…/project/.venv/bin/pycobertura", line 11, in <module>
    load_entry_point('pycobertura==0.10.4', 'console_scripts', 'pycobertura')()
  File "…/project/.venv/lib/python3.6/site-packages/click/core.py", line 716, in __call__
    return self.main(*args, **kwargs)
  File "…/project/.venv/lib/python3.6/site-packages/click/core.py", line 696, in main
    rv = self.invoke(ctx)
  File "…/project/.venv/lib/python3.6/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "…/project/.venv/lib/python3.6/site-packages/click/core.py", line 889, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "…/project/.venv/lib/python3.6/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "…/project/.venv/lib/python3.6/site-packages/pycobertura/cli.py", line 168, in diff
    report = reporter.generate()
  File "…/project/.venv/lib/python3.6/site-packages/pycobertura/reporters.py", line 229, in generate
    lines = self.get_report_lines()
  File "…/project/.venv/lib/python3.6/site-packages/pycobertura/reporters.py", line 167, in get_report_lines
    file_row = self.get_file_row(filename)
  File "…/project/.venv/lib/python3.6/site-packages/pycobertura/reporters.py", line 139, in get_file_row
    missed_lines = self.differ.diff_missed_lines(filename)
  File "…/project/.venv/lib/python3.6/site-packages/pycobertura/cobertura.py", line 359, in diff_missed_lines
    for line in self.file_source(filename):
  File "…/project/.venv/lib/python3.6/site-packages/pycobertura/cobertura.py", line 386, in file_source
    lines2 = self.cobertura2.source_lines(filename)
  File "…/project/.venv/lib/python3.6/site-packages/pycobertura/utils.py", line 14, in __call__
    return self.cache_get(self.memoized, args, lambda: self.func(*args))
  File "…/project/.venv/lib/python3.6/site-packages/pycobertura/utils.py", line 26, in cache_get
    v = cache[key] = func()
  File "…/project/.venv/lib/python3.6/site-packages/pycobertura/utils.py", line 14, in <lambda>
    return self.cache_get(self.memoized, args, lambda: self.func(*args))
  File "…/project/.venv/lib/python3.6/site-packages/pycobertura/cobertura.py", line 263, in source_lines
    with self.filesystem.open(filename) as f:
  File "/usr/lib/python3.6/contextlib.py", line 81, in __enter__
    return next(self.gen)
  File "…/project/.venv/lib/python3.6/site-packages/pycobertura/filesystem.py", line 40, in open
    raise self.FileNotFound(filename)
pycobertura.filesystem.FileNotFound: tmp/project/2many/project/app/tests/xxx_test_factoryboy.py
aconrad commented 6 years ago

Hmm, never heard about this use case before. What is the reasoning/workflow behind having your coverage report include files that aren't checked in? My assumption is that if the file isn't checked in, then it shouldn't be included as part of the coverage report in the first place.

blueyed commented 6 years ago

My use case is some WIP test file that is used only locally. Since it matches the pattern of normal test files it will be included. I could add it to coveragepy's ignore list, of course - even after the run. Also some local code e.g. app/foo/__init__.py when there is only app/foo/bar.py is included in coverage, since it gets imported.

It's only really an issue since git archive has to be used. In this regard I found https://github.com/Bachmann1234/diff-cover more useful, which uses Git directly.

If you do not want to ignore/skip them I'd suggest to improve the error message then at least.

Maybe it could also be mentioned with the recipe at https://github.com/aconrad/pycobertura#why-doesnt-pycobertura-use-git-to-diff-the-source-given-revision-shas-rather-than-passing-paths-to-the-source-code.

aconrad commented 6 years ago

Thanks @blueyed for the explanation.

I definitely think we should have a nicer error message instead of a stack trace which is not user-friendly. As for the ignore/skip I have to think more and what it entails -- but at a first glance it seems feasible.

But before that let me tell you that if you are working locally making changes as you go then you technically don't need to do the git archive step, these are mostly instructions to work with continuous integration systems. All you really need are 2 directories that have the source code at different versions with their respective coverage report.

For example, you could do:

1> git clone git@github.com:blueyed/myrepo.git
2> cp -a myrepo myrepo-master  # make a copy
3> cd myrepo-master && <generate coverage.xml>  # generate coverage report for master
4> cd ../myrepo && git checkout myfeature  # go into your working branch
5> <make changes for myfeature> && <generate coverage.xml>
6> pycobertura diff ../myrepo-master/coverage.xml coverage.xml

Then just repeat steps 5 and 6 as you make code changes to your code base.

diff-cover works well for the most part but has its drawbacks, which is the reason I built pycobertura in the first place. By design, diff-cover does a git diff and then checks if the lines in the diff output are covered in the coverage report. But if you stop calling a function that is not in the output of git diff (e.g. a function you wrote a long time ago) then coverage will drop because that function is no longer executed and diff-cover won't tell you where it dropped. Pycobertura addresses this issue by diffing the coverage reports themselves (and not just the diff of sources) and generates very accurate coverage diff reports where dead code is surfaced in the coverage diff report.

Let me know if the steps above work for you and I will update the README to make it clearer.

gro1m commented 2 years ago

Hi @aconrad @blueyed As I am working on a regex to include files you want, it could maybe be a possibility to add an option to pass a file (for example .gitignore) in which you could specify the files or file patterns you want to exclude from the report. What do you think?

aconrad commented 2 years ago

Hm. I think that's a good idea. We should still prevent missing files from crashing pycobertura to address the root issue. But ignoring coverage for files included in .gitignore could be useful. Can we think of any downsides? Should it be enabled by default or be an opt-in to begin with?