Handle failure in setup eg fixtures

[pda]

Currently, errors during setup (e.g. fixtures) cause a result of unknown without any error/failure information.

For example:

import pytest

@pytest.fixture
def greeting():
    return "hello" + 1 # TypeError: can only concatenate str (not "int") to str

def test_with_fixture(greeting):
    assert greeting == "hello"

Because the setup phase failed, the call phase never runs, and so the assertion never runs.

This pull request adds handling for report.when == 'setup' and report.failed, which captures this scenario.

Related:

• Stacked on Clearer failure/errors in failure_reason & failure_expanded #64
• https://docs.pytest.org/en/7.1.x/reference/reference.html#pytest.hookspec.pytest_runtest_protocol

[Copilot]

Pull Request Overview

This PR enhances the pytest plugin to properly handle test failures that occur during the setup phase (e.g., fixture failures), which were previously resulting in "unknown" status without failure information.

Key changes include:

• Added support for capturing failures during the setup phase in addition to the call phase
• Introduced a new failure_reasons module to extract detailed failure information from pytest's longrepr
• Enhanced TestResultFailed to include expanded failure details with backtrace information

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/buildkite_test_collector/pytest_plugin/buildkite_plugin.py	Modified to handle setup phase failures and refactored failure processing logic
src/buildkite_test_collector/pytest_plugin/failure_reasons.py	New module for extracting failure reasons and expanded details from pytest longrepr
src/buildkite_test_collector/collector/payload.py	Enhanced TestResultFailed to include failure_expanded field and updated serialization
tests/buildkite_test_collector/pytest_plugin/test_plugin.py	Added comprehensive tests for various failure scenarios including setup failures
tests/buildkite_test_collector/conftest.py	Updated failed_test fixture to include failure_expanded data
tests/buildkite_test_collector/collector/test_payload.py	Updated test expectations for new failure_expanded field

[Copilot]

Using hasattr for method existence checking followed by method call can be fragile. Consider using getattr(entry, 'getsource', None) or wrapping in a try-except block for more robust error handling.

Suggested change

	source = entry.getsource() if hasattr(entry, "getsource") else None
	if source:
	expanded.extend(str(line) for line in source)
	getsource_method = getattr(entry, "getsource", None)
	if callable(getsource_method):
	source = getsource_method()
	if source:
	expanded.extend(str(line) for line in source)

[Copilot]

Direct attribute access to er.reprcrash.message without null checking could raise AttributeError if reprcrash is None. Consider adding a null check or using defensive programming.

Suggested change

	case ExceptionRepr() as er if er.reprcrash is not None:
	failure_reason = er.reprcrash.message # e.g. "ZeroDivisionError: division by zero"
	case ExceptionRepr() as er:
	if er.reprcrash is not None:
	failure_reason = er.reprcrash.message # e.g. "ZeroDivisionError: division by zero"
	else:
	failure_reason = None

[gchan]

Do we want to add a similar debug message to update_test_result?

[pda]

I mostly wanted to add it to the PyTest hooks themselves…
the update_test_result method does debug-log when it takes action…

                logger.debug('-> test passed')
                logger.debug('-> test skipped')
                logger.debug('-> test failed: %s', failure_reason)

I think it's okay for it to remain silent when it doesn't do anything.

Also I wasn't sure I should even leave in as much debugging as I did.
I added it for my own purposes in the moment, and tried to make it a bit more consistent than it was, but maybe it should be pruned back down a bit. Not sure!