Introduce a new `detect_nondeterminism = true` attribute to `gpui::test`

Antonio Scandurra created

Change summary

crates/gpui/src/executor.rs           | 72 ++++++++++++++++++++++--
crates/gpui/src/test.rs               | 84 +++++++++++++++++++++++-----
crates/gpui_macros/src/gpui_macros.rs | 27 ++++++---
3 files changed, 150 insertions(+), 33 deletions(-)

Detailed changes

crates/gpui/src/executor.rs 🔗

@@ -66,21 +66,31 @@ struct DeterministicState {
     rng: rand::prelude::StdRng,
     seed: u64,
     scheduled_from_foreground: collections::HashMap<usize, Vec<ForegroundRunnable>>,
-    scheduled_from_background: Vec<Runnable>,
+    scheduled_from_background: Vec<BackgroundRunnable>,
     forbid_parking: bool,
     block_on_ticks: std::ops::RangeInclusive<usize>,
     now: std::time::Instant,
     next_timer_id: usize,
     pending_timers: Vec<(usize, std::time::Instant, postage::barrier::Sender)>,
     waiting_backtrace: Option<backtrace::Backtrace>,
+    next_runnable_id: usize,
+    poll_history: Vec<usize>,
+    runnable_backtraces: collections::HashMap<usize, backtrace::Backtrace>,
 }
 
 #[cfg(any(test, feature = "test-support"))]
 struct ForegroundRunnable {
+    id: usize,
     runnable: Runnable,
     main: bool,
 }
 
+#[cfg(any(test, feature = "test-support"))]
+struct BackgroundRunnable {
+    id: usize,
+    runnable: Runnable,
+}
+
 #[cfg(any(test, feature = "test-support"))]
 pub struct Deterministic {
     state: Arc<parking_lot::Mutex<DeterministicState>>,
@@ -117,11 +127,24 @@ impl Deterministic {
                 next_timer_id: Default::default(),
                 pending_timers: Default::default(),
                 waiting_backtrace: None,
+                next_runnable_id: 0,
+                poll_history: Default::default(),
+                runnable_backtraces: Default::default(),
             })),
             parker: Default::default(),
         })
     }
 
