Skip to content

chore(test): Run ram_blowup_regression only on CI#9717

Closed
aakoshh wants to merge 1 commit intomasterfrom
af/ram-blowup-ci-only
Closed

chore(test): Run ram_blowup_regression only on CI#9717
aakoshh wants to merge 1 commit intomasterfrom
af/ram-blowup-ci-only

Conversation

@aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Sep 3, 2025

Description

Problem*

Executing integration can be slow, in particular ram_blowup_regression takes up a disproportionate amount of time.

Summary*

Changes the nargo_cli integration tests to exclude ram_blowup_regression unless the CI env var is set.

Additional Context

Say I want to have an idea whether commenting out some code in an SSA pass breaks any of the integration or regression tests. I can run the following command, which runs almost 700 tests; way less than the total (over 5000). With this PR, this takes just over 15 seconds on my laptop.

cargo test -p nargo_cli --test execute forcebrillig_false_inliner_i64_min

However if I include the ram_blowup_regressions, this goes up to almost 2 minutes, which means I'm less likely to run the tests:

CI=1 cargo test -p nargo_cli --test execute ram_blowup_regression::forcebrillig_false_inliner_i64_min
    Finished `test` profile [optimized + debuginfo] target(s) in 0.61s
     Running tests/execute.rs (target/debug/deps/execute-44c3ac57c912e029)

running 2 tests
test tests::execution_success::test_ram_blowup_regression::forcebrillig_false_inliner_i64_min_expects has been running for over 60 seconds
test tests::interpret_execution_success::test_ram_blowup_regression::forcebrillig_false_inliner_i64_min_expects has been running for over 60 seconds
test tests::interpret_execution_success::test_ram_blowup_regression::forcebrillig_false_inliner_i64_min_expects ... ok
test tests::execution_success::test_ram_blowup_regression::forcebrillig_false_inliner_i64_min_expects ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 5381 filtered out; finished in 108.74s

(It is not exactly clear why it's so slow, because if I cd into the directory and use cargo run -p nargo_cli -- execute --force then it's fast).

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

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

@aakoshh aakoshh requested a review from a team September 3, 2025 10:21
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite Current: af2f824 Previous: 8fd9446 Ratio
test_report_zkpassport_noir-ecdsa_ 3 s 2 s 1.50
test_report_zkpassport_noir_rsa_ 2 s 1 s 2

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

@asterite
Copy link
Collaborator

asterite commented Sep 3, 2025

I think this means this test will never run, as it currently doesn't run in CI:

[profile.ci]
# Do not cancel the test run on the first failure.
fail-fast = false
# We do not run `ram_blowup_regression` by default as it's very slow
default-filter = "not test(ram_blowup_regression)"

@asterite
Copy link
Collaborator

asterite commented Sep 3, 2025

Maybe it could only run in nightly? Not sure how that runs but I know there's something that runs nightly from which we get fuzz failures?

@TomAFrench
Copy link
Member

We run this test in CI on the master branch. We just disable it for PRs to make them quicker to merge.

I'd like to lean into just using nextest profiles if we want to vary how tests are run under certain circumstances.

@aakoshh
Copy link
Contributor Author

aakoshh commented Sep 3, 2025

Okay thanks, I'll use nextest to pick a profile locally 👍

cargo nextest run -p nargo_cli --test execute forcebrillig_false_inliner_i64_min --filter-expr "not test(ram_blowup)"

@aakoshh aakoshh closed this Sep 3, 2025
@asterite
Copy link
Collaborator

asterite commented Sep 3, 2025

I do agree that ram_blowup_regression takes a lot of time, and lately I run the tests locally very frequently, so having something as a default for everyone (except master) would be great.

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.

3 participants