chore(uptime): Make redis use in tests safer. (#205)

This generalizes our Redis test lock so that we can easily apply it to
other tests. Also flushes the redis db when we're done to avoid test
pollution.

---------

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>

Author: wedamija

Cargo.lock

@@ -43,7 +43,7 @@ tokio-rustls = "0.26.1"
 rustls = "0.23.21"
 rcgen = "0.11"
 time = "0.3"
-
+ctor = "0.2"
 [patch.crates-io]
 rdkafka = { git = "https://github.com/fede1024/rust-rdkafka" }
 reqwest = { git = "https://github.com/getsentry/reqwest", branch = "restricted-connector" }
@@ -53,3 +53,6 @@ lto = true
 codegen-units = 1
 panic = 'abort'
 strip = "debuginfo"
+
+[dev-dependencies]
+redis-test-macro = { path = "./redis-test-macro" }

Cargo.toml

@@ -16,9 +16,9 @@ WORKDIR /app
 # Just copy the Cargo.toml files and trigger a build so that we compile our
 # dependencies only. This way we avoid layer cache invalidation if our
 # dependencies haven't changed, resulting in faster builds.
+COPY Cargo.toml Cargo.lock ./
+COPY redis-test-macro redis-test-macro/
 
-COPY Cargo.toml .
-COPY Cargo.lock .
 ENV RUSTFLAGS="-Ctarget-feature=-crt-static"
 ENV PKG_CONFIG_ALLOW_CROSS=1
 RUN cargo build --release && rm -rf src/

Dockerfile

@@ -0,0 +1,11 @@
+[package]
+name = "redis-test-macro"
+version = "0.1.0"
+edition = "2021"
+
+[lib]
+proc-macro = true
+
+[dependencies]
+syn = { version = "2.0", features = ["full"] }
+quote = "1.0"

redis-test-macro/Cargo.toml

@@ -0,0 +1,43 @@
+use proc_macro::TokenStream;
+use quote::quote;
+use syn::{parse_macro_input, ItemFn, parse::Parse, parse::ParseStream, Token, LitBool, Ident};
+
+struct TestArgs {
+    start_paused: bool,
+}
+
+impl Parse for TestArgs {
+    fn parse(input: ParseStream) -> syn::Result<Self> {
+        let mut start_paused = false;
+        
+        if !input.is_empty() {
+            let ident: Ident = input.parse()?;
+            if ident != "start_paused" {
+                return Err(syn::Error::new(ident.span(), "expected `start_paused`"));
+            }
+            input.parse::<Token![=]>()?;
+            start_paused = input.parse::<LitBool>()?.value;
+        }
+        
+        Ok(TestArgs { start_paused })
+    }
+}
+
+#[proc_macro_attribute]
+pub fn redis_test(attr: TokenStream, item: TokenStream) -> TokenStream {
+    let args = parse_macro_input!(attr as TestArgs);
+    let input = parse_macro_input!(item as ItemFn);
+    let fn_name = &input.sig.ident;
+    let body = &input.block;
+    let start_paused = args.start_paused;
+
+    let expanded = quote! {
+        #[tokio::test(start_paused = #start_paused)]
+        async fn #fn_name() {
+            let _guard = crate::test_utils::sync::TEST_MUTEX.lock().await;
+            #body
+        }
+    };
+
+    TokenStream::from(expanded)
+} 

redis-test-macro/src/lib.rs

@@ -274,15 +274,15 @@ mod tests {
     use httpmock::Method;
 
     use sentry::protocol::SpanId;
-    use tokio::io::AsyncWriteExt;
-    use tokio::net::TcpListener;
     use uuid::Uuid;
     #[cfg(target_os = "linux")]
     use {
         rcgen::{Certificate, CertificateParams},
         rustls::pki_types::{CertificateDer, PrivateKeyDer, PrivatePkcs8KeyDer},
         rustls::ServerConfig,
         std::sync::Arc,
+        tokio::io::AsyncWriteExt,
+        tokio::net::TcpListener,
         tokio_rustls::TlsAcceptor,
     };
 

src/checker/http_checker.rs

@@ -11,6 +11,7 @@ mod manager;
 mod metrics;
 mod producer;
 mod scheduler;
+mod test_utils;
 mod types;
 
 use std::process;
@@ -30,3 +31,18 @@ pub fn main() {
     Hub::current().client().map(|x| x.close(None));
     process::exit(exit_code);
 }
+
+#[cfg(test)]
+mod test {
+    use crate::app::config::Config;
+    use redis::Client;
+
+    #[ctor::dtor]
+    fn cleanup() {
+        // Flushes Redis back to the default state to avoid state crossover between runs.
+        let config = Config::default();
+        let client = Client::open(config.redis_host).unwrap();
+        let mut conn = client.get_connection().unwrap();
+        let _: () = redis::cmd("FLUSHDB").query(&mut conn).unwrap();
+    }
+}

src/main.rs

@@ -170,9 +170,9 @@ mod tests {
     use crate::app::config::Config;
     use chrono::{Duration, Utc};
     use redis::{Client, Commands};
+    use redis_test_macro::redis_test;
     use similar_asserts::assert_eq;
     use std::sync::Arc;
-    use tokio::sync::Mutex;
     use tokio::sync::{mpsc, oneshot};
     use tokio_util::sync::CancellationToken;
     use tracing_test::traced_test;
@@ -190,14 +190,10 @@ mod tests {
             result::{CheckResult, CheckStatus},
         },
     };
-    static TEST_MUTEX: Mutex<()> = Mutex::const_new(());
 
     #[traced_test]
-    #[tokio::test(start_paused = true)]
+    #[redis_test(start_paused = true)]
     async fn test_scheduler() {
-        // We don't want these tests to run at the same time, since they access the external redis.
-        let _guard = TEST_MUTEX.lock().await;
-
         let config = Config::default();
         let partition = 0;
 
@@ -297,10 +293,8 @@ mod tests {
     }
 
     #[traced_test]
-    #[tokio::test(start_paused = true)]
+    #[redis_test(start_paused = true)]
     async fn test_scheduler_multi_region() {
-        // We don't want these tests to run at the same time, since they access the external redis.
-        let _guard = TEST_MUTEX.lock().await;
         let config = Config {
             region: "us_west".to_string(),
             ..Default::default()
@@ -407,10 +401,8 @@ mod tests {
     }
 
     #[traced_test]
-    #[tokio::test(start_paused = true)]
+    #[redis_test(start_paused = true)]
     async fn test_scheduler_start() {
-        // We don't want these tests to run at the same time, since they access the external redis.
-        let _guard = TEST_MUTEX.lock().await;
         // TODO: Better abstraction here
         let config = Config::default();
         let partition = 1;

src/scheduler.rs

@@ -0,0 +1,6 @@
+#[cfg(test)]
+pub mod sync {
+    use tokio::sync::Mutex;
+
+    pub static TEST_MUTEX: Mutex<()> = Mutex::const_new(());
+}