coveragepy icon indicating copy to clipboard operation
coveragepy copied to clipboard

Regression with [paths] in 6.x

Open lhupfeldt opened this issue 4 years ago • 8 comments

[paths] mapping working in 5.5 no longer works in 6.x

paths]
source =
    src/
    # Set by nox
    $VENV_PACKAGE_DIRS/

To Reproduce

  1. Python 3.6, 3.10
  2. Coverage 6.x

Code attached run with nox

Expected behavior 100% coverage

Additional context Change test/requirements.txt to use coverage 5.5 and the coverage will be 100%

cov-paths-error.zip

lhupfeldt avatar Nov 16 '21 14:11 lhupfeldt

The problem here is not about [paths]. Your code isn't being measured by coverage.py in the first place. If you add COVERAGE_DEBUG=trace to your environment, you'll see:

Not tracing '/.../cov-paths-error/.nox/unit/lib/python3.9/site-packages/dummy/dummy.py': inside --source, but is third-party

Because your package is installed for testing, coverage.py thinks it's a third-party package that shouldn't be measured.

You can fix it by adding --cov=dummy to the pytest command line. Then coverage.py can find the module you are interested in, and will measure it properly.

nedbat avatar Nov 17 '21 03:11 nedbat

Hi Ned, thank you for your quick response. Yes, I deliberately run all test against the installed package. This is the only way to make 'sure' that the package will actually work after installing. This is also why I use the src layout to avoid mistakenly importing anything directly from checked out sources.

I see that this change to ignore third-party modules is mentioned` in release notes for 5.6b1. Even if I had found that, I would not have made the connection to the missing coverage as it is my own package, and it was correctly measured pre 6.x.

The strange thing is that pre 6.x I would get coverage for my own package, but not for other installed (real third-party) packages. What caused that difference, and could that be used to identify installed packages which are the ones from the sources?

I tried your suggestion of using --cov=dummy and obviously it works. However, listing all modules in a package would be hard to maintain, so I tried renaming the package to dummypkg and specifying --cov=dummypkg. This works, but unfortunately gives the following warning:

test/dummy_test.py .                                                                                                                                                      [100%]/home/xxx/Downloads/cov-paths-error/.nox/unit/lib64/python3.10/site-packages/coverage/inorout.py:517: CoverageWarning: Module dummypkg was never imported. (module-not-imported)
  self.warn(f"Module {pkg} was never imported.", slug="module-not-imported")

lhupfeldt avatar Nov 17 '21 20:11 lhupfeldt

I'm having the same issue, however I already explicitly denote the --cov=<package name> option.

Similar to @lhupfeldt, I always test with the installed package since in the case of systemrdl-compiler, I need to run against compiled C++ extensions.

Zero coverage: https://coveralls.io/github/SystemRDL/systemrdl-compiler?branch=master Command invocation via pytest: https://github.com/SystemRDL/systemrdl-compiler/blob/master/.github/workflows/build.yml#L44 paths options: https://github.com/SystemRDL/systemrdl-compiler/blob/master/test/.coveragerc#L12-L16

amykyta3 avatar Nov 27 '21 19:11 amykyta3

Tried enabling COVERAGE_DEBUG=trace but it did not yeild any interesting messages to explain why coverage was getting skipped:

Not tracing '/home/alex/Desktop/SystemRDL/systemrdl-compiler/test/.venv/lib/python3.8/site-packages/pytest_cov/compat.py': module 'pytest_cov.compat' falls outside the --source spec
Not tracing '/home/alex/Desktop/SystemRDL/systemrdl-compiler/test/.venv/lib/python3.8/site-packages/coverage/control.py': module 'coverage.control' falls outside the --source spec
Not tracing '/home/alex/Desktop/SystemRDL/systemrdl-compiler/test/.venv/lib/python3.8/site-packages/coverage/collector.py': module 'coverage.collector' falls outside the --source spec
/home/alex/Desktop/SystemRDL/systemrdl-compiler/test/.venv/lib/python3.8/site-packages/coverage/report.py:87: CoverageWarning: Couldn't parse '/home/alex/Desktop/SystemRDL/systemrdl-compiler/systemrdl/parser/sa_systemrdl_cpp_parser.py': No source for code: '/home/alex/Desktop/SystemRDL/systemrdl-compiler/systemrdl/parser/sa_systemrdl_cpp_parser.py'. (couldnt-parse)
  coverage._warn(msg, slug="couldnt-parse")

amykyta3 avatar Nov 27 '21 19:11 amykyta3

Just to be clear: I'm fully on-board with testing packages that have been installed. It just means you have to explain the situation to coverage.py, usually with --source=....

@amykyta3 can you pastebin the entire output of --debug=trace? At the top will be a line explaining what "the --source spec" is, so we can see why your code falls outside of it.

nedbat avatar Nov 27 '21 20:11 nedbat

@amykyta3 or, can you provide me with specific instruction to reproduce the problem?

nedbat avatar Nov 28 '21 00:11 nedbat

Sure - to reproduce in my project:

  • Clone https://github.com/SystemRDL/systemrdl-compiler
  • cd systemrdl-compiler/test
  • ./run.sh

Tests run twice, with & without the C++ accelerator extension. You can prune some steps out of the run.sh script to speed things up. Also installing ccache will save you lots of time when re-running since it'll used cached results from GCC (sudo apt install ccache) - each recompile is ~3 mins which can get tedious.

I'm running through pytest-cov.

Interesting thing I just noticed - If I comment out the relative_files = True option in my .coveragerc then it seems to collect coverage correctly. I wonder if that is somehow creating a conflict. (https://github.com/SystemRDL/systemrdl-compiler/blob/master/test/.coveragerc#L3)

amykyta3 avatar Nov 28 '21 02:11 amykyta3

Hello there, and thanks @nedbat for coverage.py which is absolutely great.

In case it helps you narrow down the issue, I had the same problem here, without using pytest-cov, and

comment out the relative_files = True option in my .coveragerc then it seems to collect coverage correctly

is what got my combine command yield the expected result.

  • I've tested on my workstation, that means the absolute paths all had the same prefix
  • I've tested on Gitlab CI, but on similar executors (python containers), so the absolute must have had all the same prefix again.

A few links, just in case

cblegare avatar Feb 16 '22 13:02 cblegare

Sorry for the long delay on this. Is anyone here still experiencing this problem and can provide new steps to reproduce it?

nedbat avatar Oct 15 '22 19:10 nedbat

Yeah I just confirmed my steps will still reproduce it. I have pushed a branch that simplifies the testcases a bit:

  • Clone https://github.com/SystemRDL/systemrdl-compiler
  • Checkout the branch tmp/coveragepy
  • cd to the test folder
  • ./run.sh
  • Observe that coverage is 0%
  • in the .coveragerc file, comment out the relative_files = True setting
  • Run tests again
  • Observe coverage is reported correctly again

amykyta3 avatar Oct 15 '22 20:10 amykyta3

If I make this change to run.sh:

@@ -24,7 +24,8 @@ cd $this_dir

 # Run unit tests while collecting coverage
 export SYSTEMRDL_DISABLE_ACCELERATOR=1
-pytest --cov=systemrdl
+python -m coverage run -m pytest || true
+python -m coverage report

 # Generate coverage report
 coverage html -i -d $this_dir/htmlcov

Then I get 95% coverage:

Name                         Stmts   Miss Branch BrPart    Cover
----------------------------------------------------------------
test_accesstype.py              38      0      0      0   100.0%
test_aliases.py                112      0      6      0   100.0%
test_bridge.py                  12      0      0      0   100.0%
test_dpa_type_names.py          16      0      0      0   100.0%
test_enums.py                   60      0      6      0   100.0%
test_errors.py                  28      0      0      0   100.0%
test_expressions.py            491      0      0      0   100.0%
test_external.py                32      0      6      0   100.0%
test_importer.py                76      1      6      1    97.6%
test_inferred_placement.py     330      0     14      0   100.0%
test_intr_prop.py               31      0      0      0   100.0%
test_memories.py                21      1      8      1    93.1%
test_misc_examples.py           83      0      0      0   100.0%
test_node_utils.py             234      0     24      0   100.0%
test_parameters.py             122      0      0      0   100.0%
test_parse_accelerator.py       10      2      4      2    71.4%
test_preprocessor.py           109    100      2      0     9.9%
test_prop_refs.py               28      0      0      0   100.0%
test_prop_side_effects.py       78      0      0      0   100.0%
test_prop_typecast.py           14      0      0      0   100.0%
test_properties.py              24      0      0      0   100.0%
test_property_errors.py         13      0      0      0   100.0%
test_rdlformatcode.py           59      0      4      0   100.0%
test_references.py             245      0      0      0   100.0%
test_scopes.py                  32      0      0      0   100.0%
test_signals.py                 34      0      0      0   100.0%
test_structs.py                 83      0      2      0   100.0%
test_udp.py                    121      0      0      0   100.0%
unittest_utils.py               65     18     16      4    67.9%
----------------------------------------------------------------
TOTAL                         2601    122     98      8    95.0%

So perhaps this is a problem with pytest-cov?

nedbat avatar Oct 15 '22 22:10 nedbat

Oh cool! I didn't realize you can do that.

I'll flag this with the pytest-cov folks. Thanks for looking into it!

amykyta3 avatar Oct 15 '22 22:10 amykyta3

Oh wait.. thats not right. It seems to be reporting the coverage of the testcases, not the actual source code

amykyta3 avatar Oct 15 '22 22:10 amykyta3

Oops, I was so focused on the total, I didn't notice the file names. I'll dig into it.

nedbat avatar Oct 15 '22 23:10 nedbat

Making progress... This patch makes your code work:

diff --git a/coverage/python.py b/coverage/python.py
index da43e6e8..c8b8e774 100644
--- a/coverage/python.py
+++ b/coverage/python.py
@@ -151,7 +151,14 @@ def __init__(self, morf, coverage=None):

         filename = source_for_morf(morf)

-        super().__init__(canonical_filename(filename))
+        fname = filename
+        canonicalize = True
+        if self.coverage is not None:
+            if self.coverage.config.relative_files:
+                canonicalize = False
+        if canonicalize:
+            fname = canonical_filename(filename)
+        super().__init__(fname)

         if hasattr(morf, '__name__'):
             name = morf.__name__.replace(".", os.sep)

But it makes one test in the suite fail, working on that.

nedbat avatar Oct 24 '22 11:10 nedbat

This is now fixed in commit 45cf7936.

nedbat avatar Oct 30 '22 20:10 nedbat

This is now released as part of coverage 6.6.0b1.

nedbat avatar Oct 31 '22 11:10 nedbat

Confirmed on my end. Thanks!

amykyta3 avatar Nov 01 '22 03:11 amykyta3

This is now released as part of coverage 7.0.0b1.

nedbat avatar Dec 03 '22 18:12 nedbat