Skip to content

Commit 1d986e0

Browse files
Jason Barnettclaude
andcommitted
Capture collection errors (e.g. import failures) in JSON report
When a test file fails to import, pytest fires pytest_collectreport instead of the pytest_runtest_* hooks. The plugin had no handler for this, so collection errors were silently dropped from the JSON report. With pytest-xdist and bktec retries, this caused builds to report green even when a test file failed to collect. Add pytest_collectreport to BuildkitePlugin to capture collection errors as failed TestData entries in the payload. Fixes #106 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 582f67e commit 1d986e0

4 files changed

Lines changed: 251 additions & 1 deletion

File tree

src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,48 @@ def pytest_collection_modifyitems(self, config, items):
4848
config.hook.pytest_deselected(items=unfiltered_items)
4949
items[:] = filtered_items
5050

51+
def pytest_collectreport(self, report):
52+
"""Capture collection errors (e.g. import failures) as failed tests.
53+
54+
When a test file fails to import, pytest fires this hook instead of
55+
the pytest_runtest_* hooks. Without handling it, collection errors
56+
are silently dropped from the JSON report.
57+
"""
58+
if not report.failed:
59+
return
60+
61+
if not report.nodeid:
62+
return
63+
64+
logger.debug('hook=pytest_collectreport nodeid=%s outcome=%s', report.nodeid, report.outcome)
65+
66+
if not self.payload.is_started():
67+
self.payload = self.payload.started()
68+
69+
chunks = report.nodeid.split("::")
70+
scope = "::".join(chunks[:-1])
71+
name = chunks[-1]
72+
file_name = chunks[0]
73+
74+
test_data = TestData.start(
75+
uuid4(),
76+
scope=scope,
77+
name=name,
78+
file_name=file_name,
79+
location=file_name,
80+
)
81+
82+
failure_reason, failure_expanded = failure_reasons(longrepr=report.longrepr)
83+
logger.debug('-> collection error: %s', failure_reason)
84+
85+
test_data = test_data.failed(
86+
failure_reason=failure_reason,
87+
failure_expanded=failure_expanded,
88+
)
89+
test_data = test_data.finish()
90+
91+
self.payload = self.payload.push_test_data(test_data)
92+
5193
def pytest_runtest_logstart(self, nodeid, location):
5294
"""pytest_runtest_logstart hook callback"""
5395
logger.debug('hook=pytest_runtest_logstart nodeid=%s', nodeid)
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
"""Sample test file with an import error.
2+
3+
This file deliberately imports a non-existent module to trigger a collection
4+
error in pytest — the same scenario as importing a removed/renamed symbol
5+
from a real package.
6+
"""
7+
8+
from nonexistent_module import does_not_exist
9+
10+
11+
def test_should_be_reported_as_failed():
12+
"""This test can never run, but should still appear as a failure in the report."""
13+
assert does_not_exist() == 42

tests/buildkite_test_collector/pytest_plugin/test_plugin.py

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from buildkite_test_collector.pytest_plugin import BuildkitePlugin
66

77
from _pytest._code.code import ExceptionInfo
8-
from _pytest.reports import TestReport
8+
from _pytest.reports import CollectReport, TestReport
99

1010
def test_runtest_logstart_with_unstarted_payload(fake_env):
1111
payload = Payload.init(fake_env)
@@ -346,3 +346,110 @@ def test_save_json_payload_concurrent_merge(fake_env, tmp_path, successful_test)
346346

