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: skip tests having oracle calls #6666

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
16 changes: 16 additions & 0 deletions acvm-repo/acir/src/circuit/brillig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,22 @@ pub struct BrilligBytecode<F> {
pub bytecode: Vec<BrilligOpcode<F>>,
}

impl<F> BrilligBytecode<F> {
/// Returns true if the bytecode contains a foreign call
/// whose name matches the given predicate.
pub fn has_oracle<Fun>(&self, filter: Fun) -> bool
where
Fun: Fn(&str) -> bool,
{
self.bytecode.iter().any(|op| {
if let BrilligOpcode::ForeignCall { function, .. } = op {
filter(function)
} else {
false
}
})
}
}
/// Id for the function being called.
#[derive(
Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Hash, Copy, Default, PartialOrd, Ord,
Expand Down
4 changes: 4 additions & 0 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ pub struct CompileOptions {
/// A lower value keeps the original program if it was smaller, even if it has more jumps.
#[arg(long, hide = true, allow_hyphen_values = true)]
pub max_bytecode_increase_percent: Option<i32>,

/// Flag to skip tests using oracles.
#[arg(long, hide = true)]
pub skip_oracle: bool,
guipublic marked this conversation as resolved.
Show resolved Hide resolved
}

pub fn parse_expression_width(input: &str) -> Result<ExpressionWidth, std::io::Error> {
Expand Down
2 changes: 2 additions & 0 deletions docs/docs/how_to/how-to-oracles.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,8 @@ nargo test --oracle-resolver http://localhost:5555

This tells `nargo` to use your RPC Server URL whenever it finds an oracle decorator.

You can also skip the tests using oracles by using the flag `--skip-oracle` in the `nargo test` command.

## Step 4 - Usage with NoirJS

In a JS environment, an RPC server is not strictly necessary, as you may want to resolve your oracles without needing any JSON call at all. NoirJS simply expects that you pass a callback function when you generate proofs, and that callback function can be anything.
Expand Down
5 changes: 5 additions & 0 deletions tooling/lsp/src/requests/test_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ fn on_test_run_request_inner(
result: "error".to_string(),
message: Some(diag.diagnostic.message),
},
TestStatus::Skipped => NargoTestRunResult {
id: params.id.clone(),
result: "skipped".to_string(),
message: None,
},
};
Ok(result)
}
Expand Down
4 changes: 4 additions & 0 deletions tooling/nargo/src/ops/foreign_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ impl ForeignCall {
_ => None,
}
}

pub(crate) fn invalid_name(name: &str) -> bool {
ForeignCall::lookup(name).is_none()
aakoshh marked this conversation as resolved.
Show resolved Hide resolved
}
}

/// This struct represents an oracle mock. It can be used for testing programs that use oracles.
Expand Down
22 changes: 20 additions & 2 deletions tooling/nargo/src/ops/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,24 @@ use noirc_frontend::hir::{def_map::TestFunction, Context};

use crate::{errors::try_to_diagnose_runtime_error, NargoError};

use super::{execute_program, DefaultForeignCallExecutor};
use super::{execute_program, DefaultForeignCallExecutor, ForeignCall};

pub enum TestStatus {
Pass,
Fail { message: String, error_diagnostic: Option<FileDiagnostic> },
Skipped,
CompileError(FileDiagnostic),
}

impl TestStatus {
pub fn failed(&self) -> bool {
!matches!(self, TestStatus::Pass)
matches!(self, TestStatus::Fail { .. } | TestStatus::CompileError(_))
}
pub fn pass(&self) -> bool {
guipublic marked this conversation as resolved.
Show resolved Hide resolved
matches!(self, TestStatus::Pass)
}
pub fn skipped(&self) -> bool {
matches!(self, TestStatus::Skipped)
}
}

