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

chore(ci): Distinguish between expected failures and compiler panics in compile_failure tests. #2518

Conversation

nickysn
Copy link
Contributor

@nickysn nickysn commented Aug 31, 2023

Problem*

Tests in the compiler_error subdirectory are treated as successful even if they cause the compiler to crash. This patch fixes that.

PR Checklist*

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

@yordanmadzhunkov
Copy link
Contributor

@jfecher Compile panics and should never happen. Currently we observed one panic during a test that fails with compile error.

@TomAFrench
Copy link
Member

Looks good. Will approve and merge this afternoon

Copy link
Contributor

@jfecher jfecher 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 TomAFrench added this pull request to the merge queue Sep 1, 2023
@TomAFrench TomAFrench removed this pull request from the merge queue due to a manual request Sep 1, 2023
@TomAFrench TomAFrench changed the title fix(tests): don't treat compile_failure tests as successful if they cause a compiler error chore(ci): Distinguish between expected failures and compiler panics in compile_failure tests. Sep 1, 2023
@TomAFrench TomAFrench added this pull request to the merge queue Sep 1, 2023
Merged via the queue into noir-lang:master with commit e9f952d Sep 1, 2023
14 checks passed
TomAFrench added a commit that referenced this pull request Sep 1, 2023
* master: (47 commits)
  fix: Initialize structs during def collection, not name resolution (#2528)
  feat: Apply optimizations to unconstrained code (#2348)
  chore(ci): Distinguish between expected failures and compiler panics in `compile_failure` tests. (#2518)
  chore: improve types in `acvm-backend-barretenberg` (#2516)
  feat(aztec_noir): abstract kernel return types (#2521)
  chore: remove usage of `Backend` trait (#2514)
  chore: delete `ProveAndVerifyCommand` (#2520)
  chore: Remove dead code from `acvm_backend_barretenberg` (#2512)
  chore: only install `tokio-util` dependency on windows (#2425)
  chore(aztec_noir):  imply the open keyword (#2508)
  chore: pull `acvm-backend-barretenberg` into main Noir repo (#2495)
  chore: clippy fix (#2507)
  chore: check if the noir aztec library is installed (#2505)
  chore: update ACIR artifacts (#2503)
  chore!: Update to `acvm-backend-barretenberg` v0.12.0 (#2377)
  fix: Bring back accidentally deleted double_verify_proof test. (#2501)
  chore(aztec_noir): import aztec library if not found yet (#2492)
  chore(abi)!: Replace struct name with fully qualified struct path (#2374)
  chore!: Remove keys from preprocessed artifacts (#2283)
  chore(noir): Release 0.10.5 (#2482)
  ...
@nickysn nickysn deleted the testrunner_fix_for_compiler_panic branch September 1, 2023 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants