test(connlib): assert determinism of strategies in unit-test (#6846)

In the past, we struggled a lot of the reproducibility of `tunnel_test`
failures because our input state and transition strategies were not
deterministic. In the end, we found out that it was due to the iteration
order of `HashMap`s.

To make sure this doesn't regress, we added a check to CI at the time
that compares the debug output of all regression seeds against a 2nd run
and ensures they are the same. That is overall a bit wonky.

We can do better by simple sampling a value from the strategy twice from
a test runner with the same seed. If the strategy is deterministic,
those need to be the same. We still rely on the debug output being
identical because:

a. Deriving `PartialEq` on everything is somewhat cumbersome
b. We actually care about the iteration order which a fancy `PartialEq`
implementation might ignore
This commit is contained in:
Thomas Eizinger
2024-09-28 00:46:51 +10:00
committed by GitHub
parent ab66a8fec7
commit 7209060c42
2 changed files with 43 additions and 17 deletions

View File

@@ -111,21 +111,6 @@ jobs:
rg --count --no-ignore "Performed IP-NAT46" $TESTCASES_DIR
rg --count --no-ignore "Performed IP-NAT64" $TESTCASES_DIR
# Backup dumped state and transition samples
mv $TESTCASES_DIR $TESTCASES_BACKUP_DIR
# Re-run only the regression seeds
PROPTEST_CASES=0 cargo test --all-features ${{ steps.setup-rust.outputs.packages }} -- tunnel_test --nocapture
# Assert that sampled state and transitions don't change between runs
for file in "$TESTCASES_DIR"/*.{state,transitions}; do
filename=$(basename "$file")
if ! diff "$file" "$TESTCASES_BACKUP_DIR/$filename"; then
echo "Found non-deterministic testcase: $filename"
exit 1
fi
done
env:
# <https://github.com/rust-lang/cargo/issues/5999>
# Needed to create tunnel interfaces in unit tests
@@ -134,7 +119,6 @@ jobs:
PROPTEST_CASES: 1000 # Default is only 256.
CARGO_PROFILE_TEST_OPT_LEVEL: 1 # Otherwise the tests take forever.
TESTCASES_DIR: "connlib/tunnel/testcases"
TESTCASES_BACKUP_DIR: "connlib/tunnel/testcases_backup"
# Runs the Tauri client smoke test, built in debug mode. We can't run it in release
# mode because of a known issue: <https://github.com/firezone/firezone/blob/456e044f882c2bb314e19cc44c0d19c5ad817b7c/rust/windows-client/src-tauri/src/client.rs#L162-L164>

View File

@@ -1,6 +1,10 @@
use crate::tests::{flux_capacitor::FluxCapacitor, sut::TunnelTest};
use assertions::PanicOnErrorEvents;
use proptest::test_runner::{Config, TestError, TestRunner};
use core::fmt;
use proptest::{
strategy::{Strategy, ValueTree as _},
test_runner::{Config, RngAlgorithm, TestError, TestRng, TestRunner},
};
use proptest_state_machine::ReferenceStateMachine;
use reference::ReferenceState;
use std::sync::atomic::{self, AtomicU32};
@@ -113,6 +117,44 @@ fn tunnel_test() {
}
}
#[test]
fn reference_state_is_deterministic() {
for n in 0..1000 {
let state1 = sample_from_strategy(n, ReferenceState::init_state());
let state2 = sample_from_strategy(n, ReferenceState::init_state());
assert_eq!(format!("{state1:?}"), format!("{state2:?}"));
}
}
#[test]
fn transitions_are_deterministic() {
for n in 0..1000 {
let state = sample_from_strategy(n, ReferenceState::init_state());
let transitions1 = sample_from_strategy(n, ReferenceState::transitions(&state));
let transitions2 = sample_from_strategy(n, ReferenceState::transitions(&state));
assert_eq!(format!("{transitions1:?}"), format!("{transitions2:?}"));
}
}
fn sample_from_strategy<S, T>(seed: u64, strategy: S) -> T
where
S: Strategy<Value = T> + fmt::Debug,
T: fmt::Debug,
{
strategy
.new_tree(&mut TestRunner::new_with_rng(
Config::default(),
TestRng::from_seed(
RngAlgorithm::default(),
seed.to_be_bytes().repeat(4).as_slice(),
),
))
.unwrap()
.current()
}
/// Initialise logging for [`TunnelTest`].
///
/// Log-level can be controlled with `RUST_LOG`.