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

feat: configurable external check failures #6810

Merged
merged 37 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
3c669de
Run external checks with `--format json`
asterite Dec 13, 2024
825a909
Include "suite" in test end event
asterite Dec 16, 2024
79a60b7
Compare test results
asterite Dec 16, 2024
e087aaa
Output inside github.workspace
asterite Dec 16, 2024
d114780
Move files around
asterite Dec 16, 2024
5e06ce3
Merge branch 'master' into ab/external-checks-ignore-test-failures
asterite Dec 16, 2024
d7bde50
More fixes
asterite Dec 16, 2024
eafea81
debug
asterite Dec 16, 2024
c35ab86
Need to checkout current repo?
asterite Dec 16, 2024
8eed8c9
Checkout in noir-repo
asterite Dec 16, 2024
1068209
Show actual run output
asterite Dec 16, 2024
1635062
Try to use correct names
asterite Dec 16, 2024
b31ac6b
More files
asterite Dec 16, 2024
fabdcd0
Fix script
asterite Dec 16, 2024
fe2e626
Reuse function and better output
asterite Dec 16, 2024
bd8bb22
Only compare failed and ignored tests
asterite Dec 16, 2024
29a19ae
Only compare failures
asterite Dec 16, 2024
7bacd52
Run rollup-lib tests with 2 threads
asterite Dec 16, 2024
41baafc
Try with 1 thread
asterite Dec 16, 2024
77a1eb6
Skip rollup-lib for now
asterite Dec 16, 2024
e204139
chore: always run external checks
TomAFrench Dec 16, 2024
99170d9
Check rollup-lib again
asterite Dec 16, 2024
c62e152
Merge branch 'ab/external-checks-ignore-test-failures' of github.com:…
asterite Dec 16, 2024
769b89b
Merge branch 'master' into ab/external-checks-ignore-test-failures
asterite Dec 16, 2024
5e37a5e
Revert "Check rollup-lib again"
asterite Dec 16, 2024
883ed4e
Merge branch 'master' into ab/external-checks-ignore-test-failures
asterite Dec 16, 2024
70e9a34
Reapply "Check rollup-lib again"
asterite Dec 17, 2024
ef0e1b9
Increase timeout minutes for external checks
asterite Dec 17, 2024
3643e83
Document a bit, renames, and some fixes
asterite Dec 17, 2024
eaef064
Don't create file if command fails
asterite Dec 17, 2024
c7ecc5a
Missed renaming file
asterite Dec 17, 2024
00117c7
Use one thread for rollup-lib
asterite Dec 17, 2024
1db6d59
Add file
asterite Dec 17, 2024
9cda5db
--skip-brillig-constraints-check
asterite Dec 17, 2024
fe66452
Explain why we use 1 thread for rollup-lib
asterite Dec 17, 2024
4aa9232
Restore the 30 minutes limit
asterite Dec 17, 2024
a753ddb
Update scripts/check-critical-libraries.sh
TomAFrench Dec 17, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
35 changes: 35 additions & 0 deletions .github/scripts/check_test_results.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#!/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, status: .event}' | 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 $1)
$(process_json_lines $2)
diff $1.jq $2.jq
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
15 changes: 13 additions & 2 deletions .github/workflows/test-js-packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -557,11 +557,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 }
# This one currently doesn't compile (it runs out of memory or it takes too long)
# - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-lib, nargo_args: "--test-threads 1" }
asterite marked this conversation as resolved.
Show resolved Hide resolved

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:
Expand Down Expand Up @@ -592,9 +598,14 @@ jobs:

- name: Run nargo test
working-directory: ./test-repo/${{ matrix.project.path }}
run: nargo test -q --silence-warnings
run: |
nargo test --silence-warnings --format json ${{ matrix.project.nargo_args }} > ${{ 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 }}.expected.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.
Expand Down
2 changes: 1 addition & 1 deletion scripts/check-critical-libraries.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ for REPO in ${REPOS_TO_CHECK[@]}; do
TAG=$(getLatestReleaseTagForRepo $REPO)
git clone $REPO -c advice.detachedHead=false --depth 1 --branch $TAG $TMP_DIR

nargo test -q --program-dir $TMP_DIR
nargo test --program-dir $TMP_DIR --format json
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved

rm -rf $TMP_DIR
done
1 change: 1 addition & 0 deletions tooling/nargo_cli/src/cli/test_cmd/formatters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading