pytest: use output_files for junit XML generation option #10136

tdyas · 2020-06-23T05:15:58Z

Problem

When running pytest with JUnit XML generation enabled, the Pants pytest_runner uses output_directories instead of output_files to make the output file known to the process executor. This works fine with local execution but breaks with remote execution because RE servers can skip output_directories that are not in fact directories.

This issue manifests as errors like this:

$ ./pants --pytest-junit-xml-dir=dist/test-results [remote-config options] test XXX
05:07:26.01 [INFO] Completed: Run Pytest for XXX
05:07:26.02 [WARN] Failed to generate JUnit XML data for XXX.
05:07:26.02 [INFO] Tests failed: XXX
𐄂 XXX
ERROR: --junitxml must be a filename, given: XXX.xml

Solution

Use output_files instead for the JUnit XML generation file.

Result

The test runs and JUnit XML is properly generated and written to the local filesystem.

Eric-Arellano · 2020-06-23T05:22:22Z

Thanks Tom!

It’d be good for us to test Junit XML. You can mostly copy the test_success method, minus setting up the flag + inspecting the XML file in the TestResult. I’m happy to help tomorrow with this.

asherf · 2020-06-23T15:06:18Z

This issue is a problem long term, we basically have an API (output_directories) that works locally but won't work with remote execution, which is bad.
this needs to be solved at the infra level somehow or we just need to deprecate the API that is not compatible with RE.

src/python/pants/backend/python/rules/pytest_runner.py

tdyas · 2020-06-23T17:11:19Z

this needs to be solved at the infra level somehow or we just need to deprecate the API that is not compatible with RE.

REv2 is what actually defines output_files and output_directories. REv2.1 deprecates those options in favor of an output_paths attribute given the issues (as Pants encountered here) with having to differentiate between files and directories in the earlier API versions.

We should probably have the local execution API print a warning (or maybe even error out) if a file ends up in output_directories or a directory ends up in output_files. That would flag the issue earlier.

tdyas · 2020-06-23T21:28:00Z

src/python/pants/backend/python/rules/pytest_runner_integration_test.py

+        assert file.path.startswith("dist/test-results")
+        assert b"pants_test.test_good" in file.content
+
+    @pytest.mark.skip(reason="https://github.com/pantsbuild/pants/issues/10141")


This test is skipped as per discussion with @Eric-Arellano who will fix as per #10141.

Eric-Arellano

Thank you, Tom!

Eric-Arellano · 2020-06-23T21:38:18Z

src/python/pants/backend/python/rules/pytest_runner_integration_test.py

+        assert "test_good.py ." in result.stdout
+        assert result.xml_results is not None
+
+        files: FilesContent = self.request_single_product(FilesContent, result.xml_results)


I don't think the type hint is necessary. MyPy should be able to refer the type. I can fix this in a followup, though.

Suggested change

files: FilesContent = self.request_single_product(FilesContent, result.xml_results)

files = self.request_single_product(FilesContent, result.xml_results)

Will remove.

tdyas · 2020-06-23T22:18:48Z

@Eric-Arellano: I have some additional testing to do this afternoon, will let you know when this is ready. I'm still testing the coverage-related change against build.toolchain.com

Eric-Arellano · 2020-06-23T22:31:35Z

MyPy failure: https://travis-ci.com/github/pantsbuild/pants/jobs/352858202#L596

tdyas · 2020-06-23T23:33:42Z

Coverage passed remotely:

23:33:01.84 [INFO] Completed: Building coverage.pex with 1 requirement: coverage>=5.0.3,<5.1
23:33:03.85 [INFO] Completed: Generate Pytest report coverage report.

Name                                                                  Stmts   Miss Branch BrPart  Cover
-------------------------------------------------------------------------------------------------------
XXX.py                                          12     12      6      0     0%
[lots of stuff cut]
.
.
.

Eric-Arellano · 2020-06-24T03:06:02Z

Thanks again, Tom!

…antsbuild#10136) When running pytest with JUnit XML generation enabled, the Pants pytest_runner [uses `output_directories` instead of `output_files` to make the output file known to the process executor](https://github.com/pantsbuild/pants/blob/master/src/python/pants/backend/python/rules/pytest_runner.py#L237-L245). This works fine with local execution but breaks with remote execution because RE servers can skip `output_directories` that are not in fact directories. This issue manifests as errors like this: ``` $ ./pants --pytest-junit-xml-dir=dist/test-results [remote-config options] test XXX 05:07:26.01 [INFO] Completed: Run Pytest for XXX 05:07:26.02 [WARN] Failed to generate JUnit XML data for XXX. 05:07:26.02 [INFO] Tests failed: XXX 𐄂 XXX ERROR: --junitxml must be a filename, given: XXX.xml ``` Use `output_files` instead for the JUnit XML generation file. The test runs and JUnit XML is properly generated and written to the local filesystem.

