chore: Use --show-output flag on execution rather than compilation

[vezenovm]

Description

Problem*

When we moved println from being an ACIR opcode to a Brillig foreign call the --show-output logic changed as we no longer need to thread the flag down to ACIR generation. We can now provide it as an execute options to execute_circuit.

This fixes the previous logic we had where we do not print during nargo test but only print if specified with the --show-output flag.

I also adding reporting for compile errors inside of run_test. We previously were not reporting compile errors in tests and would simply fail with: Error: 1 test failed which provides no information.

I have added this test into strings:

fn failed_constraint(hex_as_field: Field) {
    std::println(hex_as_field);
    assert(hex_as_field != 0x41);
}

#[test]
fn test_failed_constraint() {
    failed_constraint(0x41);
}

which now reports this:

unning 3 test functions...
Testing test_call_main...
error: 
   ┌─ /mnt/user-data/maxim/noir/crates/nargo_cli/tests/test_data/strings/src/main.nr:52:12
   │
52 │     assert(hex_as_field != 0x41);
   │            -------------------- Failed constraint

Testing test_prints_strings...
ok
Testing test_prints_array...
ok
Error: 1 test failed

Location:
    crates/nargo_cli/src/cli/mod.rs:74:5

We should make sure to advise users in the docs to use nargo execute if they want to debug failing constraints with println statements. This is due to every input in a test being a constant rather than a witness, so we issue an error during compilation while we only print during execution (which comes after compilation).

We could consider not error'ing out on a failed constraint during test compilation, thus leaving print statements to still be executed. This should be done in a separate PR though that threads a "nargo test" compile option into the evaluator.

Summary*

Documentation

• This PR requires documentation updates when merged.

  • I will submit a noir-lang/docs PR.
  • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[vezenovm]

Converting back to a draft until I fix why the same program using nargo execute has println output while a test does not

[vezenovm]

We should make sure to advise users in the docs to use nargo execute if they want to debug failing constraints with println statements. This is due to every input in a test being a constant rather than a witness, so we issue an error during compilation while we only print during execution (which comes after compilation).

This note in the main PR description is why I requested an update to the docs

[TomAFrench]

In order to debug failing constraints with println calls nargo execute should be used instead

I think it might be confusing to describe it as "nargo test doesn't print while nargo execute does". It's closer to "any constraints which fail at compile time prevent printing any debug info".

Perhaps we should allow debug builds of Noir code which doesn't enforce Constrain instructions at compile time?

[vezenovm]

Perhaps we should allow debug builds of Noir code which doesn't enforce Constrain instructions at compile time?

I think this might be worth it so that we don't have inconsistent functionality. I suggested this in the main PR description:

We could consider not error'ing out on a failed constraint during test compilation, thus leaving print statements to still be executed. This should be done in a separate PR though that threads a "nargo test" compile option into the evaluator.

I suggested it for a separate PR though as I thought it might warrant a discussion and I mainly wanted to fix the --show-output flag. We don't print anything on a failing test right now anyway so this PR isn't really changing functionality there.

[TomAFrench]

Ah apologies, I didn't see you mentioned this in the PR description. Agree on it being OOS for this PR but could you add an issue for this?

I'm a little concerned with users reading that they should use nargo execute and then running into the same issue as they did in nargo test though.

[vezenovm]

Made an issue here: #2128

I'm a little concerned with users reading that they should use nargo execute and then running into the same issue as they did in nargo test though.

Do you mean with how we currently have it that if a user has a failing compile-time constraint they will not see prints?

[TomAFrench]

Yes that's right

[vezenovm]

Yeah I see. I guess we should just note in the docs then that until we have a debug mode, println will not work for failed constraints caught at compile time. I altered the comment and linked the issue. Let me know what you think @TomAFrench.

[TomAFrench]

LGTM. Yeah, no issues with the PR as it stands, just want to avoid incorrect docs.

[Savio-Sou]

Would this be going into v0.10.0?

[TomAFrench]

Yes, all PRs being merged currently will be going into the 0.10.0 release.