Skip to content

feat(testing): improve error message quality in miden-testing#2251

Merged
PhilippGackstatter merged 11 commits into0xMiden:nextfrom
avorylli:feat(testing)-improve-error-message-quality-in-miden-testing
Jan 17, 2026
Merged

feat(testing): improve error message quality in miden-testing#2251
PhilippGackstatter merged 11 commits into0xMiden:nextfrom
avorylli:feat(testing)-improve-error-message-quality-in-miden-testing

Conversation

@avorylli
Copy link
Copy Markdown
Contributor

@avorylli avorylli commented Jan 9, 2026

Introduce ExecError newtype wrapper around ExecutionError that uses PrintDiagnostic for better error formatting.

Changes:

  • Add ExecError wrapper with improved diagnostics
  • Update TransactionContext::execute_code to return ExecError
  • Replace all miette::Result with anyhow::Result in tests
  • Remove all .into_diagnostic() calls
  • Update assert_execution_error! macro to handle ExecError

Closes #2040

@avorylli
Add ExecError wrapper for better error reporting
ffa15c0
@avorylli
Add errors module to tx_context
d8a6ce4
@avorylli
Refactor execute_code to use ExecError
a5036a6 
Updated error handling in execute_code function to use ExecError instead of ExecutionError.
@avorylli
Wrap ExecutionError in ExecError for assertions
b8af7d4
@avorylli
Refactor error handling to use 'anyhow' in tests
0565baf 
Refactor error handling in tests to use 'anyhow' for better context management. Update function signatures to return 'anyhow::Result' instead of 'miette::Result'.
@avorylli
Replace miette with anyhow for error handling
e7e0727
@avorylli
Update lib.rs
d3e1b53
@avorylli
Update Cargo.toml
7ab3912
@avorylli
Copy link
Copy Markdown
Contributor Author

avorylli commented Jan 9, 2026

Also I have a question. Firstly, before pushing a pr I run these commands:

  1. make check
  2. make test
  3. make clippy
  4. make lint

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?

@avorylli avorylli closed this Jan 9, 2026
@avorylli avorylli reopened this Jan 9, 2026
@avorylli
Copy link
Copy Markdown
Contributor Author

avorylli commented Jan 9, 2026

Sorry, closed it accidentally

@PhilippGackstatter
Copy link
Copy Markdown
Contributor

PhilippGackstatter commented Jan 14, 2026

So the question is are these all the commands needed to be ran or is there anything else?

I think make lint and make doc should cover most of the CI checks.

PhilippGackstatter
PhilippGackstatter reviewed Jan 15, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@PhilippGackstatter PhilippGackstatter left a comment •
edited
Loading

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!

@PhilippGackstatter
Copy link
Copy Markdown
Contributor

PhilippGackstatter commented Jan 16, 2026

Unfortunately, the merge conflicts weren't resolved correctly and CI isn't passing now. Please check locally with make test and make lint that everything is working before requesting a review, thank you!

@bobbinth
Copy link
Copy Markdown
Contributor

bobbinth commented Jan 16, 2026

Seems like the CI is still not passing. @avorylli - is this something you can fix?

@PhilippGackstatter PhilippGackstatter force-pushed the feat(testing)-improve-error-message-quality-in-miden-testing branch from e637d19 to e5abc22 Compare January 17, 2026 08:06
@PhilippGackstatter PhilippGackstatter added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Jan 17, 2026
PhilippGackstatter
PhilippGackstatter approved these changes Jan 17, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@PhilippGackstatter PhilippGackstatter left a 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.

@PhilippGackstatter PhilippGackstatter merged commit 83e4ef3 into 0xMiden:next Jan 17, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PhilippGackstatter PhilippGackstatter PhilippGackstatter approved these changes

Assignees

No one assigned

Labels

no changelog This PR does not require an entry in the `CHANGELOG.md` file

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Improve error message quality in miden-testing

3 participants

@avorylli @PhilippGackstatter @bobbinth