-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Attribute panics to the mdtests that cause them #15241
Conversation
pub(crate) fn catch_unwind<F, R>(f: F) -> Result<R, PanicError> | ||
pub fn catch_unwind<F, R>(f: F) -> Result<R, PanicError> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the chatter in this crate is just to publicize this existing helper function.
crates/red_knot_test/src/lib.rs
Outdated
info.info | ||
.to_string() | ||
.split('\n') | ||
.map(String::from) | ||
.collect(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not include the full backtrace of the panic, since that seemed to be more verbose than we need at this point. If we want it, we just change info.info
to info
here.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this looks great! :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat
* main: [`ruff`] Avoid reporting when `ndigits` is possibly negative (`RUF057`) (#15234) Attribute panics to the mdtests that cause them (#15241) Show errors for attempted fixes only when passed `--verbose` (#15237) [`RUF`] Add rule to detect empty literal in deque call (`RUF025`) (#15104) TD003: remove issue code length restriction (#15175) Preserve multiline implicit concatenated strings in docstring positions (#15126) [`pyflakes`] Ignore errors in `@no_type_check` string annotations (`F722`, `F821`) (#15215) style(AIR302): rename removed_airflow_plugin_extension as check_airflow_plugin_extension (#15233) [`pylint`] Re-implement `unreachable` (`PLW0101`) (#10891) refactor(AIR303): move duplicate qualified_name.to_string() to Diagnostic argument (#15220) Misc. clean up to rounding rules (#15231) Avoid syntax error when removing int over multiple lines (#15230) Migrate renovate config (#15228) Remove `Type::tuple` in favor of `TupleType::from_elements` (#15218)
Hmm, this PR unfortunately now seems to be suppressing some of the internal error messages from ruff/crates/red_knot_test/src/lib.rs Lines 32 to 37 in b264489
On a PR branch I'm working on, I got this (not very informative!) test failure: If I revert this commit on that PR branch, I get this instead, which is much more helpful: |
I opened #15317 so we don't forget about this 👍 |
Thanks Alex! I'll take a look |
This updates the mdtest harness to catch any panics that occur during type checking, and to display the panic message as an mdtest failure. (We don't know which specific line causes the failure, so we attribute panics to the first line of the test case.)
Sample output (for an artificial panic):
Closes #13899