pytest-pants: add JUnit XML output support, make test collector optional#534
pytest-pants: add JUnit XML output support, make test collector optional#534nprizal wants to merge 2 commits into
Conversation
Previously, pytest-pants always required buildkite-test-collector (JSON output). Now bktec detects the output format from the test command: --junit-xml uses JUnit XML (no extra dependencies), --json uses the collector's JSON output and prints the existing info message. If neither flag is present, CommandNameAndArgs returns a clear error. Also updates the docs to present JUnit as the default and the collector as an optional upgrade. Closes TE-6055 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Solid, well-scoped change, and the structure mirrors the pytest runner closely.
One thing worth checking before merging: adding ResultFormat() turns on result uploads for pytest-pants that previously never happened (it used to fall through to the "" default in RunnerConfig). The JUnit case returning "junit" matches the nunit/gotest runners, but the JSON case returns "json" where the pytest runner returns "", so bktec would now upload the collector's JSON itself. I've left an inline question about whether that's intended or could double up with the collector's own upload.
Want to dig deeper? The full session log is attached to this Buildkite build. Download the session file and open a new pi session with it:
Download the buildsworth logs from build 508, then answer my questions about the findings.
| case strings.Contains(c.TestCommand, "--junit-xml"): | ||
| resultFormat = "junit" | ||
| case strings.Contains(c.TestCommand, "--json="): | ||
| resultFormat = "json" |
There was a problem hiding this comment.
Question: This "json" value flows through ResultFormat() into uploadResults, so when BUILDKITE_TEST_ENGINE_UPLOAD_RESULTS is set, bktec will now upload the result file for the JSON path. The sibling pytest runner returns "" for its collector/JSON case (see pytest.go's ResultFormat()), deliberately leaving the upload to buildkite-test-collector, which sends results directly via BUILDKITE_ANALYTICS_TOKEN (set in buildCommand). Is the divergence here intentional — e.g. the collector doesn't receive the token inside the pants sandbox, so bktec has to do the upload — or could this end up uploading the same results twice?
There was a problem hiding this comment.
fixed in the latest commit.
| TestCommand: "pants test //passing_test.py -- --junit-xml={{resultPath}}", | ||
| ResultPath: "result-passed.xml", | ||
| }, | ||
| resultFormat: "junit", |
There was a problem hiding this comment.
Non-blocking: These JUnit tests set resultFormat on the struct directly, so the new detection in NewPytestPants (--junit-xml -> "junit") isn't exercised end-to-end. The JSON branch is covered via NewPytestPants(...) in the existing tests, but the JUnit branch — the headline behaviour of this PR — isn't. A small test asserting that a --junit-xml={{resultPath}} command through NewPytestPants yields ResultFormat() == "junit" (and the .xml temp path) would lock that in cheaply.
What
Adds JUnit XML output support to the
pytest-pantsrunner and makesbuildkite-test-collectoroptional.Previously,
pytest-pantsalways assumed JSON output from the collector. Now bktec detects the output format from the test command:--junit-xml={{resultPath}}→ JUnit XML mode, no extra dependencies needed--json={{resultPath}} --merge-json→ JSON mode, requiresbuildkite-test-collectorin the pants resolve (existing behaviour, prints the existing info message)CommandNameAndArgsreturns a clear errorThe format is stored as a
resultFormat stringfield onPytestPantsinstead of auseJUnit bool, soResultFormat()now returns"junit","json", or"".Why
buildkite-test-collectorcan't be verified by bktec in the pants context (it lives inside the pants sandbox, not the system Python), so requiring it silently caused runtime failures. JUnit XML works out of the box without any extra setup.Docs
Updated
docs/pytest-pants.mdto present JUnit as the default and the collector as an optional upgrade, following the structure ofdocs/pytest.md.Closes TE-6055