From 7209060c4238cadde373f26047e3d153ae08cfb1 Mon Sep 17 00:00:00 2001
From: Thomas Eizinger <thomas@eizinger.io>
Date: Sat, 28 Sep 2024 00:46:51 +1000
Subject: [PATCH] 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
---
 .github/workflows/_rust.yml      | 16 ------------
 rust/connlib/tunnel/src/tests.rs | 44 +++++++++++++++++++++++++++++++-
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/.github/workflows/_rust.yml b/.github/workflows/_rust.yml
index 5f160edb1..86e4383ce 100644
--- a/.github/workflows/_rust.yml
+++ b/.github/workflows/_rust.yml
@@ -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>
diff --git a/rust/connlib/tunnel/src/tests.rs b/rust/connlib/tunnel/src/tests.rs
index cb3842316..2cd2b248e 100644
--- a/rust/connlib/tunnel/src/tests.rs
+++ b/rust/connlib/tunnel/src/tests.rs
@@ -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`.