Merge pull request #96 from zed-industries/retry-flaky-tests

Nathan Sobo created

Add a "retries" option to gpui::test macro and use it in flaky tests

Change summary

Cargo.lock                     | 10 ++-
gpui/Cargo.toml                |  2 
gpui/build.rs                  |  6 +-
gpui/src/platform/mac/fonts.rs | 20 +++++--
gpui_macros/src/lib.rs         | 90 ++++++++++++++++++++++++++++++++---
zed/src/file_finder.rs         |  2 
6 files changed, 105 insertions(+), 25 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -1,5 +1,7 @@
 # This file is automatically @generated by Cargo.
 # It is not intended for manual editing.
+version = 3
+
 [[package]]
 name = "addr2line"
 version = "0.14.1"
@@ -221,9 +223,9 @@ checksum = "904dfeac50f3cdaba28fc6f57fdcddb75f49ed61346676a78c4ffe55877802fd"
 
 [[package]]
 name = "bindgen"
-version = "0.57.0"
+version = "0.58.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "fd4865004a46a0aafb2a0a5eb19d3c9fc46ee5f063a6cfc605c69ac9ecf5263d"
+checksum = "0f8523b410d7187a43085e7e064416ea32ded16bd0a4e6fc025e21616d01258f"
 dependencies = [
  "bitflags 1.2.1",
  "cexpr",
@@ -2428,9 +2430,9 @@ checksum = "cc30b1e1e8c40c121ca33b86c23308a090d19974ef001b4bf6e61fd1a0fb095c"
 
 [[package]]
 name = "shlex"
-version = "0.1.1"
+version = "1.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7fdf1b9db47230893d76faad238fd6097fd6d6a9245cd7a4d90dbd639536bbd2"
+checksum = "42a568c8f2cd051a4d283bd6eb0343ac214c1b0f1ac19f93e1175b2dee38c73d"
 
 [[package]]
 name = "signal-hook"

gpui/Cargo.toml 🔗

@@ -30,7 +30,7 @@ tree-sitter = "0.19"
 usvg = "0.14"
 
 [build-dependencies]
-bindgen = "0.57"
+bindgen = "0.58.1"
 cc = "1.0.67"
 
 [dev-dependencies]

gpui/build.rs 🔗

@@ -17,8 +17,8 @@ fn generate_dispatch_bindings() {
 
     let bindings = bindgen::Builder::default()
         .header("src/platform/mac/dispatch.h")
-        .whitelist_var("_dispatch_main_q")
-        .whitelist_function("dispatch_async_f")
+        .allowlist_var("_dispatch_main_q")
+        .allowlist_function("dispatch_async_f")
         .parse_callbacks(Box::new(bindgen::CargoCallbacks))
         .generate()
         .expect("unable to generate bindings");
@@ -94,7 +94,7 @@ fn compile_metal_shaders() {
 fn generate_shader_bindings() {
     let bindings = bindgen::Builder::default()
         .header(SHADER_HEADER_PATH)
-        .whitelist_type("GPUI.*")
+        .allowlist_type("GPUI.*")
         .parse_callbacks(Box::new(bindgen::CargoCallbacks))
         .generate()
         .expect("unable to generate bindings");

gpui/src/platform/mac/fonts.rs 🔗

@@ -311,17 +311,24 @@ impl FontSystemState {
 
 #[cfg(test)]
 mod tests {
+    use crate::MutableAppContext;
+
     use super::*;
     use font_kit::properties::{Style, Weight};
     use platform::FontSystem as _;
 
-    #[test]
-    fn test_layout_str() -> anyhow::Result<()> {
+    #[crate::test(self, retries = 5)]
+    fn test_layout_str(_: &mut MutableAppContext) {
+        // This is failing intermittently on CI and we don't have time to figure it out
         let fonts = FontSystem::new();
-        let menlo = fonts.load_family("Menlo")?;
-        let menlo_regular = fonts.select_font(&menlo, &Properties::new())?;
-        let menlo_italic = fonts.select_font(&menlo, &Properties::new().style(Style::Italic))?;
-        let menlo_bold = fonts.select_font(&menlo, &Properties::new().weight(Weight::BOLD))?;
+        let menlo = fonts.load_family("Menlo").unwrap();
+        let menlo_regular = fonts.select_font(&menlo, &Properties::new()).unwrap();
+        let menlo_italic = fonts
+            .select_font(&menlo, &Properties::new().style(Style::Italic))
+            .unwrap();
+        let menlo_bold = fonts
+            .select_font(&menlo, &Properties::new().weight(Weight::BOLD))
+            .unwrap();
         assert_ne!(menlo_regular, menlo_italic);
         assert_ne!(menlo_regular, menlo_bold);
         assert_ne!(menlo_italic, menlo_bold);
@@ -342,7 +349,6 @@ mod tests {
         assert_eq!(line.runs[1].glyphs.len(), 4);
         assert_eq!(line.runs[2].font_id, menlo_regular);
         assert_eq!(line.runs[2].glyphs.len(), 5);
-        Ok(())
     }
 
     #[test]

gpui_macros/src/lib.rs 🔗

@@ -1,14 +1,16 @@
-use std::mem;
-
 use proc_macro::TokenStream;
 use quote::{format_ident, quote};
-use syn::{parse_macro_input, parse_quote, AttributeArgs, ItemFn, Meta, NestedMeta};
+use std::mem;
+use syn::{
+    parse_macro_input, parse_quote, AttributeArgs, ItemFn, Lit, Meta, MetaNameValue, NestedMeta,
+};
 
 #[proc_macro_attribute]
 pub fn test(args: TokenStream, function: TokenStream) -> TokenStream {
     let mut namespace = format_ident!("gpui");
 
     let args = syn::parse_macro_input!(args as AttributeArgs);
+    let mut max_retries = 0;
     for arg in args {
         match arg {
             NestedMeta::Meta(Meta::Path(name))
@@ -16,6 +18,14 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream {
             {
                 namespace = format_ident!("crate");
             }
+            NestedMeta::Meta(Meta::NameValue(meta)) => {
+                if let Some(result) = parse_retries(&meta) {
+                    match result {
+                        Ok(retries) => max_retries = retries,
+                        Err(error) => return TokenStream::from(error.into_compile_error()),
+                    }
+                }
+            }
             other => {
                 return TokenStream::from(
                     syn::Error::new_spanned(other, "invalid argument").into_compile_error(),
@@ -34,9 +44,32 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream {
             fn #outer_fn_name() {
                 #inner_fn
 
-                #namespace::App::test_async((), move |cx| async {
-                    #inner_fn_name(cx).await;
-                });
+                if #max_retries > 0 {
+                    let mut retries = 0;
+                    loop {
+                        let result = std::panic::catch_unwind(|| {
+                            #namespace::App::test_async((), move |cx| async {
+                                #inner_fn_name(cx).await;
+                            });
+                        });
+
+                        match result {
+                            Ok(result) => return result,
+                            Err(error) => {
+                                if retries < #max_retries {
+                                    retries += 1;
+                                    println!("retrying: attempt {}", retries);
+                                } else {
+                                    std::panic::resume_unwind(error);
+                                }
+                            }
+                        }
+                    }
+                } else {
+                    #namespace::App::test_async((), move |cx| async {
+                        #inner_fn_name(cx).await;
+                    });
+                }
             }
         }
     } else {
@@ -45,9 +78,32 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream {
             fn #outer_fn_name() {
                 #inner_fn
 
-                #namespace::App::test((), |cx| {
-                    #inner_fn_name(cx);
-                });
+                if #max_retries > 0 {
+                    let mut retries = 0;
+                    loop {
+                        let result = std::panic::catch_unwind(|| {
+                            #namespace::App::test((), |cx| {
+                                #inner_fn_name(cx);
+                            });
+                        });
+
+                        match result {
+                            Ok(result) => return result,
+                            Err(error) => {
+                                if retries < #max_retries {
+                                    retries += 1;
+                                    println!("retrying: attempt {}", retries);
+                                } else {
+                                    std::panic::resume_unwind(error);
+                                }
+                            }
+                        }
+                    }
+                } else {
+                    #namespace::App::test((), |cx| {
+                        #inner_fn_name(cx);
+                    });
+                }
             }
         }
     };
@@ -55,3 +111,19 @@ pub fn test(args: TokenStream, function: TokenStream) -> TokenStream {
 
     TokenStream::from(quote!(#outer_fn))
 }
+
+fn parse_retries(meta: &MetaNameValue) -> Option<syn::Result<usize>> {
+    let ident = meta.path.get_ident();
+    if ident.map_or(false, |n| n == "retries") {
+        if let Lit::Int(int) = &meta.lit {
+            Some(int.base10_parse())
+        } else {
+            Some(Err(syn::Error::new(
+                meta.lit.span(),
+                "retries mut be an integer",
+            )))
+        }
+    } else {
+        None
+    }
+}

zed/src/file_finder.rs 🔗

@@ -651,7 +651,7 @@ mod tests {
         finder.read_with(&cx, |f, _| assert_eq!(f.matches.len(), 0));
     }
 
-    #[gpui::test]
+    #[gpui::test(retries = 5)]
     async fn test_multiple_matches_with_same_relative_path(mut cx: gpui::TestAppContext) {
         let tmp_dir = temp_tree(json!({
             "dir1": { "a.txt": "" },