Support extra test runner output. #11741
Conversation
Test runners may, depending on their runtime configuration, output extra data that we don't know about, but the user may want to see. For example, pytest may be configured to run the pytest-html plugin, which will emit an HTML report into the process execution sandbox, and we won't pick it up from there. This change adds generic support for "extra output" from a test runner. All the user needs to do is ensure this output goes somewhere where the appropriate test execution rule can find it. Pants will dump any extra output it finds into dist/. In the specific case of pytest, we look for this output in the extra-output/ subdir. This means that users must ensure that output is directed there (e.g., using the --html=extra-output/report.html flag to pytest). We must document this, of course. This change also enables pytest-html in the Pants repo itself, as a proof of concept. To get a report: ./pants test <tgt> -- --html=extra-output/report.html [ci skip-rust] [ci skip-build-wheels]
|
Addresses #11735 |
src/python/pants/core/goals/test.py
Outdated
| if result.extra_output and result.extra_output.files: | ||
| workspace.write_digest( | ||
| result.extra_output.digest, | ||
| path_prefix=os.path.join("dist", result.address.path_safe_spec), |
There was a problem hiding this comment.
You want to have this rule take in a pants.core.util_rules.distdir.DistDir parameter and use that.
There was a problem hiding this comment.
You've got dist/<address>/<files> here, but test coverage uses dist/coverage/python/<files>. It seems like dist/<goal>/[not sure] may be a good way to go. Too nested seems bad for project in one language. Not nested enough seems like it could cause problems for large repos or many languages repos. Since its out of scope for this PR and dist will need to be gone over for all cases with unavoidable fallout, perhaps its best to leave as you have it and batch the fallout into a dedicated dist cleanup project.
There was a problem hiding this comment.
D'oh re using DistDir , good catch.
I'll go for dist/test/<address>/<files> - adding the test goal disambiguator is good, and the <address> should be enough to avoid collisions (we merge coverage, so don't need the <address> there).
There was a problem hiding this comment.
But this does remind me that it's unclear how useful this feature is, since you end up with a separate html file for each test file. There is no existing functionality to merge them (and pytest-html maintainers have resisted creating such a feature: pytest-dev/pytest-html#48), but I suppose it wouldn't be too hard to do if someone wants to.
| @@ -147,12 +147,15 @@ def run_pytest( | |||
| config: Optional[str] = None, | |||
| force: bool = False, | |||
| ) -> TestResult: | |||
| # For some reason we need to explicitly request setuptools to get pytest-html to work. | |||
There was a problem hiding this comment.
This is an ~ancient version of pytest-html that has a real dependency that it does not declare. Modern versions (e.g.: 3.1.1) don't appear to use pkg_resources.
There was a problem hiding this comment.
Alas, that is the most recent version that is compatible with the ancient pytest we pin to. I will update the comment.
[ci skip-rust] [ci skip-build-wheels]
[ci skip-rust] [ci skip-build-wheels]
| workspace.write_digest( | ||
| result.extra_output.digest, | ||
| path_prefix=str(dist_dir.relpath / "test" / result.address.path_safe_spec), | ||
| ) |
There was a problem hiding this comment.
Do you think we should log this?
There was a problem hiding this comment.
I thought about it, but A) it doesn't mesh well with the existing output and B) I couldn't find a useful way to phrase the log, since we have no idea what's in that extra output.
Test runners may, depending on their runtime configuration, output extra data that we don't know about, but the user may want to see. For example, pytest may be configured to run the pytest-html plugin, which will emit an HTML report into the process execution sandbox, and we won't pick it up from there. This change adds generic support for "extra output" from a test runner. All the user needs to do is ensure this output goes somewhere where the appropriate test execution rule can find it. Pants will dump any extra output it finds into dist/. In the specific case of pytest, we look for this output in the extra-output/ subdir. This means that users must ensure that output is directed there (e.g., using the --html=extra-output/report.html flag to pytest). We must document this, of course. This change also enables pytest-html in the Pants repo itself, as a proof of concept. To get a report: ./pants test <tgt> -- --html=extra-output/report.html [ci skip-rust] [ci skip-build-wheels]
Since we ask the directory to be collected as an output we need to ensure its there, even if empty. Follow-up to pantsbuild#11741 to fix remote caching. # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
Since we ask the directory to be collected as an output we need to ensure its there, even if empty. Follow-up to #11741 to fix remote caching.
Test runners may, depending on their runtime configuration,
output extra data that we don't know about, but the user may
want to see.
For example, pytest may be configured to run the pytest-html
plugin, which will emit an HTML report into the process execution
sandbox, and we won't pick it up from there.
This change adds generic support for "extra output" from a test
runner. All the user needs to do is ensure this output goes
somewhere where the appropriate test execution rule can find it.
Pants will dump any extra output it finds into dist/.
In the specific case of pytest, we look for this output in the
extra-output/ subdir. This means that users must ensure that output
is directed there (e.g., using the --html=extra-output/report.html
flag to pytest). We must document this, of course.
This change also enables pytest-html in the Pants repo itself,
as a proof of concept. To get a report:
./pants test -- --html=extra-output/report.html
[ci skip-rust]
[ci skip-build-wheels]