347347
# All writers must have merged their entry
348348
assert len(result) == num_writers
349+
350+
351+
# ---------------------------------------------------------------------------
352+
# pytest_collectreport tests
353+
# ---------------------------------------------------------------------------
354+
355+
def test_pytest_collectreport_import_error(fake_env):
356+
"""Collection errors are captured as failed tests."""
357+
payload = Payload.init(fake_env)
358+
plugin = BuildkitePlugin(payload)
359+
360+
report = CollectReport(
361+
nodeid="tests/foo.py",
362+
outcome="failed",
363+
longrepr="ModuleNotFoundError: No module named 'bar'",
364+
result=None,
365+
)
366+
367+
plugin.pytest_collectreport(report)
368+
369+
assert plugin.payload.is_started()
370+
assert len(plugin.in_flight) == 0
371+
assert len(plugin.payload.data) == 1
372+
373+
test_data = plugin.payload.data[0]
374+
assert test_data.name == "tests/foo.py"
375+
assert test_data.file_name == "tests/foo.py"
376+
assert test_data.scope == ""
377+
assert isinstance(test_data.result, TestResultFailed)
378+
assert "ModuleNotFoundError" in test_data.result.failure_reason
379+
assert test_data.is_finished()
380+
381+
382+
def test_pytest_collectreport_passed_ignored(fake_env):
383+
"""Successful collection reports are ignored."""
384+
payload = Payload.init(fake_env)
385+
plugin = BuildkitePlugin(payload)
386+
387+
report = CollectReport(
388+
nodeid="tests/foo.py",
389+
outcome="passed",
390+
longrepr=None,
391+
result=[],
392+
)
393+
394+
plugin.pytest_collectreport(report)
395+
396+
assert not plugin.payload.is_started()
397+
assert len(plugin.payload.data) == 0
398+
399+
400+
def test_pytest_collectreport_empty_nodeid_ignored(fake_env):
401+
"""The root session collect report (empty nodeid) is ignored."""
402+
payload = Payload.init(fake_env)
403+
plugin = BuildkitePlugin(payload)
404+
405+
report = CollectReport(
406+
nodeid="",
407+
outcome="failed",
408+
longrepr="some error",
409+
result=None,
410+
)
411+
412+
plugin.pytest_collectreport(report)
413+
414+
assert len(plugin.payload.data) == 0
415+
416+
417+
def test_pytest_collectreport_does_not_restart_payload(fake_env):
418+
"""If the payload is already started, pytest_collectreport does not restart it."""
419+
payload = Payload.init(fake_env)
420+
plugin = BuildkitePlugin(payload)
421+
422+
plugin.payload = plugin.payload.started()
423+
original_started_at = plugin.payload.started_at
424+
425+
report = CollectReport(
426+
nodeid="tests/foo.py",
427+
outcome="failed",
428+
longrepr="ModuleNotFoundError: No module named 'bar'",
429+
result=None,
430+
)
431+
432+
plugin.pytest_collectreport(report)
433+
434+
assert plugin.payload.started_at == original_started_at
435+
assert len(plugin.payload.data) == 1
436+
437+
438+
def test_pytest_collectreport_tuple_longrepr(fake_env):
439+
"""Tuple longrepr (path, lineno, message) is handled correctly."""
440+
payload = Payload.init(fake_env)
441+
plugin = BuildkitePlugin(payload)
442+
443+
report = CollectReport(
444+
nodeid="tests/foo.py",
445+
outcome="failed",
446+
longrepr=("tests/foo.py", 8, "ModuleNotFoundError: No module named 'bar'"),
447+
result=None,
448+
)
449+
450+
plugin.pytest_collectreport(report)
451+
452+
assert len(plugin.payload.data) == 1
453+
test_data = plugin.payload.data[0]
454+
assert isinstance(test_data.result, TestResultFailed)
455+
assert "ModuleNotFoundError" in test_data.result.failure_reason
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
"""Integration test: collection errors (import failures) are captured in the JSON report.
2+
3+
When a test file has an import error, pytest fails during collection. The
4+
``pytest_collectreport`` hook on ``BuildkitePlugin`` captures the error and
5+
adds it to the JSON report as a failed test entry.
6+
7+
See: https://github.com/buildkite/test-collector-python/issues/106
8+
"""
9+
10+
import json
11+
import subprocess
12+
import sys
13+
from pathlib import Path
14+
15+
16+
BROKEN_FILE = Path(__file__).parent / "data" / "test_sample_import_error.py"
17+
PASSING_FILE = Path(__file__).parent / "data" / "test_sample_skip.py"
18+
19+
20+
class TestImportErrorReporting:
21+
"""Verify that import errors are captured in the JSON report."""
22+
23+
def _run_pytest(self, tmp_path, test_files, *extra_args):
24+
json_output = tmp_path / "results.json"
25+
cmd = [
26+
sys.executable,
27+
"-m",
28+
"pytest",
29+
*[str(f) for f in test_files],
30+
f"--json={json_output}",
31+
"-v",
32+
*extra_args,
33+
]
34+
result = subprocess.run(cmd, capture_output=True, text=True)
35+
return result, json_output
36+
37+
def test_pytest_exits_nonzero_on_import_error(self, tmp_path):
38+
"""Pytest itself correctly detects the import error and exits non-zero."""
39+
result, _ = self._run_pytest(tmp_path, [BROKEN_FILE])
40+
41+
assert result.returncode != 0, (
42+
"pytest should exit non-zero when a test file has an import error"
43+
)
44+
assert "ModuleNotFoundError" in result.stdout, (
45+
"pytest output should mention the import error"
46+
)
47+
48+
def test_import_error_captured_in_json_report(self, tmp_path):
49+
"""The JSON report captures the collection error as a failed test."""
50+
result, json_output = self._run_pytest(tmp_path, [BROKEN_FILE])
51+
52+
assert result.returncode != 0
53+
54+
assert json_output.exists(), f"JSON not created. stderr:\n{result.stderr}"
55+
data = json.loads(json_output.read_text())
56+
57+
assert len(data) == 1, (
58+
f"Expected 1 entry for the collection error, got {len(data)}: "
59+
f"{[t.get('name') for t in data]}"
60+
)
61+
62+
entry = data[0]
63+
assert entry["result"] == "failed"
64+
assert "ImportError" in (entry.get("failure_reason") or "")
65+
66+
def test_import_error_reported_alongside_passing_tests(self, tmp_path):
67+
"""With --continue-on-collection-errors, both passing tests and the
68+
collection error appear in the report."""
69+
result, json_output = self._run_pytest(
70+
tmp_path,
71+
[BROKEN_FILE, PASSING_FILE],
72+
"--continue-on-collection-errors",
73+
)
74+
75+
assert result.returncode != 0, "pytest should still exit non-zero"
76+
77+
data = json.loads(json_output.read_text())
78+
names = [t["name"] for t in data]
79+
80+
assert len(data) == 6, (
81+
f"Expected 6 entries (5 passing + 1 collection error), got {len(data)}: {names}"
82+
)
83+
84+
assert "test_passing" in names
85+
86+
failed = [t for t in data if t["result"] == "failed"]
87+
assert len(failed) == 1
88+
assert "ImportError" in (failed[0].get("failure_reason") or "")

0 commit comments

Comments
 (0)