-
Notifications
You must be signed in to change notification settings - Fork 200
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: Handle should_fail_with
case
#2541
Conversation
When I run this PR with the code:
It still passes, even though I'd expect it to fail. Edit: Oh I see, we're just testing if the actual message contains the expected message. I didn't expect that but now that I know it makes sense. |
Just asked Lasse and he would expect that to fail, so I'll change it! |
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.
Mainly some style suggestions, but non-blocking. It would nice to add nargo test
capabilities to our testing suite as well, but that is for a separate PR.
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Co-authored-by: Maxim Vezenov <mvezenov@gmail.com>
Fixes #1965 with should fail test cases. Should be redone with complete failure messages when noir-lang/noir#2541 is ready.
Description
resolves #2484 .
I think the code for testing could do with quite a bit of love and refactoring.
Problem*
Resolves
Summary*
Documentation
This PR requires documentation updates when merged.
Additional Context
PR Checklist*
cargo fmt
on default settings.