Abscissa context poisoned in tests

[greg-szabo]

I've been using the Abscissa test framework to do integration testing.

The shared context gets poisoned if one of the tests fails, bringing down all the other tests with it. (Regardless of if they should succeed or fail.)

I guess this should be considered a bug, but I don't know the working details to evaluate the issue.

This is a log from GitHub Actions: https://github.com/informalsystems/ibc-rs/runs/926134260
The query_channel_id test fails properly, but the upcoming query_client_id and query_connection_id tests, which should fail similarly, panic instead, because of the poisoned mutex.
Note: the tests all fail, even if only one fails properly and the rest should succeed.

---- query_channel_id stdout ----
thread 'query_channel_id' panicked at 'assertion failed: `(left == right)`
  left: `"\u{1b}[0m\u{1b}[0m\u{1b}[1m\u{1b}[36m     Options\u{1b}[0m QueryChannelOptions { port_id: PortId(\"firstport\"), channel_id: ChannelId(\"firstchannel\"), height: 0, proof: true }"`,
 right: `"     Options QueryChannelOptions { port_id: PortId(\"firstport\"), channel_id: ChannelId(\"firstchannel\"), height: 0, proof: true }"`', /rustc/5c1f21c3b82297671ad3ae1e8c942d2ca92e84f2/src/libstd/macros.rs:16:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- query_client_id stdout ----
thread 'query_client_id' panicked at 'poisoned cmd mutex!: "PoisonError { inner: .. }"', /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/abscissa_core-0.5.2/src/testing/runner.rs:195:26

---- query_connection_id stdout ----
thread 'query_connection_id' panicked at 'poisoned cmd mutex!: "PoisonError { inner: .. }"', /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/abscissa_core-0.5.2/src/testing/runner.rs:195:26

[tony-iqlusion]

Yeah, this is a bug. The runner should catch the panics but otherwise release the mutex cleanly.

[simonsan]

Yeah, this is a bug. The runner should catch the panics but otherwise release the mutex cleanly.

// The lock is poisoned by this point, but the returned result can be
// pattern matched on to return the underlying guard on both branches.
let mut guard = match lock.lock() {
    Ok(guard) => guard,
    Err(poisoned) => poisoned.into_inner(),
};

src.: https://doc.rust-lang.org/std/sync/struct.Mutex.html#examples

Wouldn't be just matching on the result in https://github.com/iqlusioninc/abscissa/blob/main/core/src/testing/runner.rs#L195 be easier than to panic::catch_unwind. In the end, it is kind of expected that a thread might panic during a test case, which would poison the mutex.

[tony-iqlusion]

Without catch_panic the mutex will be poisoned.

Adding a match expression where you're suggesting won't prevent the mutex from becoming poisoned in the first place.