https://pantsbuild.org/ logo
Join Slack
Powered by
hmm we lost our test coverage output after upgradi...
# general
e
hmm we lost our test coverage output after upgrading to 1.23.0, we had this:
Copy code
[test.pytest]
coverage=auto
h
Hello! Yes, that should still work, including in 1.29. What’s the issue you’re hitting?
e
Copy code
# <https://jira.pinadmin.com/browse/SRE-4755> - all pytest config can probably be set back to defaults once we remove py2.7 support
[pytest]
version: pytest>=4.6.6,<4.7
pytest_plugins: +["setuptools<45; python_version < '3'","configparser==4.0.2; python_version < '3'","zipp<2; python_version < '3'","importlib-metadata<1.6; python_version < '3'"]

[test.pytest]
coverage=auto
coverage-html and coverage-xml are just blank
do I need to add pytest-coverage as a plugin?
before we had this, which worked :
Copy code
[pytest]
timeout_requirements: pytest-timeout==1.2.0
cov_requirements: pytest-cov==2.4.0

[test.pytest]
coverage=auto
h
You shouldn’t need to because it’s already in the default, and you’re adding to the list Try 
./pants options | grep pytest
to double check that it’s resolving the option correctly
e
Copy code
$  ./pants options |grep pytest
Scrubbed PYTHONPATH=/Users/amckenna/code/python-commons/src/main/python from the environment.
pytest.cov_requirements = pytest-cov>=2.8.1,<3 (from HARDCODED)
pytest.pytest_plugins = ['pytest-timeout>=1.3.3,<1.4', 'pytest-cov>=2.8.1,<3', "unittest2>=1.1.0 ; python_version<'3'", "more-itertools<6.0.0 ; python_version<'3'", "setuptools<45; python_version < '3'", "configparser==4.0.2; python_version < '3'", "zipp<2; python_version < '3'", "importlib-metadata<1.6; python_version < '3'"] (from CONFIG in pants.ini)
pytest.requirements = pytest>=4.6.6,<4.7 (from HARDCODED)
pytest.timeout_requirements = pytest-timeout>=1.3.3,<1.4 (from HARDCODED)
pytest.unittest2_requirements = unittest2>=1.1.0 ; python_version<'3' (from HARDCODED)
pytest.version = pytest>=4.6.6,<4.7 (from HARDCODED)
test.pytest.chroot = False (from HARDCODED)
test.pytest.coverage = auto (from CONFIG in pants.ini)
test.pytest.coverage_include_test_sources = False (from HARDCODED)
test.pytest.coverage_output_dir = None (from NONE)
test.pytest.coverage_reports = ['xml', 'html'] (from HARDCODED)
test.pytest.extra_pythonpath = [] (from NONE)
test.pytest.fast = True (from HARDCODED)
test.pytest.junit_xml_dir = None (from NONE)
test.pytest.passthrough_args = [] (from NONE)
test.pytest.profile = None (from NONE)
test.pytest.skip = False (from HARDCODED)
test.pytest.test_shard = None (from NONE)
test.pytest.timeout_default = None (from NONE)
test.pytest.timeout_maximum = None (from NONE)
test.pytest.timeout_terminate_wait = 10 (from HARDCODED)
test.pytest.timeouts = True (from HARDCODED)
h
Hm, okay, so looks like 
pytest-cov
still shows up. Good. The coverage options all look correct too This has never been working in 1.23 for you? We’ve barely touched that codepath the past few months, so I’m trying to pin down what would be causing the issue
what was the most recent version that worked for you?
e
yeah it hasn’t worked since then, I just got time to look into it today
last version was 1.16.0 iirc
👍 1
Copy code
===== 17 passed, 5 warnings in 2.78 seconds ======

12:34:26 00:40       [coverage-html]

12:34:28 00:42       [coverage-xml]

                   src/test/python/.../devapp_tool                                           .....   SUCCESS
               Waiting for background workers to finish.
12:34:29 00:43   [complete]
               SUCCESS
h
Huh. So it’s running at least. Does it make any difference if you change the option 
coverage_output_dir
in 
[test.pytest]
? You can explicitly set to 
'dist/coverage'
, for example
e
no change
test.pytest.coverage_output_dir = dist/coverage (from CONFIG in pants.ini)
h
Hm, bummer. I’ve got to run to a meeting but will try to to reproduce after, using your same 
pants.ini
you sent. Anything of note with your 
python_tests
target, e.g. if it explicitly sets the 
coverage
field? Is the test target in the same folder as the src code?
e
I’m just running ./pants test with no other options from the command line right now
Copy code
$ grep -i coverage pants pants.ini
pants.ini:coverage=auto
pants.ini:coverage_output_dir=dist/coverage
h
Are you running 
./pants test
or 
./pants test path/to:test_target
?
e
with a path
👍 1
h
and does the 
python_tests
target you’re specifying live in the same folder as the 
src
code? Or you’re setting the 
coverage
field in the BUILD file?
e
nothing about coverage in the build files, iirc
i
For an extra data point, this appears to not work in 1.26 as well
😶 1
h
I set up a minimal repro, and am seeing this in my output: 
No .coverage file was found! Skipping coverage reporting.
Does that look familiar?
Put up the repro at https://github.com/Eric-Arellano/coverage-debug. Going to ask some folks if they know what’s up. Sorry you both for the issue!
i
Don't want to muddy the waters too much from what @echoing-manchester-70122 is seeing, but running with level debug is showing the coverage config pulling the proper files. I get the line "No data to report" after the 
[coverage-<type>]
tag.
👍 1
h
Okay, I’m able to reproduce what you’re getting Connor in that example repo. I think what is going on is that 
auto
is limited / doesn’t always work. Try setting the field 
coverage=['helloworld.util.demo']
in your 
python_tests
target, but with the module that you expect to cover. Lmk if that works to get data generated I suspect that when it worked on 1.16, you were running against a different target where 
auto
did happen to work. Now you’re running on a target that can’t be inferred, for whatever reason. In other words, I suspect Pants hasn’t changed, but rather the command being run has changed
Huh, so my setup had a weird structure. John got it working using 
auto
https://github.com/Eric-Arellano/coverage-debug/pull/1 If 
auto
is not working, I suspect the issue is that the tests are not in the same folder as the 
src
code, e.g. having 
src/python
and 
tests/python
folders. In that case, you’d specify the 
coverage
field in each 
python_tests
target
i
^ That is the case for our project. Is there documentation for that 
coverage
field?
e
our targets haven’t changed.. this ‘just worked’ before on all of our tests
but no, the tests aren’t in the same folder, we have source under src/main and tests under src/test
I don’t know why it was set up that way, but that’s the way it’s always been
e
Adam, can you share the output of 
./pants roots
?
e
Copy code
$ ./pants roots
Scrubbed PYTHONPATH=/Users/amckenna/code/python-commons/src/main/python from the environment.
3rdparty/python: python
src/__pycache__: __pycache__
src/main: *
src/main/attic: attic
src/main/config: config
src/main/python: python
src/main/thrift: thrift
src/test: *
src/test/__pycache__: __pycache__
src/test/attic: attic
src/test/python: python
e
OK, and when you say:
coverage-html and coverage-xml are just blank
Do you mean the generated index.html and coverage.xml under dist/coverage/... are blank? Or they just don't even exist?
e
no output
e
There is supposed to be no output under those sections in the console ui.
They run and generate files under dist/coverage
You are missing a section above though that contains console output.
To be clear, in Eric's example repo linked above, I get:
Copy code
17:10:35 00:06       [coverage-report]
                     Name                                                                                                                                      Stmts   Miss Branch BrPart  Cover
                     ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
                     /home/jsirois/dev/Eric-Arellano/jsirois-coverage-debug/.pants.d/pyprep/sources/7f6099ca983f89070bc723af80d653cdf20ba5d3/pkg/__init__.py       1      0      0      0   100%
                     src/python/pkg/demo.py                                                                                                                        2      0      0      0   100%
                     src/python/pkg/demo_test.py                                                                                                                   3      0      0      0   100%
                     ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
                     TOTAL                                                                                                                                         6      0      0      0   100%
                     
