libtest: Use deterministic HashMap, avoid spawning thread if there is no concurrency

[RalfJung]

It seems desirable to make a test and bench runner deterministic, which this achieves by using a deterministic hasher. Also, we we only have 1 thread, we don't bother spawning one and just use the main thread.

The motivation for this is to be able to run the test harness in miri, where we can neither access the OS RNG, nor spawn threads.

[rust-highfive]

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

[Centril]

Hmm... isn't the non-determinism we have now on purpose so that we expose more bugs in the system under test that relies on orderings in ways it should not?

[RalfJung]

Hmm... isn't the non-determinism we have now on purpose so that we expose more bugs in the system under test that relies on orderings in ways it should not?

Really? Is that documented anywhere? It looks like a mere "HashMap is the default" to me.

Also, for benchmarks, you want them as reproducible as possible, so there the non-determinism is certainly not desired.

[Centril]

Really? Is that documented anywhere? It looks like a mere "HashMap is the default" to me.

I thought I read a blog post about this somewhere but I cannot find it right now... There's http://jakegoulding.com/blog/2012/10/18/run-your-tests-in-a-deterministic-random-order/ tho.

[RalfJung]

There's http://jakegoulding.com/blog/2012/10/18/run-your-tests-in-a-deterministic-random-order/ tho.

Interesting. However, libtest did not provide deterministic random order for sure, and also I'd say the problem of global state is much smaller in Rust.

[Centril]

@RalfJung Sure; Having some way for libtest to spit out the seed before it starts testing would be good; you could then reuse that seed for the determinism angle. This is essentially what tools such as QuickCheck in Haskell and proptest do.

In this case, the global mutable state wouldn't be an object in Rust per se, but rather some object such as a file system or database that the test may write and read from; if you then have a deterministic ordering of the tests, that could cause tests to pass when they shouldn't; randomizing the order would in some cases expose the dependencies and cause warranted failures.

[RalfJung]

Having some way for libtest to spit out the seed before it starts testing would be good; you could then reuse that seed for the determinism angle. This is essentially what tools such as QuickCheck in Haskell and proptest do.

Not just spit out, one would also have to be able to set the seed.

However, this entire discussion has nothing to do with the patch.^^ The affected HashMap serves to keep track of which tests are still running, and the only time it is iterated is in get_timed_out_tests to compute which tests timed out.

[Dylan-DPC-zz]

Ping from triage @sfackler awaiting your review on this.

[RalfJung]

@alexcrichton could you have a look, or pick someone who could have a look?

[alexcrichton]

This looks fine by me to land, thanks @RalfJung! If we're gonna comment /* concurrency= */ in most places, though, perhaps an enum could be used like Concurrent::Yes or Concurrent::No? Other than that r=me!

[Dylan-DPC-zz]

r? @alexcrichton

[RalfJung]

@bors r=alexcrichton

[bors]

📌 Commit c28c287 has been approved by alexcrichton

[asomers]

I believe this PR is the cause of #58907 . The switch to running all tests in a single-thread is a user-visible change, and it needs to be mentioned in the Rust release notes.

[RalfJung]

This PR didn't introduce a thread pool, so at least that issue description cannot be entirely right. But indeed, it seems like the libtest code used to spawn a new thread per test; now it's all happening in the main thread.