Having a flag #[should_panic] for tests that are meant to fail

[harshnambiar]

Problem

Suppose I have a set of methods that have an assert statement inside them (say Denominator should not be 0, the maximum permissible right shifts be 100, etc), then to test them with nargo test, there should be a mechanism to be able to write negative tests that 'pass' when the method 'fails'. After discussing yesterday on the dev hour call with the Noir team, have created this feature request.

Happy Case

One way would be to have #[should_panic] as a tag, similar to Rust in Noir. It would just need to invert the passing or failing or the tagged test case.

Alternatives Considered

Alternatively, if there were a way to error handle such a function, and then assert that the error thrown was the same one we expected to be thrown, that would work as well.

Additional Context

No response

Would you like to submit a PR for this Issue?

No

Support Needs

No response

[kevaundray]

@guipublic assigned this to you, the easiest way to do this in the short term, would be to add a assert_false method which would desugar into an assert and a not

[TomAFrench]

The addition of assert_not wouldn't resolve this issue imo. Consider the function require_equal and test below:

fn require_equal(x: Field, y: Field) {
    assert(x == y);
}


#[test]
fn ne_should_panic() {
    require_equal(1, 2)
}

The assert call in require_equal is part of the program's business logic and so can't be changed for testing purposes. There's then no way to rewrite the test to test for panics. This example is a bit contrived as you could rewrite require_equal as is_equal and apply the constraint in the test function, but in general test cases will need to check constraints which exist inside the function being tested (e.g. range checks to prevent overflow).

I also think that we don't need a assert_not(x) function as it's sufficiently covered by assert(!x). We can add a assert_ne(x, y) function when we add assert_eq() to match Rust however.

[harshnambiar]

The addition of assert_not wouldn't resolve this issue imo. Consider the function require_equal and test below:

fn require_equal(x: Field, y: Field) {
    assert(x == y);
}


#[test]
fn ne_should_panic() {
    require_equal(1, 2)
}

The assert call in require_equal is part of the program's business logic and so can't be changed for testing purposes. There's then no way to rewrite the test to test for panics. This example is a bit contrived as you could rewrite require_equal as is_equal and apply the constraint in the test function, but in general test cases will need to check constraints which exist inside the function being tested (e.g. range checks to prevent overflow).

I also think that we don't need a assert_not(x) function as it's sufficiently covered by assert(!x). We can add a assert_ne(x, y) function when we add assert_eq() to match Rust however.

Thank you Tom for highlighting this. Suppose the method is signed int division and someone is just importing the library, then they would not be able to use assert_false on the final repository test cases. A should_panic tag would ensure that any outcome other than the test "passing", including those without an assert, say someone trying to mutate a string in Noir currently leading to a failure of the method, would pass because we are merely inverting the test case result.

[TomAFrench]

If we can add an attribute for this into the lexer then we could support this relatively easily imo. We'd just need to invert this match statement (or interact with the ACVM directly so we can query the ACVMStatus::Success vs ACVMStatus::Failure)

noir/crates/nargo_cli/src/cli/test_cmd.rs

Lines 104 to 116 in 5526c52

	// Run the backend to ensure the PWG evaluates functions like std::hash::pedersen,
	// otherwise constraints involving these expressions will not error.
	match execute_circuit(backend, program.circuit, WitnessMap::new()) {
	Ok(_) => Ok(()),
	Err(error) => {
	let writer = StandardStream::stderr(ColorChoice::Always);
	let mut writer = writer.lock();
	writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).ok();
	writeln!(writer, "failed").ok();
	writer.reset().ok();
	Err(error.into())
	}
	}

[Ethan-000]

Kev typing:

We can do:

#[test(should_fail)]

since we don't have support for multiple attributes