https://pantsbuild.org/ logo
#general
Title
# general
e

echoing-manchester-70122

06/09/2020, 7:22 PM
hmm we lost our test coverage output after upgrading to 1.23.0, we had this:
Copy code
[test.pytest]
coverage=auto
h

hundreds-father-404

06/09/2020, 7:30 PM
Hello! Yes, that should still work, including in 1.29. What’s the issue you’re hitting?
e

echoing-manchester-70122

06/09/2020, 7:30 PM
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

hundreds-father-404

06/09/2020, 7:32 PM
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

echoing-manchester-70122

06/09/2020, 7:33 PM
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

hundreds-father-404

06/09/2020, 7:36 PM
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

echoing-manchester-70122

06/09/2020, 7:36 PM
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

hundreds-father-404

06/09/2020, 7:40 PM
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

echoing-manchester-70122

06/09/2020, 7:43 PM
no change
test.pytest.coverage_output_dir = dist/coverage (from CONFIG in pants.ini)
h

hundreds-father-404

06/09/2020, 8:59 PM
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

echoing-manchester-70122

06/09/2020, 8:59 PM
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

hundreds-father-404

06/09/2020, 9:00 PM
Are you running
./pants test
or
./pants test path/to:test_target
?
e

echoing-manchester-70122

06/09/2020, 9:00 PM
with a path
👍 1
h

hundreds-father-404

06/09/2020, 9:01 PM
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

echoing-manchester-70122

06/09/2020, 9:06 PM
nothing about coverage in the build files, iirc
i

important-librarian-62877

06/09/2020, 10:57 PM
For an extra data point, this appears to not work in 1.26 as well
😶 1
h

hundreds-father-404

06/09/2020, 11:05 PM
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

important-librarian-62877

06/09/2020, 11:24 PM
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

hundreds-father-404

06/09/2020, 11:36 PM
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

important-librarian-62877

06/10/2020, 12:01 AM
^ That is the case for our project. Is there documentation for that
coverage
field?
e

echoing-manchester-70122

06/10/2020, 12:02 AM
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

enough-analyst-54434

06/10/2020, 12:04 AM
Adam, can you share the output of
./pants roots
?
e

echoing-manchester-70122

06/10/2020, 12:05 AM
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

enough-analyst-54434

06/10/2020, 12:08 AM
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?
no output
e

enough-analyst-54434

06/10/2020, 12:09 AM
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

hundreds-father-404

06/10/2020, 12:12 AM
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

echoing-manchester-70122

06/10/2020, 12:12 AM
correct, there is no coverage-report section
e

enough-analyst-54434

06/10/2020, 12:13 AM
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

echoing-manchester-70122

06/10/2020, 12:14 AM
yes those are there
e

enough-analyst-54434

06/10/2020, 12:15 AM
OK ... so is this affecting just you or everyone?
e

echoing-manchester-70122

06/10/2020, 12:15 AM
I’m not sure
e

enough-analyst-54434

06/10/2020, 12:15 AM
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

echoing-manchester-70122

06/10/2020, 12:18 AM
No console-report in our automated jobs
the reports under dist/coverage appear up to date.
e

enough-analyst-54434

06/10/2020, 12:20 AM
OK. What version of Pants is this again?
e

echoing-manchester-70122

06/10/2020, 12:20 AM
1.23.0
e

enough-analyst-54434

06/10/2020, 12:20 AM
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

hundreds-father-404

06/10/2020, 12:22 AM
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

enough-analyst-54434

06/10/2020, 12:24 AM
Apparently console reports were removed deliberately.
h

hundreds-father-404

06/10/2020, 12:26 AM
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

enough-analyst-54434

06/10/2020, 12:27 AM
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

echoing-manchester-70122

06/10/2020, 12:27 AM
ok thanks
e

enough-analyst-54434

06/10/2020, 12:40 AM
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

important-librarian-62877

06/10/2020, 12:46 AM
@hundreds-father-404 I'm assuming that coverage field cannot take a glob?
h

hundreds-father-404

06/10/2020, 12:48 AM
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

enough-analyst-54434

06/10/2020, 12:59 AM
@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

important-librarian-62877

06/10/2020, 1:10 AM
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

hundreds-father-404

06/10/2020, 1:21 AM
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

important-librarian-62877

06/10/2020, 1:39 AM
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

hundreds-father-404

06/10/2020, 1:41 AM
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

enough-analyst-54434

06/10/2020, 11:18 PM
@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

echoing-manchester-70122

06/10/2020, 11:20 PM
ok, thanks!
i

important-librarian-62877

06/11/2020, 12:42 AM
@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

hundreds-father-404

06/11/2020, 12:44 AM
Ah, yay to you to having a workaround! Thanks for the ticket also
i

important-librarian-62877

06/11/2020, 1:31 AM
h

hundreds-father-404

06/17/2020, 1:52 AM
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

jolly-midnight-72759

06/25/2020, 9:15 PM
@hundreds-father-404 Does this looser module trick work with v1 engine, too?
h

hundreds-father-404

06/25/2020, 9:20 PM
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