feat: configurable external check failures (#6810)

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

.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.

.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"}

.github/critical_libraries_status/noir-lang/ec/.actual.jsonl.jq

@@ -0,0 +1 @@
+{"suite":"one","name":"foo"}

.github/scripts/check_test_results.sh

@@ -0,0 +1,38 @@
+#!/bin/bash
+set -eu
+
+# Usage: ./check_test_results.sh <expected.json> <actual.jsonl>
+# 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

.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.

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();