Skip to content
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: Add test(should_fail) attribute for tests that are meant to fail #2418

Merged
merged 35 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
04f20b9
test(should_fail)
Ethan-000 Aug 23, 2023
d684879
update comment
Ethan-000 Aug 23, 2023
b8dc4f5
make only unsatisfied constraint success
Ethan-000 Aug 23, 2023
54137b1
testfunc type alias to replace tuple
Ethan-000 Aug 23, 2023
5ee5dfb
inline comment
Ethan-000 Aug 23, 2023
2268b1e
change to expect failure
Ethan-000 Aug 23, 2023
4c5145f
change Test(bool) to Test{}
Ethan-000 Aug 23, 2023
4bd422e
clippy
Ethan-000 Aug 23, 2023
09b0799
remove duplication
Ethan-000 Aug 23, 2023
148ab5c
remove duplication
Ethan-000 Aug 23, 2023
f165f48
remove duplication
Ethan-000 Aug 23, 2023
52ca47f
add some spacing
Ethan-000 Aug 23, 2023
131eec5
Update crates/noirc_frontend/src/hir/resolution/resolver.rs
Ethan-000 Aug 23, 2023
83a281e
Update crates/noirc_frontend/src/hir/def_map/mod.rs
Ethan-000 Aug 23, 2023
f550b30
make testfunc a struct
Ethan-000 Aug 23, 2023
a0c3771
fix typo
Ethan-000 Aug 23, 2023
34f3af7
rename testfunc to testfunction
Ethan-000 Aug 24, 2023
0a33986
only pass in test function
Ethan-000 Aug 24, 2023
aa1a719
Update crates/nargo_cli/src/cli/test_cmd.rs
Ethan-000 Aug 24, 2023
d741ef3
use pattern matching
Ethan-000 Aug 24, 2023
49bd10a
use a single filtermap untested
Ethan-000 Aug 24, 2023
3d96f3f
clippy
Ethan-000 Aug 24, 2023
77d9c33
use if le
Ethan-000 Aug 24, 2023
9d55fbf
Update crates/noirc_frontend/src/hir/def_map/mod.rs
Ethan-000 Aug 24, 2023
8f8106a
chore: use general scope variant instead of expect_failure (#2427)
kevaundray Aug 24, 2023
74ef56f
fix clippy
Ethan-000 Aug 24, 2023
a49655d
use cfg test to fix clippy
Ethan-000 Aug 24, 2023
cb36da0
Update crates/nargo_cli/src/cli/test_cmd.rs
Ethan-000 Aug 24, 2023
d1188a2
rename function to should_fail
Ethan-000 Aug 24, 2023
6cddf30
fix clippy by putting a cfg on mod tests
kevaundray Aug 24, 2023
53d4bdf
Merge branch 'e/should_panic' of github.com:noir-lang/noir into e/sho…
kevaundray Aug 24, 2023
f1ff2ad
Merge branch 'e/should_panic' of github.com:noir-lang/noir into e/sho…
kevaundray Aug 24, 2023
8b26cb8
add comments
kevaundray Aug 24, 2023
239484f
write error with color
Ethan-000 Aug 24, 2023
095741b
add more comments regarding programs that are never satisfiable
kevaundray Aug 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ fn on_code_lens_request(
let tests = context
.get_all_test_functions_in_crate_matching(&crate_id, FunctionNameMatch::Anything);

for (func_name, func_id) in tests {
for (func_name, func_id, _should_fail) in tests {
let location = context.function_meta(&func_id).name.location;
let file_id = location.file;

Expand Down
64 changes: 43 additions & 21 deletions crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,20 @@
let writer = StandardStream::stderr(ColorChoice::Always);
let mut writer = writer.lock();

for (test_name, test_function) in test_functions {
for (test_name, test_function, should_fail) in test_functions {
write!(writer, "[{}] Testing {test_name}... ", package.name)
.expect("Failed to write to stdout");
writer.flush().expect("Failed to flush writer");

match run_test(backend, &test_name, test_function, &context, show_output, compile_options) {
match run_test(
backend,
&test_name,
test_function,
should_fail,
&context,
show_output,
compile_options,
) {
Ok(_) => {
writer
.set_color(ColorSpec::new().set_fg(Some(Color::Green)))
Expand Down Expand Up @@ -124,29 +132,43 @@
backend: &B,
test_name: &str,
main: FuncId,
should_fail: bool,
phated marked this conversation as resolved.
Show resolved Hide resolved
context: &Context,
show_output: bool,
config: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut program = compile_no_check(context, config, main).map_err(|err| {
noirc_errors::reporter::report_all(&context.file_manager, &[err], config.deny_warnings);
CliError::Generic(format!("Test '{test_name}' failed to compile"))
})?;

// Note: We could perform this test using the unoptimized ACIR as generated by `compile_no_check`.
program.circuit = optimize_circuit(backend, program.circuit).unwrap().0;

// 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(), show_output) {
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())
if !should_fail {
let mut program = compile_no_check(context, config, main).map_err(|err| {
noirc_errors::reporter::report_all(&context.file_manager, &[err], config.deny_warnings);
CliError::Generic(format!("Test '{test_name}' failed to compile"))
})?;
phated marked this conversation as resolved.
Show resolved Hide resolved

// Note: We could perform this test using the unoptimized ACIR as generated by `compile_no_check`.

Check warning on line 146 in crates/nargo_cli/src/cli/test_cmd.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (unoptimized)
program.circuit = optimize_circuit(backend, program.circuit).unwrap().0;

// 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(), show_output) {
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())
}
}
} else {
let program = compile_no_check(context, config, main);
if program.is_err() {
return Ok(());
}
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
let mut program = program.unwrap();
program.circuit = optimize_circuit(backend, program.circuit).unwrap().0;
match execute_circuit(backend, program.circuit, WitnessMap::new(), show_output) {
Ok(_) => Err(CliError::Generic(format!("Test '{test_name}' should fail"))),
Err(_) => Ok(()),
}
}
}
2 changes: 1 addition & 1 deletion crates/noirc_frontend/src/ast/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl From<FunctionDefinition> for NoirFunction {
let kind = match fd.attribute {
Some(Attribute::Builtin(_)) => FunctionKind::Builtin,
Some(Attribute::Foreign(_)) => FunctionKind::LowLevel,
Some(Attribute::Test) => FunctionKind::Normal,
Some(Attribute::Test(_)) => FunctionKind::Normal,
Some(Attribute::Oracle(_)) => FunctionKind::Oracle,
Some(Attribute::Deprecated(_)) | None => FunctionKind::Normal,
Some(Attribute::Custom(_)) => FunctionKind::Normal,
Expand Down
14 changes: 12 additions & 2 deletions crates/noirc_frontend/src/hir/def_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,22 @@ impl CrateDefMap {
pub fn get_all_test_functions<'a>(
&'a self,
interner: &'a NodeInterner,
) -> impl Iterator<Item = FuncId> + 'a {
) -> impl Iterator<Item = (FuncId, bool)> + 'a {
phated marked this conversation as resolved.
Show resolved Hide resolved
self.modules.iter().flat_map(|(_, module)| {
module
.value_definitions()
.filter_map(|id| id.as_function())
.filter(|id| interner.function_meta(id).attributes == Some(Attribute::Test))
.filter(|id| {
interner.function_meta(id).attributes == Some(Attribute::Test(true))
|| interner.function_meta(id).attributes == Some(Attribute::Test(false))
})
.map(|id| {
if interner.function_meta(&id).attributes == Some(Attribute::Test(true)) {
(id, true)
} else {
(id, false)
}
})
})
}

Expand Down
17 changes: 8 additions & 9 deletions crates/noirc_frontend/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,22 +107,21 @@ impl Context {
&self,
crate_id: &CrateId,
pattern: FunctionNameMatch,
) -> Vec<(String, FuncId)> {
) -> Vec<(String, FuncId, bool)> {
phated marked this conversation as resolved.
Show resolved Hide resolved
let interner = &self.def_interner;
let def_map = self.def_map(crate_id).expect("The local crate should be analyzed already");

def_map
.get_all_test_functions(interner)
.filter_map(|id| {
.filter_map(|(id, not_fail)| {
let fully_qualified_name = self.fully_qualified_function_name(crate_id, &id);
match &pattern {
FunctionNameMatch::Anything => Some((fully_qualified_name, id)),
FunctionNameMatch::Exact(pattern) => {
(&fully_qualified_name == pattern).then_some((fully_qualified_name, id))
}
FunctionNameMatch::Contains(pattern) => {
fully_qualified_name.contains(pattern).then_some((fully_qualified_name, id))
}
FunctionNameMatch::Anything => Some((fully_qualified_name, id, not_fail)),
FunctionNameMatch::Exact(pattern) => (&fully_qualified_name == pattern)
.then_some((fully_qualified_name, id, not_fail)),
FunctionNameMatch::Contains(pattern) => fully_qualified_name
.contains(pattern)
.then_some((fully_qualified_name, id, not_fail)),
}
})
.collect()
Expand Down
4 changes: 3 additions & 1 deletion crates/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,9 @@
self.push_err(ResolverError::DistinctNotAllowed { ident: func.name_ident().clone() });
}

if attributes == Some(Attribute::Test) && !parameters.is_empty() {
if (attributes == Some(Attribute::Test(true)) || attributes == Some(Attribute::Test(false)))
&& !parameters.is_empty()
{
self.push_err(ResolverError::TestFunctionHasParameters {
span: func.name_ident().span(),
});
Expand Down Expand Up @@ -1934,7 +1936,7 @@
println(f"I want to print {0}");

let new_val = 10;
println(f"randomstring{new_val}{new_val}");

Check warning on line 1939 in crates/noirc_frontend/src/hir/resolution/resolver.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (randomstring)
}
fn println<T>(x : T) -> T {
x
Expand Down
20 changes: 16 additions & 4 deletions crates/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,8 @@
Builtin(String),
Oracle(String),
Deprecated(Option<String>),
Test,
// Boolean flag that represents if the test should fail
Test(bool),
phated marked this conversation as resolved.
Show resolved Hide resolved
Custom(String),
}

Expand All @@ -335,7 +336,8 @@
Attribute::Foreign(ref k) => write!(f, "#[foreign({k})]"),
Attribute::Builtin(ref k) => write!(f, "#[builtin({k})]"),
Attribute::Oracle(ref k) => write!(f, "#[oracle({k})]"),
Attribute::Test => write!(f, "#[test]"),
Attribute::Test(false) => write!(f, "#[test]"),
Attribute::Test(true) => write!(f, "#[test(should_fail)]"),
Attribute::Deprecated(None) => write!(f, "#[deprecated]"),
Attribute::Deprecated(Some(ref note)) => write!(f, r#"#[deprecated("{note}")]"#),
Attribute::Custom(ref k) => write!(f, "#[{k}]"),
Expand Down Expand Up @@ -391,7 +393,17 @@

Attribute::Deprecated(name.trim_matches('"').to_string().into())
}
["test"] => Attribute::Test,
["test"] => Attribute::Test(false),
["test", name] => {
if name != &"should_fail" {
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
return Err(LexerErrorKind::MalformedFuncAttribute {
span,
found: word.to_owned(),
});
}

Attribute::Test(true)
}
tokens => {
tokens.iter().try_for_each(|token| validate(token))?;
Attribute::Custom(word.to_owned())
Expand Down Expand Up @@ -431,7 +443,7 @@
Attribute::Builtin(string) => string,
Attribute::Oracle(string) => string,
Attribute::Deprecated(Some(string)) => string,
Attribute::Test | Attribute::Deprecated(None) => "",
Attribute::Test(_) | Attribute::Deprecated(None) => "",
Attribute::Custom(string) => string,
}
}
Expand Down Expand Up @@ -496,7 +508,7 @@
Keyword::Field => write!(f, "Field"),
Keyword::Fn => write!(f, "fn"),
Keyword::For => write!(f, "for"),
Keyword::FormatString => write!(f, "fmtstr"),

Check warning on line 511 in crates/noirc_frontend/src/lexer/token.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (fmtstr)
Keyword::Global => write!(f, "global"),
Keyword::If => write!(f, "if"),
Keyword::Impl => write!(f, "impl"),
Expand Down Expand Up @@ -538,7 +550,7 @@
"Field" => Keyword::Field,
"fn" => Keyword::Fn,
"for" => Keyword::For,
"fmtstr" => Keyword::FormatString,

Check warning on line 553 in crates/noirc_frontend/src/lexer/token.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (fmtstr)
"global" => Keyword::Global,
"if" => Keyword::If,
"impl" => Keyword::Impl,
Expand Down
Loading