17:10:36 00:07       [coverage-html]
                     
17:10:36 00:07       [coverage-xml]
                     
                   src/python/pkg:tests                                                            .....   SUCCESS
Your output seems to be missing the 
[coverage-report]
console output section. Is that correct?
h
Is there documentation for that coverage field?
On Pants 1.27+, 
./pants target-types --details=python_tests
. On older, 
./pants targets --details=python_tests
. (We’re also soon working on adding docs to the new website about the v2 engine implementation, which is quite similar)
💯 1
e
correct, there is no coverage-report section
e
Aha. Can you please confirm / deny the existence of index.html / coverage.xml under dist/coverage and whether or not their contents look sane if present?
e
yes those are there
e
OK ... so is this affecting just you or everyone?
e
I’m not sure
e
My hunch is a personal config file for pytest or coverage turning off console-reports.
I can't recall if Pants of that vintage is properly defensive against such things.
... so for crystal clarity: 1. If you open the index.html in a browser, does the report look like you expect? 2. Can you find a buddy to check out whether they see the console-report section?
e
No console-report in our automated jobs
the reports under dist/coverage appear up to date.
e
OK. What version of Pants is this again?
e
1.23.0
e
Aha! OK, Erics example repo uses older.
It's simply now an option that defaults to html and xml, no console. Just a sec ...
Just a sec for pointers ...
h
I was using 1.17 because Adam said it worked with 1.16. I went back to 1.17 because the oldest the current 
./pants
script lets you use without using an older version of the bash script, and I was being a little lazy.
e
Apparently console reports were removed deliberately.
h
Oof. Well, so, mystery solved? Sorry for all this confusion, Adam! Fwit, we added them back with the v2 engine in more recent Pants versions..but you must explicitly opt into them now
e
Yeah, apologies. I reviewed that change but either missed console being dropped or ... not sure. We could have kept those as an option.
👍 1
e
ok thanks
e
Typically we've been encouraging users to upgrade to Pants v2, but this drop of functionality never went through a deprecation cycle which we try to be vigilant about (https://pants.readme.io/docs/deprecation-policy). I know you may not be in a position to upgrade and the fix is very small:
Copy code
$ git diff src/python/pants/backend/python/tasks/pytest_run.py
diff --git a/src/python/pants/backend/python/tasks/pytest_run.py b/src/python/pants/backend/python/tasks/pytest_run.py
index fa279e1a0..0f9b96408 100644
--- a/src/python/pants/backend/python/tasks/pytest_run.py
+++ b/src/python/pants/backend/python/tasks/pytest_run.py
@@ -137,7 +137,7 @@ class PytestRun(ChrootedTestRunnerTaskMixin, Task):
         register(
             "--coverage-reports",
             fingerprint=True,
-            choices=("xml", "html"),
+            choices=("xml", "html", "console"),
             type=list,
             member_type=str,
             default=("xml", "html"),
@@ -432,6 +432,8 @@ class PytestRun(ChrootedTestRunnerTaskMixin, Task):
                     else:
                         coverage_workdir = workdirs.coverage_path
                         coverage_reports = self.get_options().coverage_reports
+                        if "console" in coverage_reports:
+                            coverage_run('report', ['-i', '--rcfile', coverage_rc])
                         if "html" in coverage_reports:
                             coverage_run(
                                 "html", ["-i", "--rcfile", coverage_rc, "-d", coverage_workdir]
I think this warrants a fix, and I'll post that. The stickier issue is backporting to a point release for 1.23 - 1.28.
👍 1
i
@hundreds-father-404 I'm assuming that coverage field cannot take a glob?
h
Unfortunately, it cannot 😕 It can either be a single string for the module name, or a list of module names Partially for this reason, with Pants’s own repo, we’ve been slowly migrating from having separate 
src/python
and 
tests/python
source roots to putting everything into 
src/python
. When tests are in the same folder as the src code, 
auto
should work. (We also like being able to easily see which files have tests and which don’t)
e
@important-librarian-62877 the --test-pytest-coverage option (unlike the python_tests.coverage field) can take a path. This can act like a glob prefixing the parts of the python source tree you want to gather coverage for. That's only really useful if you have a single source root for all of your production python code though.
👀 1
👍 1
i
Almost all of our code is setup as 
toplevel/module/module/*.py
, 
toplevel/module/tests_module/*.py
, where 
toplevel
can be libraries, services, etc. There can also be subdirectories under the second level 
module
. I'm not really seeing a simple way to set up coverage for a case like that 🙁
h
This is not at all a satisfying answer - ideally you should not have to reorganize your codebase to work with a tool - but would you all have any interest in moving the tests to co-live with prod files? Or you like the separation? I haven’t spent much time with the Pants coverage implementation, and am wondering how feasible it would be to implement a more automated solution to your setup with the v2 engine implementation. I’m trying to also gauge where it would fall on the roadmap
i
I can check with the team, but I would assume the overall feeling would lean towards keeping the structure the way it is. Coverage isn't an absolute need right now, it's more of a nice to have, but having it on the v2 engine roadmap would be awesome.
👍 1
h
Cool, let us know 🙂 Also, please feel free to open a ticket at https://github.com/pantsbuild/pants/issues/new so that we can track this better. It’d help to copy the explanation you gave above of your setup, along with explaining you don’t want to have to use the 
coverage
field on every target
e
@echoing-manchester-70122 circling back on this. The fix to restore console coverage reports is in on master and staged for the 1.29.0 release.: https://github.com/pantsbuild/pants/pull/10013 That does not help you though. I have cherry-picks in CI for 1.2{1,2,3,4,5,6,7,8} though and will be performing releases for those. There appears to be a substantial amount of CI babysitting needed so those may take a while. Eventually though, there will be a 1.23.1 release with the fix. You'll have to configure 
--test-pytest-coverage-reports=console
on the CLI or else add this to the list of reports printed by default either in ~/.pants.rc (for just you personally) or pants.ini (for the whole repo):
Copy code
[test.pytest]
coverage_reports: +["console"]
e
ok, thanks!
i
@hundreds-father-404 Circling back here, I managed to cobble together a script using 
list
and the 
--test-pytest-coverage
option @enough-analyst-54434 mentioned to accomplish coverage for our situation. Talked with the team about moving the source and test files together and they were against it. I will write up a ticket with our situation, but again, it's definitely just a nice to have.
💯 1
h
Ah, yay to you to having a workaround! Thanks for the ticket also
i
h
Hey Connor, I’ve been working on improving v2 coverage this week and found something that might be helpful. With the 
coverage
field, you can use a looser module. That is, you can use any of 
project.util.dirutil
, 
project.util
, or 
project
. If you use 
project
, then Coverage will calculate the coverage for any module that starts with 
project
, and so on. I’ll be documenting this all this week too. (Tracked by https://github.com/pantsbuild/pants/issues/10064)
j
@hundreds-father-404 Does this looser module trick work with v1 engine, too?
h
It does indeed. Although, warning that v1 coverage is a little unexpected how it works. For example, the 
coverage
field is a little weird to get right. We redesigned it in 1.30.0 and are putting final polish on how coverage works in 2.0.0. See the PR description of https://github.com/pantsbuild/pants/pull/10100 for how it works now
🎉 1
6 Views
Open in Slack
PreviousNext
Powered by