Skip to content

Commit

Permalink
feat(nargo): Add nargo test command to run all unit tests (#728)
Browse files Browse the repository at this point in the history
* More progress on nargo test

* Get 'nargo test' working

* Fix clippy suggestion

* Add error and address comments

* Remove comment and stop before proof creation

* Color output

* clippy

* Rename prove function

* Run the backend again

* Fix clippy suggestion

* PR feedback
  • Loading branch information
jfecher authored Feb 3, 2023
1 parent 2d6b27c commit 2e1dc82
Show file tree
Hide file tree
Showing 17 changed files with 298 additions and 134 deletions.
146 changes: 73 additions & 73 deletions Cargo.lock

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions crates/nargo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ termcolor = "1.1.2"
tempdir = "0.3.7"

# Backends
aztec_backend = { optional = true, package = "barretenberg_static_lib", git = "https://github.com/noir-lang/aztec_backend", rev = "c673a14be33e445d98b35969716ac59c682036d6" }
aztec_wasm_backend = { optional = true, package = "barretenberg_wasm", git = "https://github.com/noir-lang/aztec_backend", rev = "c673a14be33e445d98b35969716ac59c682036d6" }
aztec_backend = { optional = true, package = "barretenberg_static_lib", git = "https://github.com/noir-lang/aztec_backend", rev = "b7349851d7afcffef4171e4d3a30b163ea37e579" }
aztec_wasm_backend = { optional = true, package = "barretenberg_wasm", git = "https://github.com/noir-lang/aztec_backend", rev = "b7349851d7afcffef4171e4d3a30b163ea37e579" }
marlin_arkworks_backend = { optional = true, git = "https://github.com/noir-lang/marlin_arkworks_backend", rev = "144378edad821bfaa52bf2cacca8ecc87514a4fc" }

[features]
Expand Down
6 changes: 5 additions & 1 deletion crates/nargo/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@ pub fn check_from_path<P: AsRef<Path>>(p: P, allow_warnings: bool) -> Result<(),

let mut driver = Resolver::resolve_root_config(p.as_ref(), backend.np_language())?;
add_std_lib(&mut driver);
driver.check(allow_warnings);

if driver.check_crate(allow_warnings).is_err() {
std::process::exit(1);
}

// XXX: We can have a --overwrite flag to determine if you want to overwrite the Prover/Verifier.toml files
if let Some(abi) = driver.compute_abi() {
// XXX: The root config should return an enum to determine if we are looking for .json or .toml
Expand Down
15 changes: 6 additions & 9 deletions crates/nargo/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,14 @@ pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> {
let mut circuit_path = PathBuf::new();
circuit_path.push(TARGET_DIR);

let result = generate_circuit_and_witness_to_disk(
generate_circuit_and_witness_to_disk(
circuit_name,
current_dir,
circuit_path,
witness,
allow_warnings,
);
match result {
Ok(_) => Ok(()),
Err(e) => Err(e),
}
)
.map(|_| ())
}

#[allow(deprecated)]
Expand Down Expand Up @@ -78,8 +75,8 @@ pub fn compile_circuit<P: AsRef<Path>>(
let backend = crate::backends::ConcreteBackend;
let mut driver = Resolver::resolve_root_config(program_dir.as_ref(), backend.np_language())?;
add_std_lib(&mut driver);
let compiled_program =
driver.into_compiled_program(backend.np_language(), show_ssa, allow_warnings);

Ok(compiled_program)
driver
.into_compiled_program(backend.np_language(), show_ssa, allow_warnings)
.map_err(|_| std::process::exit(1))
}
11 changes: 11 additions & 0 deletions crates/nargo/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ mod contract_cmd;
mod gates_cmd;
mod new_cmd;
mod prove_cmd;
mod test_cmd;
mod verify_cmd;

const SHORT_GIT_HASH: &str = git_version!(prefix = "git:");
Expand Down Expand Up @@ -74,6 +75,15 @@ pub fn start_cli() {
.arg(show_ssa.clone())
.arg(allow_warnings.clone()),
)
.subcommand(
App::new("test")
.about("Run the tests for this program")
.arg(
Arg::with_name("test_name")
.help("If given, only tests with names containing this string will be run"),
)
.arg(allow_warnings.clone()),
)
.subcommand(
App::new("compile")
.about("Compile the program and its secret execution trace into ACIR format")
Expand Down Expand Up @@ -104,6 +114,7 @@ pub fn start_cli() {
Some("compile") => compile_cmd::run(matches),
Some("verify") => verify_cmd::run(matches),
Some("gates") => gates_cmd::run(matches),
Some("test") => test_cmd::run(matches),
Some(x) => Err(CliError::Generic(format!("unknown command : {x}"))),
_ => unreachable!(),
};
Expand Down
94 changes: 94 additions & 0 deletions crates/nargo/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
use std::{collections::BTreeMap, io::Write};

use acvm::{PartialWitnessGenerator, ProofSystemCompiler};
use clap::ArgMatches;
use noirc_driver::Driver;
use noirc_frontend::node_interner::FuncId;
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};

use crate::{errors::CliError, resolver::Resolver};

use super::add_std_lib;

pub(crate) fn run(args: ArgMatches) -> Result<(), CliError> {
let args = args.subcommand_matches("test").unwrap();
let test_name = args.value_of("test_name").unwrap_or("");
let allow_warnings = args.is_present("allow-warnings");
run_tests(test_name, allow_warnings)
}

fn run_tests(test_name: &str, allow_warnings: bool) -> Result<(), CliError> {
let backend = crate::backends::ConcreteBackend;

let package_dir = std::env::current_dir().unwrap();
let mut driver = Resolver::resolve_root_config(&package_dir, backend.np_language())?;
add_std_lib(&mut driver);

if driver.check_crate(allow_warnings).is_err() {
std::process::exit(1);
}

let test_functions = driver.get_all_test_functions_in_crate_matching(test_name);
println!("Running {} test functions...", test_functions.len());
let mut failing = 0;

let writer = StandardStream::stderr(ColorChoice::Always);
let mut writer = writer.lock();

for test_function in test_functions {
let test_name = driver.function_name(test_function);
write!(writer, "Testing {test_name}... ").expect("Failed to write to stdout");
writer.flush().ok();

match run_test(test_name, test_function, &driver, allow_warnings) {
Ok(_) => {
writer.set_color(ColorSpec::new().set_fg(Some(Color::Green))).ok();
writeln!(writer, "ok").ok();
}
// Assume an error was already printed to stdout
Err(_) => failing += 1,
}
writer.reset().ok();
}

if failing == 0 {
writer.set_color(ColorSpec::new().set_fg(Some(Color::Green))).unwrap();
writeln!(writer, "All tests passed").ok();
} else {
let plural = if failing == 1 { "" } else { "s" };
writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).unwrap();
writeln!(writer, "{failing} test{plural} failed").ok();
std::process::exit(1);
}

writer.reset().ok();
Ok(())
}

fn run_test(
test_name: &str,
main: FuncId,
driver: &Driver,
allow_warnings: bool,
) -> Result<(), CliError> {
let backend = crate::backends::ConcreteBackend;
let language = backend.np_language();

let program = driver
.compile_no_check(language, false, allow_warnings, Some(main))
.map_err(|_| CliError::Generic(format!("Test '{test_name}' failed to compile")))?;

let mut solved_witness = BTreeMap::new();

// Run the backend to ensure the PWG evaluates functions like std::hash::pedersen,
// otherwise constraints involving these expressions will not error.
if let Err(error) = backend.solve(&mut solved_witness, program.circuit.opcodes) {
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();
return Err(error.into());
}
Ok(())
}
86 changes: 52 additions & 34 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ use acvm::acir::circuit::Circuit;
use acvm::Language;
use fm::FileType;
use noirc_abi::Abi;
use noirc_errors::{DiagnosableError, Reporter};
use noirc_errors::{DiagnosableError, ReportedError, Reporter};
use noirc_evaluator::create_circuit;
use noirc_frontend::graph::{CrateId, CrateName, CrateType, LOCAL_CRATE};
use noirc_frontend::hir::def_map::CrateDefMap;
use noirc_frontend::hir::Context;
use noirc_frontend::monomorphization::monomorphize;
use noirc_frontend::node_interner::FuncId;
use serde::{Deserialize, Serialize};
use std::path::{Path, PathBuf};

Expand All @@ -34,7 +35,10 @@ impl Driver {
pub fn compile_file(root_file: PathBuf, np_language: acvm::Language) -> CompiledProgram {
let mut driver = Driver::new(&np_language);
driver.create_local_crate(root_file, CrateType::Binary);
driver.into_compiled_program(np_language, false, false)

driver
.into_compiled_program(np_language, false, false)
.unwrap_or_else(|_| std::process::exit(1))
}

/// Compiles a file and returns true if compilation was successful
Expand Down Expand Up @@ -126,13 +130,9 @@ impl Driver {
}
}

// NOTE: Maybe build could be skipped given that now it is a pass through method.
/// Statically analyzes the local crate
pub fn check(&mut self, allow_warnings: bool) {
self.analyze_crate(allow_warnings)
}

fn analyze_crate(&mut self, allow_warnings: bool) {
/// Run the lexing, parsing, name resolution, and type checking passes,
/// returning Err(FrontendError) and printing any errors that were found.
pub fn check_crate(&mut self, allow_warnings: bool) -> Result<(), ReportedError> {
let mut errs = vec![];
CrateDefMap::collect_defs(LOCAL_CRATE, &mut self.context, &mut errs);
let mut error_count = 0;
Expand All @@ -145,7 +145,7 @@ impl Driver {
);
}

Reporter::finish(error_count);
Reporter::finish(error_count)
}

pub fn compute_abi(&self) -> Option<Abi> {
Expand All @@ -159,15 +159,26 @@ impl Driver {
Some(abi)
}

#[allow(deprecated)]
pub fn into_compiled_program(
mut self,
np_language: acvm::Language,
show_ssa: bool,
allow_warnings: bool,
) -> CompiledProgram {
self.check(allow_warnings);
) -> Result<CompiledProgram, ReportedError> {
self.check_crate(allow_warnings)?;
self.compile_no_check(np_language, show_ssa, allow_warnings, None)
}

/// Compile the current crate. Assumes self.check_crate is called beforehand!
#[allow(deprecated)]
pub fn compile_no_check(
&self,
np_language: acvm::Language,
show_ssa: bool,
allow_warnings: bool,
// Optional override to provide a different `main` function to start execution
main_function: Option<FuncId>,
) -> Result<CompiledProgram, ReportedError> {
// Check the crate type
// We don't panic here to allow users to `evaluate` libraries
// which will do nothing
Expand All @@ -180,39 +191,46 @@ impl Driver {
let local_crate = self.context.def_map(LOCAL_CRATE).unwrap();

// All Binaries should have a main function
let main_function =
local_crate.main_function().expect("cannot compile a program with no main function");
let main_function = main_function.unwrap_or_else(|| {
local_crate.main_function().expect("cannot compile a program with no main function")
});

// Create ABI for main function
let func_meta = self.context.def_interner.function_meta(&main_function);
let abi = func_meta.into_abi(&self.context.def_interner);

let program = monomorphize(main_function, self.context.def_interner);
let program = monomorphize(main_function, &self.context.def_interner);

// Compile Program
let circuit = match create_circuit(
program,
np_language.clone(),
acvm::default_is_blackbox_supported(np_language),
show_ssa,
) {
Ok(circuit) => circuit,
let blackbox_supported = acvm::default_is_blackbox_supported(np_language.clone());
match create_circuit(program, np_language, blackbox_supported, show_ssa) {
Ok(circuit) => Ok(CompiledProgram { circuit, abi: Some(abi) }),
Err(err) => {
// The FileId here will be the file id of the file with the main file
// Errors will be shown at the call site without a stacktrace
let file_id = err.location.map(|loc| loc.file);
let error_count = Reporter::with_diagnostics(
file_id,
&self.context.file_manager,
&[err.to_diagnostic()],
allow_warnings,
);
Reporter::finish(error_count);
unreachable!("reporter will exit before this point")
let error = &[err.to_diagnostic()];
let files = &self.context.file_manager;

let error_count = Reporter::with_diagnostics(file_id, files, error, allow_warnings);
Reporter::finish(error_count)?;
Err(ReportedError)
}
};
}
}

/// Returns a list of all functions in the current crate marked with #[test]
/// whose names contain the given pattern string. An empty pattern string
/// will return all functions marked with #[test].
pub fn get_all_test_functions_in_crate_matching(&self, pattern: &str) -> Vec<FuncId> {
let interner = &self.context.def_interner;
interner
.get_all_test_functions()
.filter_map(|id| interner.function_name(&id).contains(pattern).then_some(id))
.collect()
}

CompiledProgram { circuit, abi: Some(abi) }
pub fn function_name(&self, id: FuncId) -> &str {
self.context.def_interner.function_name(&id)
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_driver/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ fn main() {
driver.add_dep(LOCAL_CRATE, crate_id1, "coo4");
driver.add_dep(LOCAL_CRATE, crate_id2, "coo3");

driver.into_compiled_program(acvm::Language::R1CS, false, false);
driver.into_compiled_program(acvm::Language::R1CS, false, false).ok();
}
4 changes: 4 additions & 0 deletions crates/noirc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ pub trait DiagnosableError {
fn to_diagnostic(&self) -> CustomDiagnostic;
}

/// Returned when the Reporter finishes after reporting errors
#[derive(Copy, Clone)]
pub struct ReportedError;

#[derive(Debug, PartialEq, Eq)]
pub struct CollectedErrors {
pub file_id: fm::FileId,
Expand Down
10 changes: 6 additions & 4 deletions crates/noirc_errors/src/reporter.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::Span;
use crate::{ReportedError, Span};
use codespan_reporting::diagnostic::{Diagnostic, Label};
use codespan_reporting::term;
use codespan_reporting::term::termcolor::{
Expand Down Expand Up @@ -147,16 +147,18 @@ impl Reporter {
error_count
}

pub fn finish(error_count: usize) {
pub fn finish(error_count: usize) -> Result<(), ReportedError> {
if error_count != 0 {
let writer = StandardStream::stderr(ColorChoice::Always);
let mut writer = writer.lock();

writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).unwrap();

writeln!(&mut writer, "error: aborting due to {error_count} previous errors").unwrap();
writer.reset().ok(); // Ignore any IO errors, we're exiting at this point anyway

std::process::exit(1);
Err(ReportedError)
} else {
Ok(())
}
}
}
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,8 +741,8 @@ impl SsaContext {
if enable_logging {
Acir::print_circuit(&evaluator.opcodes);
println!("DONE");
println!("ACIR opcodes generated : {}", evaluator.opcodes.len());
}
println!("ACIR opcodes generated : {}", evaluator.opcodes.len());
Ok(())
}

Expand Down
1 change: 1 addition & 0 deletions crates/noirc_frontend/src/ast/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ impl From<FunctionDefinition> for NoirFunction {
Some(Attribute::Builtin(_)) => FunctionKind::Builtin,
Some(Attribute::Foreign(_)) => FunctionKind::LowLevel,
Some(Attribute::Alternative(_)) => FunctionKind::Normal,
Some(Attribute::Test) => FunctionKind::Normal,
None => FunctionKind::Normal,
};

Expand Down
Loading

0 comments on commit 2e1dc82

Please sign in to comment.