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.

Sign up for GitHub

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

Jump to bottom

chore: Move independent run_test function into nargo core #2468

Merged
merged 4 commits into from
Aug 29, 2023
Merged

chore: Move independent run_test function into nargo core #2468

merged 4 commits into from
Aug 29, 2023

Conversation

phated
Copy link
Contributor

@phated phated commented Aug 28, 2023

Description

Problem*

Resolves

Summary*

This moves the core run_test functionality into nargo core and ensures the stdout/stderr printing only happens within nargo_cli.

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.

TomAFrench
TomAFrench reviewed Aug 29, 2023
View reviewed changes
Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, some minor nits

crates/nargo/src/ops/test.rs Outdated Show resolved Hide resolved
crates/nargo/src/ops/test.rs Outdated Show resolved Hide resolved
crates/nargo/src/ops/test.rs Outdated Show resolved Hide resolved
crates/nargo_cli/src/cli/test_cmd.rs Outdated Show resolved Hide resolved
phated and others added 2 commits August 29, 2023 08:01
@phated @TomAFrench
Apply suggestions from code review
056d4ed 
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
@phated
fix imports and trait bounds
0d6cbd1
TomAFrench
TomAFrench reviewed Aug 29, 2023
View reviewed changes
Copy link
Member

@TomAFrench TomAFrench left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit

crates/nargo/src/ops/mod.rs Outdated Show resolved Hide resolved
@phated phated added this pull request to the merge queue Aug 29, 2023
Merged via the queue into master with commit 3e2c868 Aug 29, 2023
16 checks passed
@phated phated deleted the phated/nargo-run-test branch August 29, 2023 16:32
TomAFrench added a commit that referenced this pull request Aug 30, 2023
@TomAFrench
Merge branch 'master' into kw/bb-binaries
47f2ab6 
* master: (42 commits)
  fix(ssa): Handle right shift with constants (#2481)
  chore(noir): Release 0.10.4 (#2354)
  fix: Divide by zero should fail to satisfy constraints for `Field` and ints (#2475)
  fix(ssa): Remove padding from ToRadix call with constant inputs (#2479)
  fix: Implement handling of array aliasing in the mem2reg optimization pass (#2463)
  chore: resolve `Instruction` inputs fully before checking against cache (#2472)
  chore: Move independent `run_test` function into nargo core (#2468)
  feat: Standard library functions can now be called with closure args  (#2471)
  feat(frontend): aztec syntactic sugar (feature flagged) (#2403)
  chore(ci): enforce compliance with `cargo fmt` (#2467)
  chore(ci): Allow releases to have additional feature flags (#2405)
  feat: Add `assert_eq` keyword (#2137)
  fix(ssa): Do not optimize for allocates in constant folding (#2466)
  feat(ssa): Reuse existing results for duplicated instructions with no side-effects (#2460)
  fix: Closure lvalue capture bugfix (#2457)
  feat: Syntax for environment types now works with generics (#2383)
  fix(parser): fixes for the parsing of 'where' clauses (#2430)
  fix: Run `wasm` nodejs tests with no fails (#2387)
  chore: Run `cargo fmt` (#2455)
  chore: Perform formatting changes to integration tests (#2448)
  ...
TomAFrench added a commit that referenced this pull request Aug 30, 2023
@TomAFrench
Merge branch 'master' into remove-keys-from-artifact
b04ed6b 
* master: (42 commits)
  fix(ssa): Handle right shift with constants (#2481)
  chore(noir): Release 0.10.4 (#2354)
  fix: Divide by zero should fail to satisfy constraints for `Field` and ints (#2475)
  fix(ssa): Remove padding from ToRadix call with constant inputs (#2479)
  fix: Implement handling of array aliasing in the mem2reg optimization pass (#2463)
  chore: resolve `Instruction` inputs fully before checking against cache (#2472)
  chore: Move independent `run_test` function into nargo core (#2468)
  feat: Standard library functions can now be called with closure args  (#2471)
  feat(frontend): aztec syntactic sugar (feature flagged) (#2403)
  chore(ci): enforce compliance with `cargo fmt` (#2467)
  chore(ci): Allow releases to have additional feature flags (#2405)
  feat: Add `assert_eq` keyword (#2137)
  fix(ssa): Do not optimize for allocates in constant folding (#2466)
  feat(ssa): Reuse existing results for duplicated instructions with no side-effects (#2460)
  fix: Closure lvalue capture bugfix (#2457)
  feat: Syntax for environment types now works with generics (#2383)
  fix(parser): fixes for the parsing of 'where' clauses (#2430)
  fix: Run `wasm` nodejs tests with no fails (#2387)
  chore: Run `cargo fmt` (#2455)
  chore: Perform formatting changes to integration tests (#2448)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomAFrench TomAFrench TomAFrench approved these changes

@kobyhallx kobyhallx Awaiting requested review from kobyhallx

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@phated @TomAFrench