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 19 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
4 changes: 2 additions & 2 deletions crates/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,8 @@ 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 {
let location = context.function_meta(&func_id).name.location;
for (func_name, test_function) in tests {
let location = context.function_meta(&test_function.get_id()).name.location;
let file_id = location.file;

// Ignore diagnostics for any file that wasn't the file we saved
Expand Down
67 changes: 46 additions & 21 deletions crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelec
use noirc_driver::{compile_no_check, CompileOptions};
use noirc_frontend::{
graph::CrateName,
hir::{Context, FunctionNameMatch},
node_interner::FuncId,
hir::{def_map::TestFunction, Context, FunctionNameMatch},
};
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};

Expand Down Expand Up @@ -123,30 +122,56 @@ fn run_tests<B: Backend>(
fn run_test<B: Backend>(
backend: &B,
test_name: &str,
main: FuncId,
test_function: TestFunction,
context: &Context,
show_output: bool,
config: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut program = compile_no_check(context, config, main).map_err(|err| {
let report_error = |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())
Err(CliError::Generic(format!("Test '{test_name}' failed to compile")))
};

let program = compile_no_check(context, config, test_function.get_id());
match program {
Ok(mut program) => {
// Note: We don't need to use the optimized ACIR here
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.
let circuit_execution =
execute_circuit(backend, program.circuit, WitnessMap::new(), show_output);

if test_function.get_expect_failure_flag() {
Ethan-000 marked this conversation as resolved.
Show resolved Hide resolved
match circuit_execution {
Ok(_) => Err(CliError::Generic(format!("Test '{test_name}' should fail"))),
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
Err(_) => Ok(()),
}
} else {
match circuit_execution {
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())
}
}
}
}
Err(err) => {
if test_function.get_expect_failure_flag() {
if !err.diagnostic.message.contains("Failed constraint") {
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
report_error(err)
} else {
Ok(())
}
} else {
report_error(err)
}
}
}
}
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
34 changes: 32 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,23 @@ impl CrateDefMap {
pub fn get_all_test_functions<'a>(
&'a self,
interner: &'a NodeInterner,
) -> impl Iterator<Item = FuncId> + 'a {
) -> impl Iterator<Item = TestFunction> + 'a {
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| {
matches!(interner.function_meta(id).attributes, Some(Attribute::Test { .. }))
})
.map(|id| {
if interner.function_meta(&id).attributes
== Some(Attribute::Test { expect_failure: true })
phated marked this conversation as resolved.
Show resolved Hide resolved
phated marked this conversation as resolved.
Show resolved Hide resolved
{
TestFunction::new(id, true)
} else {
TestFunction::new(id, false)
}
})
})
}

Expand Down Expand Up @@ -221,3 +232,22 @@ impl std::ops::IndexMut<LocalModuleId> for CrateDefMap {
&mut self.modules[local_module_id.0]
}
}

pub struct TestFunction {
id: FuncId,
expect_failure: bool,
}

impl TestFunction {
fn new(id: FuncId, expect_failure: bool) -> Self {
TestFunction { id, expect_failure }
}

pub fn get_id(&self) -> FuncId {
self.id
}

pub fn get_expect_failure_flag(&self) -> bool {
self.expect_failure
}
}
22 changes: 12 additions & 10 deletions crates/noirc_frontend/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use def_map::{Contract, CrateDefMap};
use fm::FileManager;
use std::collections::HashMap;

use self::def_map::TestFunction;

/// Helper object which groups together several useful context objects used
/// during name resolution. Once name resolution is finished, only the
/// def_interner is required for type inference and monomorphization.
Expand Down Expand Up @@ -107,22 +109,22 @@ impl Context {
&self,
crate_id: &CrateId,
pattern: FunctionNameMatch,
) -> Vec<(String, FuncId)> {
) -> Vec<(String, TestFunction)> {
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| {
let fully_qualified_name = self.fully_qualified_function_name(crate_id, &id);
.filter_map(|test_function| {
let fully_qualified_name =
self.fully_qualified_function_name(crate_id, &test_function.get_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, test_function)),
FunctionNameMatch::Exact(pattern) => (&fully_qualified_name == pattern)
.then_some((fully_qualified_name, test_function)),
FunctionNameMatch::Contains(pattern) => fully_qualified_name
.contains(pattern)
.then_some((fully_qualified_name, test_function)),
}
})
.collect()
Expand Down
3 changes: 2 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,8 @@
self.push_err(ResolverError::DistinctNotAllowed { ident: func.name_ident().clone() });
}

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

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

Check warning on line 1938 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
24 changes: 20 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,7 @@
Builtin(String),
Oracle(String),
Deprecated(Option<String>),
Test,
Test { expect_failure: bool },
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
Custom(String),
}

Expand All @@ -335,7 +335,13 @@
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 { expect_failure } => {
if !expect_failure {
write!(f, "#[test]")
} else {
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 +397,17 @@

Attribute::Deprecated(name.trim_matches('"').to_string().into())
}
["test"] => Attribute::Test,
["test"] => Attribute::Test { expect_failure: 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 { expect_failure: true }
}
tokens => {
tokens.iter().try_for_each(|token| validate(token))?;
Attribute::Custom(word.to_owned())
Expand Down Expand Up @@ -431,7 +447,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 +512,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 515 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 +554,7 @@
"Field" => Keyword::Field,
"fn" => Keyword::Fn,
"for" => Keyword::For,
"fmtstr" => Keyword::FormatString,

Check warning on line 557 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