Expand All @@ -45,6 +52,17 @@ pub fn run_test<B: BlackBoxFunctionSolver<FieldElement>>(

match compile_no_check(context, config, test_function.get_id(), None, false) {
Ok(compiled_program) => {
if config.skip_oracle {
let has_oracle = compiled_program
.program
.unconstrained_functions
.iter()
.any(|func| func.has_oracle(ForeignCall::invalid_name));
aakoshh marked this conversation as resolved.
Show resolved Hide resolved
if has_oracle {
return TestStatus::Skipped;
}
}

if test_function_has_no_arguments {
// Run the backend to ensure the PWG evaluates functions like std::hash::pedersen,
// otherwise constraints involving these expressions will not error.
Expand Down
36 changes: 28 additions & 8 deletions tooling/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,33 +263,53 @@ fn display_test_report(
compile_options.silence_warnings,
);
}
TestStatus::Skipped => {
writer
.set_color(ColorSpec::new().set_fg(Some(Color::Yellow)))
.expect("Failed to set color");
writeln!(writer, "skipped").expect("Failed to write to stderr");
}
}
writer.reset().expect("Failed to reset writer");
}

write!(writer, "[{}] ", package.name).expect("Failed to write to stderr");

let count_all = test_report.len();
let count_failed = test_report.iter().filter(|(_, status)| status.failed()).count();
let plural = if count_all == 1 { "" } else { "s" };
let count_passed = test_report.iter().filter(|(_, status)| status.pass()).count();
let count_skipped = test_report.iter().filter(|(_, status)| status.skipped()).count();
let plural_failed = if count_failed == 1 { "" } else { "s" };
let plural_passed = if count_passed == 1 { "" } else { "s" };
let plural_skipped = if count_skipped == 1 { "" } else { "s" };

if count_failed == 0 {
writer.set_color(ColorSpec::new().set_fg(Some(Color::Green))).expect("Failed to set color");
write!(writer, "{count_all} test{plural} passed").expect("Failed to write to stderr");
write!(writer, "{count_passed} test{plural_passed} passed")
.expect("Failed to write to stderr");
if count_skipped > 0 {
writer
.set_color(ColorSpec::new().set_fg(Some(Color::Yellow)))
.expect("Failed to set color");
write!(writer, ", {count_skipped} test{plural_skipped} skipped")
.expect("Failed to write to stderr");
}
writer.reset().expect("Failed to reset writer");
writeln!(writer).expect("Failed to write to stderr");
} else {
let count_passed = count_all - count_failed;
let plural_failed = if count_failed == 1 { "" } else { "s" };
let plural_passed = if count_passed == 1 { "" } else { "s" };

if count_passed != 0 {
writer
.set_color(ColorSpec::new().set_fg(Some(Color::Green)))
.expect("Failed to set color");
write!(writer, "{count_passed} test{plural_passed} passed, ",)
.expect("Failed to write to stderr");
}

if count_skipped != 0 {
writer
.set_color(ColorSpec::new().set_fg(Some(Color::Yellow)))
.expect("Failed to set color");
write!(writer, ", {count_skipped} test{plural_skipped} skipped")
.expect("Failed to write to stderr");
}
guipublic marked this conversation as resolved.
Show resolved Hide resolved
writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).expect("Failed to set color");
writeln!(writer, "{count_failed} test{plural_failed} failed")
.expect("Failed to write to stderr");
Expand Down
6 changes: 6 additions & 0 deletions tooling/nargo_cli/tests/stdlib-tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
/// Inlining happens if `inline_cost - retain_cost < aggressiveness` (see `inlining.rs`).
/// NB the CLI uses maximum aggressiveness.
///
/// Even with the same inlining aggressiveness, forcing Brillig can trigger different behaviour.

Check warning on line 44 in tooling/nargo_cli/tests/stdlib-tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (behaviour)
#[test_matrix(
[false, true],
[i64::MIN, 0, i64::MAX]
Expand Down Expand Up @@ -146,6 +146,12 @@
compile_options.silence_warnings,
);
}
TestStatus::Skipped { .. } => {
writer
.set_color(ColorSpec::new().set_fg(Some(Color::Yellow)))
.expect("Failed to set color");
writeln!(writer, "skipped").expect("Failed to write to stderr");
}
}
writer.reset().expect("Failed to reset writer");
}
Expand Down