-
Notifications
You must be signed in to change notification settings - Fork 211
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
base: master
Are you sure you want to change the base?
Conversation
🚀 Deployed on https://67533ee363257629879fd9f8--noir-docs.netlify.app |
Peak Memory Sample
|
if name == ForeignCall::CreateMock.name() { | ||
OracleResult::Mocked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why aren't all the other mock functions considered to be Mocked
status?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to identify mocked oracles by looking at the create_mock() function, but it is not easy. Instead I used an heuristic that consider that all oracles are mocked if there is one create_mock.
@@ -49,6 +49,10 @@ pub(crate) struct TestCommand { | |||
/// JSON RPC url to solve oracle calls | |||
#[clap(long)] | |||
oracle_resolver: Option<String>, | |||
|
|||
/// Flag to skip tests using oracles. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// Flag to skip tests using oracles. | |
/// Flag to skip tests using unhandled oracles without mocks. |
The way it's phrased sounds like any test with an oracle will be skipped.
tooling/nargo/src/ops/test.rs
Outdated
for brillig_function in &compiled_program.program.unconstrained_functions { | ||
match brillig_function.get_oracle_status(ForeignCall::check_oracle_status) { | ||
OracleResult::Mocked => { | ||
has_oracle = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has_oracle = false; | |
// Assume that mocks will handle any call. | |
has_oracle = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, just a couple of suggestions for additional docs/comments. I'll defer to @TomAFrench whether this should be the default on CI or the env var he did that detects a failure after attempting to run the test.
FYI @noir-lang/developerrelations on Noir doc changes. |
Description
Problem*
Resolves #6643
Summary*
Add a 'skip-oracle' flag for the nargo test command.
Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmt
on default settings.