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(nargo): Add nargo test command to run all unit tests #728

Merged
merged 15 commits into from
Feb 3, 2023
Merged

Conversation

jfecher
Copy link
Contributor

@jfecher jfecher commented Feb 1, 2023

Related issue(s)

Resolves #610

Description

Adds the nargo test command to collect all functions with the new #[test] attribute and prove each of them, notifying the user if any test fails.

Summary of changes

  • Adds nargo test command
  • Adds #[test] attribute
  • Run the frontend on the entire crate beforehand to minimize repeated work
  • Collect all #[test] functions afterward and run the backend on each.
  • nargo test accepts an optional pattern argument to only run tests with names containing the given pattern.

Dependency additions / changes

Checklist

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt with default settings.
  • I have linked this PR to the issue(s) that it resolves.
  • I have reviewed the changes on GitHub, line by line.
  • I have ensured all changes are covered in the description.

Additional context

Tasks yet to be done: (all completed):

  • Clean up test output more on success (requires Remove timing printlns acvm-backend-barretenberg#49)
  • Colored output to make it easier to read at a glance
  • Verify functions with #[test] attribute have no parameters
  • Use catch_unwind to stop any panics that may happen when running a test - we cannot do this actually since our Driver type makes use of internal mutability.

Example output running 2 tests (1 passing, 1 failing) (after aztec_backend update now):

Running 2 test functions...
Testing my_passing_test... ok
Testing my_failing_test... error: 
   ┌─ /.../example/src/main.nr:14:15
   │
14 │     constrain false;
   │               ----- Constraint is always false

error: aborting due to 1 previous errors
my_failing_test failed

1 test failed
exit 1

@jfecher
Copy link
Contributor Author

jfecher commented Feb 2, 2023

Cannot add catch_unwind since a Driver needs to be put in the lambda and it contains data which is not unwind safe (contains interior mutability).

@jfecher jfecher requested a review from vezenovm February 3, 2023 17:28
@jfecher
Copy link
Contributor Author

jfecher commented Feb 3, 2023

This PR is ready for review again

crates/nargo/src/cli/test_cmd.rs Outdated Show resolved Hide resolved
crates/nargo/src/cli/test_cmd.rs Outdated Show resolved Hide resolved
@jfecher jfecher added the nargo Noir's CLI development tool label Feb 3, 2023
@jfecher jfecher merged commit 2e1dc82 into master Feb 3, 2023
@jfecher jfecher deleted the jf/tests branch February 3, 2023 21:47
TomAFrench added a commit to TomAFrench/noir that referenced this pull request Feb 3, 2023
* master:
  feat(nargo): Add `nargo test` command to run all unit tests (noir-lang#728)
  chore(ci): Apply `doc needed` label automatically on PRs (noir-lang#733)
  chore(ci): Remove failing bors workflow (noir-lang#744)
  feat(ci): Add workflow to validate PR title (noir-lang#730)
  feat(docs): Introduce Conventional Commits & release process docs (noir-lang#717)
TomAFrench added a commit that referenced this pull request Feb 3, 2023
* master:
  feat(nargo): Add `nargo test` command to run all unit tests (#728)
  chore(ci): Apply `doc needed` label automatically on PRs (#733)
  chore(ci): Remove failing bors workflow (#744)
  feat(ci): Add workflow to validate PR title (#730)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nargo Noir's CLI development tool
Projects
None yet
3 participants