-
Notifications
You must be signed in to change notification settings - Fork 2
Put testset exceptions in the test record. #44
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
Conversation
|
One thing we should check is if throwing a custom exception works. Something like: I assume (partially based on the Malt docs), that exceptions that are only defined on the worker process would lead to a serialization failure, which we should document as a caveat. Thankfully the 1.0 interface requires the module and thus encourages the user to load the package on the main process, so it should only occur for exceptions for package that are only loaded on the worker, or newly defined on the worker. |
|
I'm only storing TestSetExceptions though, all others are rethrown (and would get converted to Strings by Malt). |
| function print_test_crashed(::Type{TestRecord}, wrkr, test, ctx::TestIOContext) | ||
| lock(ctx.lock) | ||
| try | ||
| printstyled(ctx.stderr, test, color = :red) | ||
| printstyled( | ||
| ctx.stderr, | ||
| lpad("($wrkr)", ctx.name_align - textwidth(test) + 1, " "), " |", | ||
| " "^ctx.elapsed_align, " crashed at $(now())\n", color = :red |
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.
Could this still print the time? It's sometimes helpful to know how long it takes to crash
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.
Crashes are highly unlikely, since we now catch every exception worker-side, so I rather wanted to keep the reporting logic as simple as possible. Besides, the testset right below will always report the time it took, even without --verbose:
│ │ ──────────────── CPU ──────────────── │
Test (Worker) │ Time (s) │ GC (s) │ GC % │ Alloc (MB) │ RSS (MB) │
basic (2) | crashed at 2025-10-15T08:32:27.239
Test Summary: | Error Total Time
Overall | 1 1 5.6s
basic | 1 1 3.8s
FAILURE
This makes it easier to differentiate between failures and crashes.
8b14a04 to
87f190e
Compare
Interestingly, this now works as-is, even when the main process doesn't have this definition. Try adding that to I'm not sure how that works? Is it actually serializing the struct definition too? This is probably something Malt doesn't support. |
This makes it easier to differentiate between failures and crashes.
Should also make it easier to switch to Malt.jl which doesn't automatically serialize thrown exceptions (which would be bad in order to reconstruct testset failures).