From 273988b8d569aae0b989d126f688d7367020a1dd Mon Sep 17 00:00:00 2001
From: Nathan Sobo <nathan@zed.dev>
Date: Tue, 27 Dec 2022 16:47:28 -0700
Subject: [PATCH] Set transaction group interval to ZERO by default in tests

We were seeing non-deterministic behavior in randomized tests when
generating backtraces took enough time to cause transactions to group
in some cases, but not group in others.

Tests will need to explicitly opt into grouping if they want it by
setting the interval explicitly. We have tests in the text module that
currently test the history grouping explicitly, but I'm not sure
it's needed elsewhere.
---
 crates/client/src/client.rs                             | 8 +-------
 crates/collab/src/tests/randomized_integration_tests.rs | 2 +-
 crates/gpui/src/executor.rs                             | 4 +---
 crates/text/src/tests.rs                                | 7 ++++---
 crates/text/src/text.rs                                 | 4 ++++
 5 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs
index 60af7e0d0b9225ce390a4a9054db9434c920d0e4..aa46d64fcb250d30ac4c41c97497bccd40b7ad74 100644
--- a/crates/client/src/client.rs
+++ b/crates/client/src/client.rs
@@ -851,7 +851,6 @@ impl Client {
             })
             .detach();
 
-        let t0 = Instant::now();
         let this = self.clone();
         let cx = cx.clone();
         cx.foreground()
@@ -868,12 +867,7 @@ impl Client {
                         }
                     }
                     Err(err) => {
-                        // TODO - remove. Make the test's non-determinism more apparent by
-                        // only sometimes formatting this stack trace.
-                        if Instant::now().duration_since(t0).as_nanos() % 2 == 0 {
-                            log::error!("connection error: {:?}", err);
-                        }
-
+                        log::error!("connection error: {:?}", err);
                         this.set_status(Status::ConnectionLost, &cx);
                     }
                 }
diff --git a/crates/collab/src/tests/randomized_integration_tests.rs b/crates/collab/src/tests/randomized_integration_tests.rs
index 6b6166ec0886cbe164d304720fd2558926241759..1deaafcba2fb162e12e752b47a297032314066ca 100644
--- a/crates/collab/src/tests/randomized_integration_tests.rs
+++ b/crates/collab/src/tests/randomized_integration_tests.rs
@@ -17,7 +17,7 @@ use project::{search::SearchQuery, Project};
 use rand::prelude::*;
 use std::{env, path::PathBuf, sync::Arc};
 
-#[gpui::test(iterations = 100, detect_nondeterminism = true)]
+#[gpui::test(iterations = 100)]
 async fn test_random_collaboration(
     cx: &mut TestAppContext,
     deterministic: Arc<Deterministic>,
diff --git a/crates/gpui/src/executor.rs b/crates/gpui/src/executor.rs
index a78a8c4b2eca1d3c7465a182363d6cf3bc8d3ac7..faf2a9729f2d1e71dead5176a9a9a578d325d9d0 100644
--- a/crates/gpui/src/executor.rs
+++ b/crates/gpui/src/executor.rs
@@ -198,9 +198,7 @@ impl Deterministic {
         let unparker = self.parker.lock().unparker();
         let (runnable, task) = async_task::spawn_local(future, move |runnable| {
             let mut state = state.lock();
-            state
-                .poll_history
-                .push(ExecutorEvent::EnqueuRunnable { id });
+            state.push_to_history(ExecutorEvent::EnqueuRunnable { id });
             state
                 .scheduled_from_foreground
                 .entry(cx_id)
diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs
index ae91478f89eb4a713cc0bf992f03c8d69d123c9b..4b8266120a3682e118c497c3d4ba3cea7cd9e2df 100644
--- a/crates/text/src/tests.rs
+++ b/crates/text/src/tests.rs
@@ -45,7 +45,7 @@ fn test_random_edits(mut rng: StdRng) {
     let mut buffer = Buffer::new(0, 0, reference_string.clone());
     LineEnding::normalize(&mut reference_string);
 
-    buffer.history.group_interval = Duration::from_millis(rng.gen_range(0..=200));
+    buffer.set_group_interval(Duration::from_millis(rng.gen_range(0..=200)));
     let mut buffer_versions = Vec::new();
     log::info!(
         "buffer text {:?}, version: {:?}",
@@ -488,7 +488,7 @@ fn test_anchors_at_start_and_end() {
 fn test_undo_redo() {
     let mut buffer = Buffer::new(0, 0, "1234".into());
     // Set group interval to zero so as to not group edits in the undo stack.
-    buffer.history.group_interval = Duration::from_secs(0);
+    buffer.set_group_interval(Duration::from_secs(0));
 
     buffer.edit([(1..1, "abx")]);
     buffer.edit([(3..4, "yzef")]);
@@ -524,6 +524,7 @@ fn test_undo_redo() {
 fn test_history() {
     let mut now = Instant::now();
     let mut buffer = Buffer::new(0, 0, "123456".into());
+    buffer.set_group_interval(Duration::from_millis(300));
 
     let transaction_1 = buffer.start_transaction_at(now).unwrap();
     buffer.edit([(2..4, "cd")]);
@@ -535,7 +536,7 @@ fn test_history() {
     buffer.end_transaction_at(now).unwrap();
     assert_eq!(buffer.text(), "12cde6");
 
-    now += buffer.history.group_interval + Duration::from_millis(1);
+    now += buffer.transaction_group_interval() + Duration::from_millis(1);
     buffer.start_transaction_at(now);
     buffer.edit([(0..1, "a")]);
     buffer.edit([(1..1, "b")]);
diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs
index 5aa91ede8aa61e61104c21b7fb671c33d8bdcd21..914023f305b4ba27aee8359205f3befc5245503a 100644
--- a/crates/text/src/text.rs
+++ b/crates/text/src/text.rs
@@ -115,6 +115,10 @@ impl History {
             undo_stack: Vec::new(),
             redo_stack: Vec::new(),
             transaction_depth: 0,
+            // Don't group transactions in tests unless we opt in, because it's a footgun.
+            #[cfg(any(test, feature = "test-support"))]
+            group_interval: Duration::ZERO,
+            #[cfg(not(any(test, feature = "test-support")))]
             group_interval: Duration::from_millis(300),
         }
     }