…herrypick of #10136) (#10324) When running pytest with JUnit XML generation enabled, the Pants pytest_runner [uses `output_directories` instead of `output_files` to make the output file known to the process executor](https://github.com/pantsbuild/pants/blob/master/src/python/pants/backend/python/rules/pytest_runner.py#L237-L245). This works fine with local execution but breaks with remote execution because RE servers can skip `output_directories` that are not in fact directories. This issue manifests as errors like this: ``` $ ./pants --pytest-junit-xml-dir=dist/test-results [remote-config options] test XXX 05:07:26.01 [INFO] Completed: Run Pytest for XXX 05:07:26.02 [WARN] Failed to generate JUnit XML data for XXX. 05:07:26.02 [INFO] Tests failed: XXX 𐄂 XXX ERROR: --junitxml must be a filename, given: XXX.xml ``` Use `output_files` instead for the JUnit XML generation file. The test runs and JUnit XML is properly generated and written to the local filesystem. Co-authored-by: Tom Dyas <tdyas@toolchain.com>

asherf reviewed Jun 23, 2020

View reviewed changes

src/python/pants/backend/python/rules/pytest_runner.py Outdated Show resolved Hide resolved

tdyas mentioned this pull request Jun 23, 2020

fix test for coverage in pytest_runner #10141

Closed

tdyas commented Jun 23, 2020

View reviewed changes

Eric-Arellano approved these changes Jun 23, 2020

View reviewed changes

Tom Dyas added 4 commits June 23, 2020 18:47

use output_files for junit XML generation option

3eb15e7

use output_files for coverage as well, add tests

dc100f8

link to github issue

0eb67c3

mypy fix

9addeda

tdyas force-pushed the use_output_files_for_junit_xml branch from ed655d5 to 9addeda Compare June 24, 2020 01:47

Eric-Arellano added the needs-cherrypick label Jun 24, 2020

Eric-Arellano added this to the 1.30.x milestone Jun 24, 2020

Eric-Arellano merged commit 396ec64 into pantsbuild:master Jun 24, 2020

asherf mentioned this pull request Jun 24, 2020

Generate JUNIT XML test results when running tests. #10151

Closed

stuhood removed the needs-cherrypick label Jul 14, 2020

tdyas deleted the use_output_files_for_junit_xml branch February 20, 2023 20:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pytest: use output_files for junit XML generation option #10136

pytest: use output_files for junit XML generation option #10136

tdyas commented Jun 23, 2020 •

edited

Eric-Arellano commented Jun 23, 2020

asherf commented Jun 23, 2020

tdyas commented Jun 23, 2020

tdyas Jun 23, 2020

Eric-Arellano left a comment

Eric-Arellano Jun 23, 2020

tdyas Jun 23, 2020

tdyas commented Jun 23, 2020

Eric-Arellano commented Jun 23, 2020

tdyas commented Jun 23, 2020 •

edited

Eric-Arellano commented Jun 24, 2020

	files: FilesContent = self.request_single_product(FilesContent, result.xml_results)
	files = self.request_single_product(FilesContent, result.xml_results)

pytest: use output_files for junit XML generation option #10136

pytest: use output_files for junit XML generation option #10136

Conversation

tdyas commented Jun 23, 2020 • edited

Problem

Solution

Result

Eric-Arellano commented Jun 23, 2020

asherf commented Jun 23, 2020

tdyas commented Jun 23, 2020

tdyas Jun 23, 2020

Choose a reason for hiding this comment

Eric-Arellano left a comment

Choose a reason for hiding this comment

Eric-Arellano Jun 23, 2020

Choose a reason for hiding this comment

tdyas Jun 23, 2020

Choose a reason for hiding this comment

tdyas commented Jun 23, 2020

Eric-Arellano commented Jun 23, 2020

tdyas commented Jun 23, 2020 • edited

Eric-Arellano commented Jun 24, 2020

tdyas commented Jun 23, 2020 •

edited

tdyas commented Jun 23, 2020 •

edited