feat: several nargo test improvements

[asterite]

Description

Problem

Resolves #4489
Resolves #6724
Resolves #6727

Summary

This PR does a few things:
2. We now use a maximum number of threads to run tests. The default is the system's default (taken from a Rust function) but it can be overwritten with the new --num-threads CLI option (let me know if this should be removed, though, probably always parallelizing is never an issue, unlike in Rust)
3. We now compile all packages and show all errors before starting to run tests (previously the first error would abort the run)
4. Before showing how many tests passed and failed in a package, a list of the tests that failed is shown. This is similar to Rust too. I personally find this useful because otherwise to know which failed you have to scroll up and fine the red "fail" ones, which is a bit time-consuming. But let me know and we could revert this.
5. Output is shown by package. That is, first we show all output for a package, then all output for another package, etc. However, we do run all tests in parallel.
6. Output fron Noir's println isn't printed immediately. Instead, it's captured in a String and associated to the test. Then, if --show-output is given, that output is printed after the test name status. That makes it very easy to see what output each test produced, and output from different tests is never mixed up.
7. For slow tests (those that take more than 30 seconds) we now show the time.

An example of 6:

And consider this Noir program:

#[test]
pub fn one() {
    println("one (1)");
    println("one (2)");
    println("one (3)");
}

#[test]
pub fn two() {
    println("two (1)");
    println("two (2)");
    println("two (3)");
}

#[test]
pub fn three() {
    println("three (1)");
    println("three (2)");
    println("three (3)");
}

The new output for nargo test is:

The old output was this:

Additional Context

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.

[github-actions]

Peak Memory Sample

Program	Peak Memory
keccak256	78.17M
workspace	121.67M
regression_4709	294.72M
ram_blowup_regression	2.44G
private-kernel-tail	208.76M
private-kernel-reset	861.44M
private-kernel-inner	307.68M
parity-root	174.42M

[TomAFrench]

Looks good but I'm seeing a number of new test failures in the run-external-checks tests (the private_rollup_lib tests).

[asterite]

Strange, they pass on my machine.

But also, nargo test is now much slower when there are multiple packages in the workspace, I think it's because they are compiled sequentially.

I'll see if I can compile things in parallel...

[TomAFrench]

Hmm, we'd need to buffer the test results in that case though wouldn't we otherwise we'd get tests from different packages potentially interleaved.

[asterite]

I managed to do the above (first compile all packages in parallel, then run tests in parallel for each package) but it's still slower than before. For example if there are many, many packages with a very small number of tests, for example 3, and we have 10 threads, we'll just run 3 in parallel in sequential batches.

Maybe running everything in parallel like before is fine? We could still show a summary at the end for each package separately, which is maybe what matters most. What do you think?

[asterite]

Alternatively we could try to just show things sequentially, but still run everything in parallel... but I'm not sure how we could do that. Something like "run everything in parallel, but get the output of the first package first, then the output of the second package, etc.". Maybe we can have a receiver for results: when we get a result, if it's for the package we are interested in we output it, otherwise put it in a HashMap keyed by that package name. Once we finish with a package we first show what we have in the cached output, then continue receiving output from the package, etc... though it will make the code much harder to understand. I'll try it anyway.

[aakoshh]

Couple of questions:

• are you using the same number of threads?
• what if you disable the printing of results after each test?

I think it wasn’t about going faster but to limit the amount of resource usage; the ticket mentioned running out of memory.

[TomAFrench]

Yeah, I don't mind if things are a little slower as long as we can run the protocol tests with a simple nargo test (and not needing hacks like artificially reducing the number of cores). We can always improve the speed in future PRs.

[aakoshh]

If you consider my earlier pseudo code that set up workers up front, you can imagine setting up a pipeline where you feed tests in order into the work queue, so it will be package A first then package B, and in another component you tell up front how many results are to be expected from each package. That component gets the results in a channel, and if they are out of order it buffers them, if it’s for the current item it shows them, when it reaches the expected number of reports it shows the summary and starts outputting the next one from the buffer

[asterite]

@aakoshh Right, I looked at your code to see if I could use it, because it seems what we'd need to do in the end. The problem is that to get the tests to run we need to compile a package. In the pseudocode packages are compiled sequentially and that's what's slow.

However, I'll at least change things so that all packages are compiled in parallel. There's still a slowdown because we don't run tests from different packages in parallel. If the issue was too many threads and out of memory, using worker threads solved this but then I suggested running packages sequentially and that's what caused the additional slowdown. What I'm saying is that we could revert that functionality, get rid of the "out of memory", and still have things work really fast.

[asterite]

Well, running tests for noir-contracts (with oracles failing) now takes 6 seconds vs. 2, so maybe it's not a big deal. I'll just push a commit that parallelizes the compilation.

[asterite]

Now that I think about it, I wonder if stdout for each test should be shown before each test instead of after. Or ir could be after, like now, but maybe precede it with "standard outfor for test:"

[TomAFrench]

Or ir could be after, like now, but maybe precede it with "standard outfor for test:"

I would say this.

[aakoshh]

Very nice!

[asterite]

One more thing I did: I noticed that if we have many packages, we first print how many tests are for each package, then print package tests sequentially. However, it's probably nicer if we print the package tests count and the tests all together. That is, instead of this:

[one] Running 2 test functions
[two] Running 1 test function
[one] Testing a... ok
[one] Testing b... ok
[one] 2 tests passed
[two] Testing c... ok
[two] 1 test passed

Show it like this:

[one] Running 2 test functions
[one] Testing a... ok
[one] Testing b... ok
[one] 2 tests passed
[two] Running 1 test function
[two] Testing c... ok
[two] 1 test passed

At this point I'm wondering if we actually need to prefix all the output with the package name. We could maybe print it at the beginning and the end, but not for every test. Thoughts?

(I don't have a strong opinion about this, but Rust more or less does that)

[TomAFrench]

At this point I'm wondering if we actually need to prefix all the output with the package name. We could maybe print it at the beginning and the end, but not for every test. Thoughts?

Let's keep it for now. This is consistent with other packages and we have quite a few changes in this PR already.

[aakoshh]

👏