https://pantsbuild.org/ logo
Join Slack
Powered by
:wave: pants team, I have a question around covera...
# general
b
👋 pants team, I have a question around coverage-py: I’m running coverage through pytest (
[test].use_coverage = true
) on pants version 
2.8.0
. I get an error when I try to run with these two configurations • 
[coverage-py].global_report = true
in 
pants.toml
• 
[tool.coverage.run].branch = true
in 
pyproject.toml
Everything works fine if one of these is set to 
false
, but throws following error message if both are set to `true`:
Copy code
ProcessExecutionFailure: Process 'Merge 2 Pytest coverage reports.' failed with exit code 1.
stdout:
Can't combine arc data with line data

stderr:
I tried this with a simple function + single test with the following repo structure
Copy code
library_a/
  __init__.py
  BUILD
  simple.py
test/
  __init__.py
  library_a/
    __init__.py
    BUILD
    test_simple.py
pants
pants.toml
pyproject.toml
This works fine if I run coverage command directly and only fails with pants… something must be breaking when trying to merge reports for untested files with setting 
branch = true
h
Hello 🙂 When you run coverage directly, are you running 
coverage combine
? See https://github.com/pantsbuild/pants/blob/0faf9908b811107ba31dbd13154313ab10262637/src/python/pants/backend/python/goals/coverage_py.py#L56-L74 for how Pants implements coverage support
👀 1
b
I’m not running 
coverage combine
, I meant 
coverage run
b
I haven't used the coverage feature. Is `converage-py`correct, or should that be 
coverage-py
?
h
I think that was a typo, it is coverage-py
b
Ah, should’ve just copy/pasted… fixed the typo in the original post 🙂
h
@breezy-manchester-62939 sorry for the bug
Would it be possible for you to create a small temporary repo that demonstrates the issue?
👍 1
Obviously without exposing any proprietary code?
There's some stuff that comes up when searching for that error 
Can't combine arc data with line data
, so I imagine it won't be hard to debug once we have a consistent way to reproduce the bug
➕ 1
b
Here’s a zipped up directory with some minimal code
pants-test.zip
I would also add that this actually did work for at some point… I’ll see if I can generate a sample of that version as well sometime tonight and report back also.
h
Thanks!
I'll take a look
OK, I can reproduce with this repo!
Very helpful
b
Ah, here is the successful version. This actually returns correct coverage with both branch coverage + untested files. I think the difference is 
library_a
directory is under 
source
, and I had to manually add 
[source]
configuration in 
pants.toml
.
pants-test-success.zip
Unfortunately, simply adding
Copy code
[source]
root_patterns = ["/"]
does not work for the other example.
h
OK, pretty sure I know what's going on here
Yep, confirmed
The issue is this: Because we run pytest in several processes (currently one per test) we have to merge them into a global coverage file, using 
coverage combine
. But first we create an initial, trivial, coverage file to merge into, by running coverage on an empty source file. And in that run we don't set branch=True even if you set it in your own coverage config.
🙏 2
So possible solutions include setting that properly, or just not creating this dummy coverage file, which seems error prone. But I'd first have to remind myself why we do that.
b
Ah, that makes sense. But why does the second sample work when there’s a nested source directory?
I’m also using both 
global_report = true
and 
branch = true
in that example.
h
That I don't know, but I'll take a look at that case too
🙏 1
OK, I think this is because the dummy empty .py file we run coverage against to create an empty file is not created under 
source/
but directly at the package root. So in this second case that run sees no files at all, and evidently creates a coverage file that is even more trivial, and that doesn't cause issues
@breezy-manchester-62939 Can you file a bug report about this here: https://github.com/pantsbuild/pants/issues/new/choose ? And attach these two zip files?
I can then follow up on that ticket with the findings
This creates a permanent record of the bug and the fix, instead of this ephemeral Slack conversation
Thanks!
b
Ah, thanks for looking into it! will do 🙂
h
And link the issue here of course. 🙂 Thanks!
b
h
You're on 2.8.0?
This won't go out until 2.11.x, but it is a bug, so I can backport it
At least to 2.10.x, easily
How urgent is this on your end @breezy-manchester-62939? Can you upgrade to 2.10.x, say, for the fix?
b
Sorry for late response @happy-kitchen-89482 , it’s not urgent! We can easily use a src directory for now, we can upgrade to 2.10.x if we need it soon 😁
Thanks for checking!
7 Views
Open in Slack
PreviousNext
Powered by