+    pub fn runnable_history(&self) -> Vec<usize> {
+        self.state.lock().poll_history.clone()
+    }
+
+    pub fn runnable_backtrace(&self, runnable_id: usize) -> backtrace::Backtrace {
+        let mut backtrace = self.state.lock().runnable_backtraces[&runnable_id].clone();
+        backtrace.resolve();
+        backtrace
+    }
+
     pub fn build_background(self: &Arc<Self>) -> Arc<Background> {
         Arc::new(Background::Deterministic {
             executor: self.clone(),
@@ -142,6 +165,15 @@ impl Deterministic {
         main: bool,
     ) -> AnyLocalTask {
         let state = self.state.clone();
+        let id;
+        {
+            let mut state = state.lock();
+            id = util::post_inc(&mut state.next_runnable_id);
+            state
+                .runnable_backtraces
+                .insert(id, backtrace::Backtrace::new_unresolved());
+        }
+
         let unparker = self.parker.lock().unparker();
         let (runnable, task) = async_task::spawn_local(future, move |runnable| {
             let mut state = state.lock();
@@ -149,7 +181,7 @@ impl Deterministic {
                 .scheduled_from_foreground
                 .entry(cx_id)
                 .or_default()
-                .push(ForegroundRunnable { runnable, main });
+                .push(ForegroundRunnable { id, runnable, main });
             unparker.unpark();
         });
         runnable.schedule();
@@ -158,10 +190,21 @@ impl Deterministic {
 
     fn spawn(&self, future: AnyFuture) -> AnyTask {
         let state = self.state.clone();
+        let id;
+        {
+            let mut state = state.lock();
+            id = util::post_inc(&mut state.next_runnable_id);
+            state
+                .runnable_backtraces
+                .insert(id, backtrace::Backtrace::new_unresolved());
+        }
+
         let unparker = self.parker.lock().unparker();
         let (runnable, task) = async_task::spawn(future, move |runnable| {
             let mut state = state.lock();
-            state.scheduled_from_background.push(runnable);
+            state
+                .scheduled_from_background
+                .push(BackgroundRunnable { id, runnable });
             unparker.unpark();
         });
         runnable.schedule();
@@ -178,15 +221,25 @@ impl Deterministic {
         let woken = Arc::new(AtomicBool::new(false));
 
         let state = self.state.clone();
+        let id;
+        {
+            let mut state = state.lock();
+            id = util::post_inc(&mut state.next_runnable_id);
+            state
+                .runnable_backtraces
+                .insert(id, backtrace::Backtrace::new());
+        }
+
         let unparker = self.parker.lock().unparker();
         let (runnable, mut main_task) = unsafe {
             async_task::spawn_unchecked(main_future, move |runnable| {
-                let mut state = state.lock();
+                let state = &mut *state.lock();
                 state
                     .scheduled_from_foreground
                     .entry(cx_id)
                     .or_default()
                     .push(ForegroundRunnable {
+                        id: util::post_inc(&mut state.next_runnable_id),
                         runnable,
                         main: true,
                     });
@@ -248,9 +301,10 @@ impl Deterministic {
             if !state.scheduled_from_background.is_empty() && state.rng.gen() {
                 let background_len = state.scheduled_from_background.len();
                 let ix = state.rng.gen_range(0..background_len);
-                let runnable = state.scheduled_from_background.remove(ix);
+                let background_runnable = state.scheduled_from_background.remove(ix);
+                state.poll_history.push(background_runnable.id);
                 drop(state);
-                runnable.run();
+                background_runnable.runnable.run();
             } else if !state.scheduled_from_foreground.is_empty() {
                 let available_cx_ids = state
                     .scheduled_from_foreground
@@ -266,6 +320,7 @@ impl Deterministic {
                 if scheduled_from_cx.is_empty() {
                     state.scheduled_from_foreground.remove(&cx_id_to_run);
                 }
+                state.poll_history.push(foreground_runnable.id);
 
                 drop(state);
 
@@ -298,9 +353,10 @@ impl Deterministic {
             let runnable_count = state.scheduled_from_background.len();
             let ix = state.rng.gen_range(0..=runnable_count);
             if ix < state.scheduled_from_background.len() {
-                let runnable = state.scheduled_from_background.remove(ix);
+                let background_runnable = state.scheduled_from_background.remove(ix);
+                state.poll_history.push(background_runnable.id);
                 drop(state);
-                runnable.run();
+                background_runnable.runnable.run();
             } else {
                 drop(state);
                 if let Poll::Ready(result) = future.poll(&mut cx) {

crates/gpui/src/test.rs 🔗

@@ -1,11 +1,13 @@
 use crate::{
-    elements::Empty, executor, platform, Element, ElementBox, Entity, FontCache, Handle,
-    LeakDetector, MutableAppContext, Platform, RenderContext, Subscription, TestAppContext, View,
+    elements::Empty, executor, platform, util::CwdBacktrace, Element, ElementBox, Entity,
+    FontCache, Handle, LeakDetector, MutableAppContext, Platform, RenderContext, Subscription,
+    TestAppContext, View,
 };
 use futures::StreamExt;
 use parking_lot::Mutex;
 use smol::channel;
 use std::{
+    fmt::Write,
     panic::{self, RefUnwindSafe},
     rc::Rc,
     sync::{
@@ -29,13 +31,13 @@ pub fn run_test(
     mut num_iterations: u64,
     mut starting_seed: u64,
     max_retries: usize,
+    detect_nondeterminism: bool,
     test_fn: &mut (dyn RefUnwindSafe
               + Fn(
         &mut MutableAppContext,
         Rc<platform::test::ForegroundPlatform>,
         Arc<executor::Deterministic>,
         u64,
-        bool,
     )),
     fn_name: String,
 ) {
@@ -60,10 +62,10 @@ pub fn run_test(
             let platform = Arc::new(platform::test::platform());
             let font_system = platform.fonts();
             let font_cache = Arc::new(FontCache::new(font_system));
+            let mut prev_runnable_history: Option<Vec<usize>> = None;
 
-            loop {
-                let seed = atomic_seed.fetch_add(1, SeqCst);
-                let is_last_iteration = seed + 1 >= starting_seed + num_iterations;
+            for _ in 0..num_iterations {
+                let seed = atomic_seed.load(SeqCst);
 
                 if is_randomized {
                     dbg!(seed);
@@ -82,13 +84,7 @@ pub fn run_test(
                     fn_name.clone(),
                 );
                 cx.update(|cx| {
-                    test_fn(
-                        cx,
-                        foreground_platform.clone(),
-                        deterministic.clone(),
-                        seed,
-                        is_last_iteration,
-                    );
+                    test_fn(cx, foreground_platform.clone(), deterministic.clone(), seed);
                 });
 
                 cx.update(|cx| cx.remove_all_windows());
@@ -96,8 +92,64 @@ pub fn run_test(
                 cx.update(|cx| cx.clear_globals());
 
                 leak_detector.lock().detect();
-                if is_last_iteration {
-                    break;
+
+                if detect_nondeterminism {
+                    let curr_runnable_history = deterministic.runnable_history();
+                    if let Some(prev_runnable_history) = prev_runnable_history {
+                        let mut prev_entries = prev_runnable_history.iter().fuse();
+                        let mut curr_entries = curr_runnable_history.iter().fuse();
+
+                        let mut nondeterministic = false;
+                        let mut common_history_prefix = Vec::new();
+                        let mut prev_history_suffix = Vec::new();
+                        let mut curr_history_suffix = Vec::new();
+                        loop {
+                            match (prev_entries.next(), curr_entries.next()) {
+                                (None, None) => break,
+                                (None, Some(curr_id)) => curr_history_suffix.push(*curr_id),
+                                (Some(prev_id), None) => prev_history_suffix.push(*prev_id),
+                                (Some(prev_id), Some(curr_id)) => {
+                                    if nondeterministic {
+                                        prev_history_suffix.push(*prev_id);
+                                        curr_history_suffix.push(*curr_id);
+                                    } else if prev_id == curr_id {
+                                        common_history_prefix.push(*curr_id);
+                                    } else {
+                                        nondeterministic = true;
+                                        prev_history_suffix.push(*prev_id);
+                                        curr_history_suffix.push(*curr_id);
+                                    }
+                                }
+                            }
+                        }
+
+                        if nondeterministic {
+                            let mut error = String::new();
+                            writeln!(&mut error, "Common prefix: {:?}", common_history_prefix)
+                                .unwrap();
+                            writeln!(&mut error, "Previous suffix: {:?}", prev_history_suffix)
+                                .unwrap();
+                            writeln!(&mut error, "Current suffix: {:?}", curr_history_suffix)
+                                .unwrap();
+
+                            let last_common_backtrace = common_history_prefix
+                                .last()
+                                .map(|runnable_id| deterministic.runnable_backtrace(*runnable_id));
+
+                            writeln!(
+                                &mut error,
+                                "Last future that ran on both executions: {:?}",
+                                last_common_backtrace.as_ref().map(CwdBacktrace)
+                            )
+                            .unwrap();
+                            panic!("Detected non-determinism.\n{}", error);
+                        }
+                    }
+                    prev_runnable_history = Some(curr_runnable_history);
+                }
+
+                if !detect_nondeterminism {
+                    atomic_seed.fetch_add(1, SeqCst);
                 }
             }
         });
@@ -112,7 +164,7 @@ pub fn run_test(
                     println!("retrying: attempt {}", retries);
                 } else {
                     if is_randomized {
-                        eprintln!("failing seed: {}", atomic_seed.load(SeqCst) - 1);
+                        eprintln!("failing seed: {}", atomic_seed.load(SeqCst));
                     }
                     panic::resume_unwind(error);
                 }

crates/gpui_macros/src/gpui_macros.rs 🔗

@@ -14,6 +14,7 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream {
     let mut max_retries = 0;
     let mut num_iterations = 1;
     let mut starting_seed = 0;
+    let mut detect_nondeterminism = false;
 
     for arg in args {
         match arg {
@@ -26,6 +27,9 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream {
                 let key_name = meta.path.get_ident().map(|i| i.to_string());
                 let result = (|| {
                     match key_name.as_deref() {
+                        Some("detect_nondeterminism") => {
+                            detect_nondeterminism = parse_bool(&meta.lit)?
+                        }
                         Some("retries") => max_retries = parse_int(&meta.lit)?,
                         Some("iterations") => num_iterations = parse_int(&meta.lit)?,
                         Some("seed") => starting_seed = parse_int(&meta.lit)?,
@@ -77,10 +81,6 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream {
                             inner_fn_args.extend(quote!(rand::SeedableRng::seed_from_u64(seed),));
                             continue;
                         }
-                        Some("bool") => {
-                            inner_fn_args.extend(quote!(is_last_iteration,));
-                            continue;
-                        }
                         Some("Arc") => {
                             if let syn::PathArguments::AngleBracketed(args) =
                                 &last_segment.unwrap().arguments
@@ -146,7 +146,8 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream {
                     #num_iterations as u64,
                     #starting_seed as u64,
                     #max_retries,
-                    &mut |cx, foreground_platform, deterministic, seed, is_last_iteration| {
+                    #detect_nondeterminism,
+                    &mut |cx, foreground_platform, deterministic, seed| {
                         #cx_vars
                         cx.foreground().run(#inner_fn_name(#inner_fn_args));
                         #cx_teardowns
@@ -165,9 +166,6 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream {
                         Some("StdRng") => {
                             inner_fn_args.extend(quote!(rand::SeedableRng::seed_from_u64(seed),));
                         }
-                        Some("bool") => {
-                            inner_fn_args.extend(quote!(is_last_iteration,));
-                        }
                         _ => {}
                     }
                 } else {
@@ -189,7 +187,8 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream {
                     #num_iterations as u64,
                     #starting_seed as u64,
                     #max_retries,
-                    &mut |cx, _, _, seed, is_last_iteration| #inner_fn_name(#inner_fn_args),
+                    #detect_nondeterminism,
+                    &mut |cx, _, _, seed| #inner_fn_name(#inner_fn_args),
                     stringify!(#outer_fn_name).to_string(),
                 );
             }
@@ -209,3 +208,13 @@ fn parse_int(literal: &Lit) -> Result<usize, TokenStream> {
 
     result.map_err(|err| TokenStream::from(err.into_compile_error()))
 }
+
+fn parse_bool(literal: &Lit) -> Result<bool, TokenStream> {
+    let result = if let Lit::Bool(result) = &literal {
+        Ok(result.value)
+    } else {
+        Err(syn::Error::new(literal.span(), "must be a boolean"))
+    };
+
+    result.map_err(|err| TokenStream::from(err.into_compile_error()))
+}