From 73ccd45590222fc82642a6a9aa657c2915fc2c58 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 17 Dec 2024 13:52:38 -0300 Subject: [PATCH] feat: configurable external check failures (#6810) Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- .../noir-projects/aztec-nr.failures.jsonl | 0 .../noir-contracts.failures.jsonl | 0 .../crates/blob.failures.jsonl | 0 .../crates/parity-lib.failures.jsonl | 0 .../crates/private-kernel-lib.failures.jsonl | 0 .../crates/reset-kernel-lib.failures.jsonl | 0 .../crates/rollup-lib.failures.jsonl | 0 .../crates/types.failures.jsonl | 0 .github/critical_libraries_status/README.md | 20 ++++++++++ .../noir-lang/ec/.actual.jsonl | 4 ++ .../noir-lang/ec/.actual.jsonl.jq | 1 + .../noir-lang/ec/.failures.jsonl | 0 .../noir-lang/ec/.failures.jsonl.jq | 0 .../noir-lang/eddsa/.failures.jsonl | 0 .../noir-lang/mimc/.failures.jsonl | 0 .../noir-lang/noir-bignum/.failures.jsonl | 0 .../noir-lang/noir-edwards/.failures.jsonl | 0 .../noir-lang/noir_base64/.failures.jsonl | 0 .../noir-lang/noir_bigcurve/.failures.jsonl | 0 .../noir_json_parser/.failures.jsonl | 0 .../noir-lang/noir_rsa/.failures.jsonl | 0 .../noir-lang/noir_sort/.failures.jsonl | 0 .../noir_string_search/.failures.jsonl | 0 .../noir-lang/schnorr/.failures.jsonl | 0 .../noir-lang/sparse_array/.failures.jsonl | 0 .github/scripts/check_test_results.sh | 38 +++++++++++++++++++ .github/workflows/test-js-packages.yml | 17 +++++++-- .../nargo_cli/src/cli/test_cmd/formatters.rs | 1 + 28 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 .github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/aztec-nr.failures.jsonl create mode 100644 .github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-contracts.failures.jsonl create mode 100644 .github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/blob.failures.jsonl create mode 100644 .github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/parity-lib.failures.jsonl create mode 100644 .github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/private-kernel-lib.failures.jsonl create mode 100644 .github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/reset-kernel-lib.failures.jsonl create mode 100644 .github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/rollup-lib.failures.jsonl create mode 100644 .github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/types.failures.jsonl create mode 100644 .github/critical_libraries_status/README.md create mode 100644 .github/critical_libraries_status/noir-lang/ec/.actual.jsonl create mode 100644 .github/critical_libraries_status/noir-lang/ec/.actual.jsonl.jq create mode 100644 .github/critical_libraries_status/noir-lang/ec/.failures.jsonl create mode 100644 .github/critical_libraries_status/noir-lang/ec/.failures.jsonl.jq create mode 100644 .github/critical_libraries_status/noir-lang/eddsa/.failures.jsonl create mode 100644 .github/critical_libraries_status/noir-lang/mimc/.failures.jsonl create mode 100644 .github/critical_libraries_status/noir-lang/noir-bignum/.failures.jsonl create mode 100644 .github/critical_libraries_status/noir-lang/noir-edwards/.failures.jsonl create mode 100644 .github/critical_libraries_status/noir-lang/noir_base64/.failures.jsonl create mode 100644 .github/critical_libraries_status/noir-lang/noir_bigcurve/.failures.jsonl create mode 100644 .github/critical_libraries_status/noir-lang/noir_json_parser/.failures.jsonl create mode 100644 .github/critical_libraries_status/noir-lang/noir_rsa/.failures.jsonl create mode 100644 .github/critical_libraries_status/noir-lang/noir_sort/.failures.jsonl create mode 100644 .github/critical_libraries_status/noir-lang/noir_string_search/.failures.jsonl create mode 100644 .github/critical_libraries_status/noir-lang/schnorr/.failures.jsonl create mode 100644 .github/critical_libraries_status/noir-lang/sparse_array/.failures.jsonl create mode 100755 .github/scripts/check_test_results.sh diff --git a/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/aztec-nr.failures.jsonl b/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/aztec-nr.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-contracts.failures.jsonl b/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-contracts.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/blob.failures.jsonl b/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/blob.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/parity-lib.failures.jsonl b/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/parity-lib.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/private-kernel-lib.failures.jsonl b/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/private-kernel-lib.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/reset-kernel-lib.failures.jsonl b/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/reset-kernel-lib.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/rollup-lib.failures.jsonl b/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/rollup-lib.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/types.failures.jsonl b/.github/critical_libraries_status/AztecProtocol/aztec-packages/noir-projects/noir-protocol-circuits/crates/types.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/README.md b/.github/critical_libraries_status/README.md new file mode 100644 index 00000000000..47e9d7ad6ed --- /dev/null +++ b/.github/critical_libraries_status/README.md @@ -0,0 +1,20 @@ +# Critical Libraries Status + +This directory contains one `.failures.jsonl` file per external directory that is checked by CI. +CI will run the external repository tests and compare the test failures against those recorded +in these files. If there's a difference, CI will fail. + +This allows us to mark some tests as expected to fail if we introduce breaking changes. +When tests are fixed on the external repository, CI will let us know that we need to remove +the `.failures.jsonl` failures on our side. + +The format of the `.failures.jsonl` files is one JSON per line with a failure: + +```json +{"suite":"one","name":"foo"} +``` + +If it's expected that an external repository doesn't compile (because a PR introduces breaking changes +to, say, the type system) you can remove the `.failures.jsonl` file for that repository and CI +will pass again. Once the repository compiles again, CI will let us know and require us to put +back the `.failures.jsonl` file. \ No newline at end of file diff --git a/.github/critical_libraries_status/noir-lang/ec/.actual.jsonl b/.github/critical_libraries_status/noir-lang/ec/.actual.jsonl new file mode 100644 index 00000000000..cb56f792778 --- /dev/null +++ b/.github/critical_libraries_status/noir-lang/ec/.actual.jsonl @@ -0,0 +1,4 @@ +{"event":"started","name":"one","test_count":1,"type":"suite"} +{"event":"started","name":"foo","suite":"one","type":"test"} +{"event":"failed","exec_time":0.05356625,"name":"foo","suite":"one","type":"test"} +{"event":"ok","failed":0,"ignored":0,"passed":1,"type":"suite"} \ No newline at end of file diff --git a/.github/critical_libraries_status/noir-lang/ec/.actual.jsonl.jq b/.github/critical_libraries_status/noir-lang/ec/.actual.jsonl.jq new file mode 100644 index 00000000000..1123e7e68e4 --- /dev/null +++ b/.github/critical_libraries_status/noir-lang/ec/.actual.jsonl.jq @@ -0,0 +1 @@ +{"suite":"one","name":"foo"} diff --git a/.github/critical_libraries_status/noir-lang/ec/.failures.jsonl b/.github/critical_libraries_status/noir-lang/ec/.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/noir-lang/ec/.failures.jsonl.jq b/.github/critical_libraries_status/noir-lang/ec/.failures.jsonl.jq new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/noir-lang/eddsa/.failures.jsonl b/.github/critical_libraries_status/noir-lang/eddsa/.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/noir-lang/mimc/.failures.jsonl b/.github/critical_libraries_status/noir-lang/mimc/.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/noir-lang/noir-bignum/.failures.jsonl b/.github/critical_libraries_status/noir-lang/noir-bignum/.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/noir-lang/noir-edwards/.failures.jsonl b/.github/critical_libraries_status/noir-lang/noir-edwards/.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/noir-lang/noir_base64/.failures.jsonl b/.github/critical_libraries_status/noir-lang/noir_base64/.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/noir-lang/noir_bigcurve/.failures.jsonl b/.github/critical_libraries_status/noir-lang/noir_bigcurve/.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/noir-lang/noir_json_parser/.failures.jsonl b/.github/critical_libraries_status/noir-lang/noir_json_parser/.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/noir-lang/noir_rsa/.failures.jsonl b/.github/critical_libraries_status/noir-lang/noir_rsa/.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/noir-lang/noir_sort/.failures.jsonl b/.github/critical_libraries_status/noir-lang/noir_sort/.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/noir-lang/noir_string_search/.failures.jsonl b/.github/critical_libraries_status/noir-lang/noir_string_search/.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/noir-lang/schnorr/.failures.jsonl b/.github/critical_libraries_status/noir-lang/schnorr/.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/critical_libraries_status/noir-lang/sparse_array/.failures.jsonl b/.github/critical_libraries_status/noir-lang/sparse_array/.failures.jsonl new file mode 100644 index 00000000000..e69de29bb2d diff --git a/.github/scripts/check_test_results.sh b/.github/scripts/check_test_results.sh new file mode 100755 index 00000000000..c844c025876 --- /dev/null +++ b/.github/scripts/check_test_results.sh @@ -0,0 +1,38 @@ +#!/bin/bash +set -eu + +# Usage: ./check_test_results.sh +# Compares the output of two test results of the same repository. +# If any of the files doesn't exist or is empty, the script will consider that the test suite +# couldn't be compiled. + +function process_json_lines() { + cat $1 | jq -c 'select(.type == "test" and .event == "failed") | {suite: .suite, name: .name}' | jq -s -c 'sort_by(.suite, .name) | .[]' > $1.jq +} + +if [ -f $1 ] && [ -f $2 ]; then + # Both files exist, let's compare them + $(process_json_lines $2) + if ! diff $1 $2.jq; then + echo "Error: test failures don't match expected failures" + echo "Lines prefixed with '>' are new test failures (you could add them to '$1')" + echo "Lines prefixed with '<' are tests that were expected to fail but passed (you could remove them from '$1')" + fi +elif [ -f $1 ]; then + # Only the expected file exists, which means the actual test couldn't be compiled. + echo "Error: external library tests couldn't be compiled." + echo "You could rename '$1' to '$1.does_not_compile' if it's expected that the external library can't be compiled." + exit -1 +elif [ -f $2 ]; then + # Only the actual file exists, which means we are expecting the external library + # not to compile but it did. + echo "Error: expected external library not to compile, but it did." + echo "You could create '$1' with these contents:" + $(process_json_lines $2) + cat $2.jq + exit -1 +else + # Both files don't exists, which means we are expecting the external library not + # to compile, and it didn't, so all is good. + exit 0 +fi \ No newline at end of file diff --git a/.github/workflows/test-js-packages.yml b/.github/workflows/test-js-packages.yml index d227b4eb00b..e593389a971 100644 --- a/.github/workflows/test-js-packages.yml +++ b/.github/workflows/test-js-packages.yml @@ -543,8 +543,6 @@ jobs: external-repo-checks: needs: [build-nargo, critical-library-list] runs-on: ubuntu-22.04 - # Only run when 'run-external-checks' label is present - if: contains(github.event.pull_request.labels.*.name, 'run-external-checks') timeout-minutes: 30 strategy: fail-fast: false @@ -557,11 +555,17 @@ jobs: - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/parity-lib } - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-lib } - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/reset-kernel-lib } - - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-lib } - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/types } + # Use 1 test threads for rollup-lib because each test requires a lot of memory, and multiple ones in parallel exceed the maximum memory limit. + - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-lib, nargo_args: "--test-threads 1" } name: Check external repo - ${{ matrix.project.repo }}/${{ matrix.project.path }} steps: + - name: Checkout + uses: actions/checkout@v4 + with: + path: noir-repo + - name: Checkout uses: actions/checkout@v4 with: @@ -592,9 +596,14 @@ jobs: - name: Run nargo test working-directory: ./test-repo/${{ matrix.project.path }} - run: nargo test -q --silence-warnings + run: | + out=$(nargo test --silence-warnings --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }}) && echo "$out" > ${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl env: NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true + + - name: Compare test results + working-directory: ./noir-repo + run: .github/scripts/check_test_results.sh .github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.failures.jsonl .github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl # This is a job which depends on all test jobs and reports the overall status. # This allows us to add/remove test jobs without having to update the required workflows. diff --git a/tooling/nargo_cli/src/cli/test_cmd/formatters.rs b/tooling/nargo_cli/src/cli/test_cmd/formatters.rs index 1b9b2d50378..75cf14ba120 100644 --- a/tooling/nargo_cli/src/cli/test_cmd/formatters.rs +++ b/tooling/nargo_cli/src/cli/test_cmd/formatters.rs @@ -417,6 +417,7 @@ impl Formatter for JsonFormatter { let mut json = Map::new(); json.insert("type".to_string(), json!("test")); json.insert("name".to_string(), json!(&test_result.name)); + json.insert("suite".to_string(), json!(&test_result.package_name)); json.insert("exec_time".to_string(), json!(test_result.time_to_run.as_secs_f64())); let mut stdout = String::new();