Use loom in the tests

[ZenTauro]

We could use loom to test multiple permutations in the tests.

[ZenTauro]

I've been working on it, right now all the test pass except the concurrent_insert and concurrent_remove ZenTauro@fdb2bf2

[jonhoo]

I would love for us to also run the tests through miri!

[jonhoo]

See also #37

[ZenTauro]

Nice, however, when I run the test with miri and loom they don't produce any output.

[jonhoo]

Interesting.. What do they produce?

[ZenTauro]

It just exits with 0

[jonhoo]

Hmm, I'm not sure -- @carllerche?

Separately, I do not think we want to use debug_assertions to determine whether or not to use loom. Instead, loom tests should probably be separated out and written explicitly for that purpose given how slow they can be. We also will want to run loom tests in release mode, otherwise they'll be far too slow. In tokio I know they require env "RUSTFLAGS=--cfg loom" and then guard the loom tests with #[cfg(loom)].

[carllerche]

I don't know anything about miri + loom :) seems rough to mix the two. I don't think miri supports threading.

Also, correct re: RUSTFLAGS="--cfg loom".

Using loom right now is not friendly, if anyone is so inclined, we could definitely use some UX help :)

[jonhoo]

Ah, @ZenTauro, were you trying to run both at the same time? Yeah, that probably won't work. I think we probably want to run the two independently.

[ZenTauro]

Ah, @ZenTauro, were you trying to run both at the same time? Yeah, that probably won't work. I think we probably want to run the two independently.

Yeah, I didn't know they were interfering

Also, correct re: RUSTFLAGS="--cfg loom".

I'll put them behind --cfg loom

[ZenTauro]

I am trying to implement a macro that can annotate tests in order to run them with loom only if configured to do so. I am not really familiar with proc macros so I haven't been able to make a lot of progress, if someone is interested I'll appreciate some help in the implementation.
Here is the progress so far: ZenTauro@e0b5f28

[jonhoo]

I think the way to go is to require a compile-time --cfg flag for loom like tokio does. That way the tests can just be annotated with #[cfg(loom)] and #[cfg(not(loom))]

[ZenTauro]

The macro does that, it uses the cfg!() macro to wrap the test and if the --cfg flag is passed it passes the test as a closure to loom::model to run them with loom.
So instead of writing:

#[test]
fn some_test() {
    if cfg!(loom) {
        loom::model(|| {
            // test body
        });
    } else {
        // test body
    }
}

We can write:

#[test]
#[loom_macro::wrapper]
fn some_test() {
    // test body
}

[jonhoo]

I'm not sure we ever really want to write tests that way? Most commonly, loom tests are very tailor-written for loom to avoid too large a state space for it to explore. I don't know that we're likely to ever want to share a test between loom and non-loom. Or, phrased differently, I think we're more likely to see tests written this way:

#[cfg(loom)]
mod loom_tests {
    #[test]
    fn custom_written_test_for_loom() { }
}
#[cfg(not(loom)]
mod non_loom_tests {
    #[test]
    regular_test() {}
}

[jonhoo]

I've added initial support for loom in crossbeam-epoch in crossbeam-rs/crossbeam#487. @domenicquirl you may also be interested in that. In theory we should be able to write a loom test with a patch that points crossbeam-epoch (and crossbeam-utils) at that PR. That PR should also (hopefully) give some pointers for how to add loom support to a library — basically the same steps will be needed for flurry.

[ZenTauro]

Nice, I'll look into the patch