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: Add noir_wasm testing workflow #1921

Merged
merged 34 commits into from
Aug 7, 2023
Merged

chore: Add noir_wasm testing workflow #1921

merged 34 commits into from
Aug 7, 2023

Conversation

jonybur
Copy link
Contributor

@jonybur jonybur commented Jul 13, 2023 •
edited
Loading

Description

Adds a workflow to test noir_wasm

Problem*

noir_wasm breaks and this should flag whenever it does (before we merge something that breaks it)

Resolves

Summary*

Simple workflow that adds an integration with https://github.com/noir-lang/noir-wasm-testing
It builds noir_wasm, takes the noir-wasm-testing repo and injects it - then it runs the test.

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.

@Savio-Sou
Copy link
Collaborator

Is this related to / supersedes #1660?

@jonybur
Copy link
Contributor Author

jonybur commented Jul 14, 2023

@Savio-Sou yes, it's updated to work with the wasm build workflow

@jonybur jonybur changed the title (feat): add noir_wasm testing workflow feat: add noir_wasm testing workflow Jul 24, 2023
@jonybur jonybur changed the title feat: add noir_wasm testing workflow feat: Add noir_wasm testing workflow Jul 24, 2023
@jonybur jonybur marked this pull request as ready for review July 24, 2023 13:57
@TomAFrench TomAFrench self-requested a review July 27, 2023 17:35
@TomAFrench TomAFrench mentioned this pull request Aug 4, 2023
Merged
5 tasks
@TomAFrench @jonybur
chore: Ensure that CLI and wasm compilers are sync in `noir_wasm_test…
4e0a050 
…ing` (#2079)

---------

Co-authored-by: jonybur <jobur93@gmail.com>
@TomAFrench TomAFrench changed the title feat: Add noir_wasm testing workflow chore: Add noir_wasm testing workflow Aug 4, 2023
@TomAFrench
Copy link
Member

Changing this to chore as it doesn't need to go in the changelog.

TomAFrench and others added 11 commits August 4, 2023 15:26
@TomAFrench
Update .github/workflows/wasm.yml
6c320e1
@TomAFrench
Merge branch 'master' into jb/noir-testing-workflow
0d2d438 
* master: (50 commits)
  chore: update stale comment on `create_circuit` (#2173)
  chore: Replace `resolve_path` function with a trait that impls normalize (#2157)
  chore: clippy fix (#2174)
  feat!: Allow specifying new package name with `--name` flag (#2144)
  chore!: remove unused flags on LSP command (#2170)
  chore: Hide the `show_ssa` and `show_brillig` flags (#2171)
  chore: bump `clap` to 4.3.19 (#2167)
  chore: Move the long line of `nargo info` to `long_about` (#2151)
  chore: Refactor `normalize_path` into an API on FileManager (#2156)
  fix: Implement slices of structs (#2150)
  chore: Refreshed ACIR artifacts (#2148)
  chore: Rebuild ACIR test artifacts (#2147)
  chore: remove short flags for `--show-ssa` and `--deny-warnings` (#2141)
  chore: replace usage of `Directive::Quotient` with brillig opcode  (#1766)
  chore: clippy fix (#2136)
  feat: Initial work on rewriting closures to regular functions with hi… (#1959)
  chore: Decouple acir blockid from ssa valueid (#2103)
  chore: Initialize copy array from previous values in `array_set` (#2106)
  chore: rename `ssa_refactor` module to `ssa` (#2129)
  chore: Use `--show-output` flag on execution rather than compilation  (#2116)
  ...
@TomAFrench
chore: run browser tests rather than nodejs tests
e33058f
@TomAFrench
chore: fix test command in CI
99ba717
@TomAFrench @jonybur
chore: bring noir-wasm-testing repo contents into wasm crate
778b095 
---------

Co-authored-by: jonybur <jobur93@gmail.com>
@TomAFrench
chore: update compile command to handle not being at repo root
8a545c6
@TomAFrench
chore: remove unnecessary checkout
c31e681
@TomAFrench
chore: update location to look for nargo binary
f283748
@TomAFrench
chore: add missing $
bea37a1
@TomAFrench
chore: remove self-referential dependency
978551d
@TomAFrench
chore: switch to noir-lang/noir-source-resolver version which exists
9552903
TomAFrench
TomAFrench previously approved these changes Aug 7, 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.

LGTM

@TomAFrench
Merge branch 'master' into jb/noir-testing-workflow
1b7eca9 
* master:
  chore: Remove symlink and dummy config file (#2200)
  fix: Fix an ICE when reassigning a mutable lambda variable to one with a different environment type (#2172)
  feat: Only create new witnesses for distinctiveness when duplicates exist (#2191)
  chore: Use helper functions for getting values of `AcirVar`s (#2194)
  feat: Add support for slices of structs and nested slices in brillig (#2084)
  feat: Perform sorting of constant arrays at compile time (#2195)
  chore: Improve unary error (#2199)
  chore: separate integration test cases into directories based on expected result (#2198)
  chore: remove stale comment (#2197)
  feat(nargo): Support custom entry points specified in TOML (#2158)
  fix(nargo): Indicate which TOML file is missing package name (#2177)
  fix: remove duplicated `name` option in `nargo new` (#2183)
  chore: add documentation to the `nargo lsp` command (#2169)
  feat(nargo)!: Require package `type` be specified in Nargo.toml (#2134)
  fix(nargo): Make dependencies section optional in TOML (#2161)
  chore: Do not create new memory block when not needed (#2142)
  fix: fix an ICE happening when we call a closure result from if/else (#2146)
  chore: remove unnecessary cloning of package dependencies (#2175)
@TomAFrench
Copy link
Member

@kevaundray @kobyhallx Could you force merge this once CI passes? I'm seeing a phantom CI workflow on this which is going to block this. It should be fine once it's in master however.

@kevaundray
Copy link
Contributor

@kevaundray @kobyhallx Could you force merge this once CI passes? I'm seeing a phantom CI workflow on this which is going to block this. It should be fine once it's in master however.

Removed the required phantom CI workflow

@kevaundray kevaundray added this pull request to the merge queue Aug 7, 2023
Merged via the queue into master with commit 08683e1 Aug 7, 2023
7 checks passed
@kevaundray kevaundray deleted the jb/noir-testing-workflow branch August 7, 2023 18:08
TomAFrench added a commit that referenced this pull request Aug 7, 2023
@TomAFrench
Merge branch 'master' into constant-brillig-execution
e6ee884 
* master:
  feat: Optimize equality checks between a boolean and constant (#2201)
  feat: Issue warning for signed integers (#2185)
  chore: Add `noir_wasm` testing workflow (#1921)
TomAFrench added a commit that referenced this pull request Aug 7, 2023
@TomAFrench
Merge branch 'master' into assert-eq-builtin
8bd4cbd 
* master: (35 commits)
  feat: Issue warning for signed integers (#2185)
  chore: Add `noir_wasm` testing workflow (#1921)
  chore: Remove symlink and dummy config file (#2200)
  fix: Fix an ICE when reassigning a mutable lambda variable to one with a different environment type (#2172)
  feat: Only create new witnesses for distinctiveness when duplicates exist (#2191)
  chore: Use helper functions for getting values of `AcirVar`s (#2194)
  feat: Add support for slices of structs and nested slices in brillig (#2084)
  feat: Perform sorting of constant arrays at compile time (#2195)
  chore: Improve unary error (#2199)
  chore: separate integration test cases into directories based on expected result (#2198)
  chore: remove stale comment (#2197)
  feat(nargo): Support custom entry points specified in TOML (#2158)
  fix(nargo): Indicate which TOML file is missing package name (#2177)
  fix: remove duplicated `name` option in `nargo new` (#2183)
  chore: add documentation to the `nargo lsp` command (#2169)
  feat(nargo)!: Require package `type` be specified in Nargo.toml (#2134)
  fix(nargo): Make dependencies section optional in TOML (#2161)
  chore: Do not create new memory block when not needed (#2142)
  fix: fix an ICE happening when we call a closure result from if/else (#2146)
  chore: remove unnecessary cloning of package dependencies (#2175)
  ...
@Savio-Sou Savio-Sou mentioned this pull request Aug 16, 2023
Closed
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

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

Successfully merging this pull request may close these issues.

Test noir_wasm on PR
4 participants
@jonybur @Savio-Sou @TomAFrench @kevaundray