Skip to content

fix: don't panic when a source file disappears during diagnostic rendering#665

Closed
MatthewMckee4 wants to merge 1 commit intomainfrom
fix/builtin-fixture-panics
Closed

fix: don't panic when a source file disappears during diagnostic rendering#665
MatthewMckee4 wants to merge 1 commit intomainfrom
fix/builtin-fixture-panics

Conversation

@MatthewMckee4
Copy link
Copy Markdown
Member

@MatthewMckee4 MatthewMckee4 commented Apr 11, 2026

Summary

utils::source_file is called from the diagnostic rendering path whenever we
need to attribute a span back to a test file on disk. It used to read the file
with .expect(\"Failed to read source file\"), which races against test files
being moved or deleted between discovery and diagnostic rendering and tears down
the entire worker process:

pub(crate) fn source_file(path: &Utf8Path) -> SourceFile {
    SourceFileBuilder::new(
        path.as_str(),
        std::fs::read_to_string(path).expect(\"Failed to read source file\"),
    )
    .finish()
}

It now falls back to an empty-bodied SourceFile via unwrap_or_default()
the filename is still shown in diagnostics, but spans that reference the missing
file render as empty instead of crashing the run. A unit test exercises the
non-existent-path branch so the function is guaranteed not to panic on a missing
file.

This PR originally also tried to defuse two .expect(\"builtin fixtures to not fail\") panics in package_runner::run_fixture. Those sites were removed
entirely by #645, which rewrote built-in fixtures to run as pure Python, so
there is no longer a "this fixture has no AST" case at the Rust layer. The
rebase onto post-#645 main dropped those changes.

Test Plan

  • just test (818 tests passing)
  • uvx prek run -a

@MatthewMckee4 MatthewMckee4 added bug Something isn't working diagnostics Related to diagnostics and output of Karva extensions/fixtures Related to fixtures labels Apr 11, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 11, 2026

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 1 untouched benchmark


Comparing fix/builtin-fixture-panics (dc023ed) with main (9d04e61)

Open in CodSpeed

…ering

`utils::source_file` called `read_to_string(path).expect(...)` on the
diagnostic rendering path. If a test file disappears between discovery
and diagnostic rendering, the runner now returns an empty-bodied
`SourceFile` rather than crashing.
@MatthewMckee4 MatthewMckee4 force-pushed the fix/builtin-fixture-panics branch from f3db211 to dc023ed Compare April 11, 2026 17:09
@MatthewMckee4 MatthewMckee4 changed the title fix: replace .expect() panics in built-in fixture and source-file paths fix: don't panic when a source file disappears during diagnostic rendering Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working diagnostics Related to diagnostics and output of Karva extensions/fixtures Related to fixtures

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant