From ebf2f56a25c75041e5d98e84b859bd08b405033a Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Thu, 16 Apr 2026 10:37:05 -0400 Subject: [PATCH] gpui: Make property tests deterministic by default (#54004) ### Summary `#[gpui::property_test]` was non-deterministic because it always used a random seed with proptest. So it generated different test cases and scheduler seeds on every run, causing flaky failures. `#[gpui::test]` doesn't have this problem because it defaults to fixed seeds. This changes `#[gpui::property_test]` to match: proptest's RNG seed now defaults to `0` instead of random, so the same cases run every time. The`$SEED` env var overrides both the scheduler seed and case generation seed. Self-Review Checklist: - [ ] I've reviewed my own diff for quality, security, and reliability - [ ] Unsafe blocks (if any) have justifying comments - [ ] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [ ] Tests cover the new/changed behavior - [ ] Performance impact has been considered and is acceptable Closes #ISSUE Release Notes: - N/A or Added/Fixed/Improved ... --------- Co-authored-by: cameron --- crates/gpui/src/test.rs | 21 +++++++ crates/gpui_macros/src/property_test.rs | 75 ++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/crates/gpui/src/test.rs b/crates/gpui/src/test.rs index ddcc3d27bd04d2fd82b3367a2fee6930e86ef356..2ee45f899c3941d8d0e0f87bc2ecdada83404ff2 100644 --- a/crates/gpui/src/test.rs +++ b/crates/gpui/src/test.rs @@ -37,6 +37,9 @@ use std::{ /// Strategy injected into `#[gpui::property_test]` tests to control the seed /// given to the scheduler. Doesn't shrink, since all scheduler seeds are /// equivalent in complexity. If `$SEED` is set, it always uses that value. +/// +/// Note: this function is not intended to be used directly. Rather, it is +/// public so that it can be used from the `property_test` macro. pub fn seed_strategy() -> impl Strategy { match std::env::var("SEED") { Ok(val) => Just(val.parse().unwrap()).boxed(), @@ -44,6 +47,24 @@ pub fn seed_strategy() -> impl Strategy { } } +/// Applies a fixed RNG seed to a proptest config so that case generation +/// is deterministic. Uses `$SEED` if set, otherwise defaults to `0`. +/// This bridges the GPUI `SEED` env var to proptest's RNG seed, so that +/// a single variable controls both the scheduler seed and case generation. +/// +/// Note: this function is not intended to be used directly. Rather, it is +/// public so that it can be used from the `property_test` macro. +pub fn apply_seed_to_proptest_config( + mut config: proptest::test_runner::Config, +) -> proptest::test_runner::Config { + let seed = env::var("SEED") + .ok() + .and_then(|val| val.parse::().ok()) + .unwrap_or(0); + config.rng_seed = proptest::test_runner::RngSeed::Fixed(seed); + config +} + /// Similar to [`run_test`], but only runs the callback once, allowing /// [`FnOnce`] callbacks. This is intended for use with the /// `gpui::property_test` macro and generally should not be used directly. diff --git a/crates/gpui_macros/src/property_test.rs b/crates/gpui_macros/src/property_test.rs index 65355b4dca3152b7fc315906749d1298c43f349b..2c7ed0ee0dc1389f53fb762a4c9ee7b8c489a18b 100644 --- a/crates/gpui_macros/src/property_test.rs +++ b/crates/gpui_macros/src/property_test.rs @@ -1,7 +1,12 @@ use proc_macro2::TokenStream; -use quote::{format_ident, quote, quote_spanned}; +use quote::{ToTokens, format_ident, quote, quote_spanned}; use syn::{ - FnArg, Ident, ItemFn, Type, parse2, punctuated::Punctuated, spanned::Spanned, token::Comma, + Expr, FnArg, Ident, ItemFn, MetaNameValue, Token, Type, + parse::{Parse, ParseStream}, + parse2, + punctuated::Punctuated, + spanned::Spanned, + token::Comma, }; pub fn test(args: TokenStream, item: TokenStream) -> TokenStream { @@ -12,6 +17,11 @@ pub fn test(args: TokenStream, item: TokenStream) -> TokenStream { }; }; + let args = match parse2::(args) { + Ok(args) => args, + Err(e) => return e.to_compile_error(), + }; + let test_name = func.sig.ident.clone(); let inner_fn_name = format_ident!("__{test_name}"); let outer_fn_attributes = &func.attrs; @@ -51,10 +61,12 @@ pub fn test(args: TokenStream, item: TokenStream) -> TokenStream { }, }; + let fixed_macro_invocation = args.render(); + quote! { #arg_errors - #[::gpui::proptest::property_test(proptest_path = "::gpui::proptest", #args)] + #fixed_macro_invocation #(#outer_fn_attributes)* fn #test_name(#proptest_args) { #inner_fn @@ -69,6 +81,63 @@ pub fn test(args: TokenStream, item: TokenStream) -> TokenStream { } } +struct Args { + config: Option, + remaining_args: Vec, + errors: TokenStream, +} + +impl Args { + /// By default, proptest uses random seeds unless `$PROPTEST_SEED` is set. + /// Rather than managing both `$SEED` and `$PROPTEST_SEED`, we intercept + /// `config = ...` tokens and add a call to `gpui::apply_seed_to_config`. + fn render(&self) -> TokenStream { + let user_provided_config = match &self.config { + None => quote! { ::gpui::proptest::prelude::ProptestConfig::default() }, + Some(config) => config.into_token_stream(), + }; + + let fixed_config = quote!(::gpui::apply_seed_to_proptest_config(#user_provided_config)); + let remaining_args = &self.remaining_args; + let errors = &self.errors; + + quote! { + #errors + #[::gpui::proptest::property_test( + proptest_path = "::gpui::proptest", + config = #fixed_config, + #(#remaining_args,)* + )] + } + } +} + +impl Parse for Args { + fn parse(input: ParseStream) -> syn::Result { + let pairs = Punctuated::::parse_terminated(input)?; + + let mut config = None; + let mut remaining_args = vec![]; + let mut errors = quote!(); + + for pair in pairs { + match pair.path.get_ident().map(Ident::to_string).as_deref() { + Some("config") => config = Some(pair.value), + Some("proptest_path") => errors.extend(quote_spanned! {pair.span() => + compile_error!("`gpui::property_test` overrides the `proptest_path` parameter") + }), + _ => remaining_args.push(pair), + } + } + + Ok(Self { + config, + remaining_args, + errors, + }) + } +} + #[derive(Default)] struct ParsedArgs { cx_vars: TokenStream,