Fix spurious 'no result set' warnings and dropped execution tags under pytest-forked#124
Merged
Merged
Conversation
…llection Fork-per-test runners (pytest-forked) run each test protocol in a forked child with log=False, then replay the serialized reports in the parent. The child inherits a copy of the plugin state that is discarded on exit, but the collector still finalized tests there — the result is never set in-child (no logreport), so every test logged a spurious "Test ... has no result set at finalization" warning. Record the owning process pid at plugin construction and make finalize_test a no-op in any other process. The owning process finalizes from the replayed reports, as it always did — uploaded results and durations were never affected by this bug, only the log noise. The same parent/child state split also silently dropped execution_tag markers under --forked: tags were only applied in pytest_runtest_makereport, which only fires in the discarded child. Capture markers at collection time (which runs in the owning process) and apply them at finalization, so tags now survive --forked, including in combination with xdist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
This looks fine to me, but I don't have the ability to merge to this repo. |
mcncl
reviewed
Jul 14, 2026
Re-applying the collection-time snapshot in finalize_test could overwrite a fresher value from a marker added dynamically during the test (applied in pytest_runtest_makereport), since tag_execution overwrites by key. Guard the re-apply so it only fills in missing keys, and add a regression test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
gchan
approved these changes
Jul 17, 2026
pytest 9's wheel ships a vendored top-level py.py shim (py.error/py.path only). Under `uv run --with pytest>=9` the overlay site-packages precede the project venv on sys.path, so the shim shadows the real py package that pytest-forked needs for py.process, crashing --forked runs with INTERNALERROR: module 'py' has no attribute 'process'. Add --with py to the overlay so the real package resolves in the same directory as the shim (package wins over module), and skip the forked integration test gracefully in environments where pytest-forked itself is broken this way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
gchan
approved these changes
Jul 17, 2026
|
@gchan we don't have permission to merge to this repo. If you're happy, can you merge and possibly do a release as well? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Customer escalation: a suite using
pytest --forked(required for their tests — removing it causes deadlocks) loggedWARNING - Test <nodeid> has no result set at finalizationfor every test after switching to this collector.Root cause
pytest-forkedruns each test's protocol in a forked child process withlog=False, then replays the serialized reports in the parent:pytest_runtest_logstart, then forks.pytest_runtest_makereportfires (setup/call/teardown), butpytest_runtest_logreport— the only hook where we set the pass/fail result — never fires there because oflog=False. So at teardown the child finalizes its inherited copy of the in-flight record withresult=Noneand emits the warning.So the warning was pure noise from a doomed process copy — uploaded results, durations, and split history were never affected. Reproduced and confirmed with PID-stamped debug logs (parent pid creates + correctly finalizes; child pid emits the warning).
The same parent/child state split hid a second, real bug:
execution_tagmarkers were silently dropped under--forked, because tags were only applied inpytest_runtest_makereport, which only runs in the discarded child.Changes
src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py_owner_pid) at plugin construction.finalize_testis now a no-op in any other process (i.e. a fork-per-test child), eliminating the warning at its source. Safe with xdist: each worker runs its ownpytest_configure, so plugin pid == worker pid.execution_tagmarkers into_tags_by_nodeidduringpytest_collection_modifyitems(which runs in the owning process, before any fork) and apply them infinalize_test. Tag application inpytest_runtest_makereportis kept so runtime-added markers still work in non-forked runs; re-applying the same values is idempotent.Tests
tests/buildkite_test_collector/test_integration_forked.py) running realpytest --forkedend to end, asserting no warnings, correct results, and correct tags. Skips cleanly wherepytest-forked/os.forkare unavailable.Dependencies
pytest-forkedadded to thedevextra (non-Windows only) for the integration test;uv.lockregenerated.Behaviour matrix (verified locally on pytest 8.4.1 and 9.1.1)
--forked-n 2(xdist)-n 2 --forkedResults (pass/fail/skip), failure reasons, and durations verified identical across all modes. Full suite passes (
uv run pytest), pylint 10.00/10.🤖 Generated with Claude Code