-
Notifications
You must be signed in to change notification settings - Fork 106
feat(testing): improve error message quality in miden-testing #2251
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
feat(testing): improve error message quality in miden-testing #2251
Conversation
Updated error handling in execute_code function to use ExecError instead of ExecutionError.
Refactor error handling in tests to use 'anyhow' for better context management. Update function signatures to return 'anyhow::Result' instead of 'miette::Result'.
|
Also I have a question. Firstly, before pushing a pr I run these commands:
As I understood these are the necessary command to pass all the CI here in miden base, So the question is are these all the commands needed to be ran or is there anything else? |
|
Sorry, closed it accidentally |
I think |
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.
Looks great! I left two small comments after which we can merge.
Could you also merge in latest next and run make build-no-std to rebuild the generated files and commit them? Thanks!
|
Unfortunately, the merge conflicts weren't resolved correctly and CI isn't passing now. Please check locally with |
|
Seems like the CI is still not passing. @avorylli - is this something you can fix? |
…)-improve-error-message-quality-in-miden-testing
e637d19 to
e5abc22
Compare
PhilippGackstatter
left a comment
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.
Looks good to me!
I reverted to the commit I last reviewed which was already 99% there, merged next and made small adjustments.
Introduce
ExecErrornewtype wrapper aroundExecutionErrorthat usesPrintDiagnosticfor better error formatting.Changes:
ExecErrorwrapper with improved diagnosticsTransactionContext::execute_codeto returnExecErrormiette::Resultwithanyhow::Resultin tests.into_diagnostic()callsassert_execution_error!macro to handleExecErrorCloses #2040