API for skipping current iteration

[ihtefyw]

Hello, firstly, thank you for the great tool!

In my application, we use shuttle to test interleaving of multiple operations on a specialized cache by running every possible combination of operations and IDs. We know that some of those combinations are invalid and will corrupt the cache (e.g. concurrent delete and insertion of the same ID), and when we detect such case we log the shuttle error and continue to the next combination. However, this means that we abort the shuttle execution at the first error while there could be others for the same combination of operations, possibly resulting into an actual invalid state for our application.

What would be nice to have is something like a shuttle::continue API to inform shuttle that we expected an error there and just log/ignore it.
I'm not sure if this is possible with the current design, or if there is already a way to achieve this that I overlooked.

[sarsko]

Hi there,

Thank you, I'm sure we can find a way to do that.

I'm not entirely sure I fully understand your issue, but could this be done by creating some variant of the Runner/PortfolioRunner which runs Shuttle with some scheduler, catches the panic, then keeps running a "new" Shuttle (ie a new Execution) with the same scheduler?

[ihtefyw]

I am exactly sure of what an Execution is in shuttle terms (I suppose it's all the interleaving for the provided closure).

Maybe it's better to give a more in-depth overview of my use case:
we have a set of operations (add, delete, get) on different IDs on the cache. We generate combinations of those operations and IDs (currently six per batch) in an outer loop and then run a shuttle test where each of those operations are run on a different thread (so, one shuttle test with six threads).
Some of the interleaving created by Shuttle are illegal for our cache. To detect it, we catch the panic from Shuttle and continue the outer loop (i.e. continue to the next combination of operations and IDs).
For example, if Shuttle finds an expected panic on the combination [Add ID 0, Add ID 1, Delete ID 0], we ignore it and execute shuttle on the next test case, let's say [Add ID 0, Add ID 1, Delete ID 1].

The issue here is that we are aborting the shuttle test at the first invalid interleaving, while there may be other interleaving for the same combination which may cause an unexpected panic.
Ideally, we would like to detect the expected invalid state within the Shuttle test, and inform Shuttle to discard what it's doing and test another interleaving of the same test case.
Currently, we detect this outside the Shuttle test and skip to the next test case.

I hope this makes the issue now :)

[sarsko]

Thanks, yeah, I think I understand your use case and the desired functionality.

It's not entirely clear why it's relevant whether we are within or outside the Shuttle test, the difference seems to be mostly a matter of performance, and I think the performance improvements of staying inside should be negligible. Also maybe staying inside might be a bit cleaner code wise.

That said, we actually wanted to make the same functionality to test the schedulers that @dylanjwolff has made, so hopefully the solution we make for that will cover your use case as well. We'll probably get to that sometime this week.

[ihtefyw]

I tried to play a bit with my test and to catch the panics within the shuttle test.
My test looks kinda like this

 shuttle::check_random(
        || {
            if let Err(e) = catch_unwind(move || {
                let handle = thread::spawn(|| Err::<(), String>("Expected error".to_owned()));
                if let Err(e) = handle.join().unwrap() {
                    panic_any(e);
                }
            }) {
                eprintln!("Skipping: {:?}", e.downcast_ref::<String>().unwrap());
            }
        },
        1,
    );

While this allows me to skip the erroneous state and terminate the test successfully, Shuttle still detects that a panic happened and generates the error message

test panicked in task 'main-thread'
failing schedule:
"
910104a3dcf685ddace0e15c08
"
pass that string to `shuttle::replay` to replay the failure

thread 'node_manager::lock_cache::shuttle_tests::shuttle_catch' panicked at src/node_manager/lock_cache/shuttle_tests.rs:309:21:
Expected error
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Skipping: "Expected error"

With file persistence for seeds, this will generate a schedule file for each panic, even if those are technically skipped.
Ideally, this should not give any warning or errors at all, as the test code is valid.

[sarsko]

Sorry, I didn't get to this this week. I'll probably get something out next week.

I thought about this some more, and my proposed way to solve this is kinda lazy and not that nice. I think adding a more
"native" support for this would be a lot nicer, so that's what I'll add.

[sarsko]

Regarding the code you showed:

Yeah, currently catching panics still means Shuttle serializes a schedule. I'll look at making it configurable, but until then it can be overwritten by setting your own panic hook inside the test (ie it has to be set after Shuttle sets its panic hook)

[ihtefyw]

Hi again, I have a follow-up on this.
I decided to try with setting the panic_hook within the test as you suggested, however this doesn't silence the print.
During debugging, I saw that the user provided function to the shuttle scheduler is wrapped in a catch_unwind which explicitly calls persist_failure.
Moreover, the runtime/failure.rs docs says that the panic persistence is counted to avoid printing that twice, i.e. in the hook and in the catch_unwind. And of course, disabling the hook leave the catch_unwind free to print.