Tests in static_any_map.rs are not atomic, may cause spurious failure.

[kennytm]

Generally, Rust runs tests in parallel.

In https://github.com/tinco/entity_rust/blob/master/tests/static_any_map.rs, the 3 tests all operate on the same global map. Even if the map is wrapped in a SharedMutex, it is not enough to ensure thread-safety, e.g. some code in static_map_initial_set may run in the middle of static_map_multi_set, causing the test to spuriously fail.

Thread 1	Thread 2
static_map_initial_set()
	static_map_multi_set()
my_map::clear_all()
my_map::push(&name, i)
	my_map::clear_all()
let queue = my_map::get(&name)
Received an empty queue
assert_eq!(queue.len(), 1); but queue.len() == 0
💥

The tests can be serialized using a mutex, see rust-lang/rust#43155.

[tinco]

Hi @kennytm! Thanks for reporting this, I actually have known about this issue and worked around it by running the test suite in single threaded mode somehow, it's been a while and I forgot the details.

How did you come across this repo? I worked very actively on this for a while, but I then started renovating my house which has taken the better of my free time the past year. In the meanwhile a similar entity system project managed to put out a very usable library so I think there's no use for this project any longer. I guess I should pull it from Cargo, did it break your automatic ecosystem testing or something? :)

[kennytm]

@tinco Hi,

Before we release a new version of the Rust compiler, or introduce a potentially backward-incompatible feature (this case), we will run a tool called "cargobomb" that tests the new compiler against all 11000 packages on crates.io, to ensure we don't accidentally break our stability guarantee.

However, some crates, like yours, contain "flaky tests" which caused cargobomb to report a regression, where the problem doesn't really originate from the compiler. While we do blacklist these crates, I'd like to inform their authors as well, so future versions can continue participate in the experiment.

You don't need to pull it from crates.io (we always do manual inspection since there are really a lot of false positives). Nevertheless, fixing the tests would be even better 😄.