Use automock to mock traits instead of the mock! macro

There were a couple places we were using `mock!` instead of `automock`. IIRC this was because `automock` wasn't working for `async_trait` traits; however, after reading the `mockall` docs, I found that `automock` needs to be placed _before_ `async_trait` in order for it to work. Also, specifically for the `App` trait, I just learned how to specify associated types with `automock`. With this new knowledge, we can replace a couple manual `mock!` impls with `automock`.
roadster-rs · May 23, 2024 · 486fe02 · 486fe02
1 parent 3435f67
commit 486fe02
Show file tree

Hide file tree

Showing 10 changed files with 46 additions and 77 deletions.
diff --git a/src/app.rs b/src/app.rs
@@ -86,6 +86,7 @@ where
     Ok(())
 }
 
+#[cfg_attr(test, mockall::automock(type State = (); type Cli = MockCli; type M = MockMigrator;))]
 #[async_trait]
 pub trait App: Send + Sync {
     // Todo: Are clone, etc necessary if we store it inside an Arc?
@@ -157,18 +158,3 @@ mockall::mock! {
         fn migrations() -> Vec<Box<dyn MigrationTrait>>;
     }
 }
-
-#[cfg(test)]
-mockall::mock! {
-    pub TestApp {}
-    #[async_trait]
-    impl App for TestApp {
-        type State = ();
-        #[cfg(feature = "cli")]
-        type Cli = MockCli;
-        #[cfg(feature = "db-sql")]
-        type M = MockMigrator;
-
-        async fn with_state(context: &AppContext<()>) -> anyhow::Result<<MockTestApp as App>::State>;
-    }
-}
diff --git a/src/cli/mod.rs b/src/cli/mod.rs
@@ -1,6 +1,6 @@
 use crate::app::App;
 #[cfg(test)]
-use crate::app::MockTestApp;
+use crate::app::MockApp;
 #[mockall_double::double]
 use crate::app_context::AppContext;
 use crate::cli::roadster::{RoadsterCli, RunRoadsterCommand};
@@ -101,12 +101,12 @@ mockall::mock! {
     pub Cli {}
 
     #[async_trait]
-    impl RunCommand<MockTestApp> for Cli {
+    impl RunCommand<MockApp> for Cli {
         async fn run(
                 &self,
-                app: &MockTestApp,
-                cli: &<MockTestApp as App>::Cli,
-                context: &AppContext<<MockTestApp as App>::State>,
+                app: &MockApp,
+                cli: &<MockApp as App>::Cli,
+                context: &AppContext<<MockApp as App>::State>,
             ) -> anyhow::Result<bool>;
     }
 
@@ -170,7 +170,7 @@ mod tests {
             .collect_vec();
 
         // Act
-        let (roadster_cli, _a) = super::parse_cli::<MockTestApp, _, _>(args).unwrap();
+        let (roadster_cli, _a) = super::parse_cli::<MockApp, _, _>(args).unwrap();
 
         // Assert
         assert_toml_snapshot!(roadster_cli);

diff --git a/src/controller/http/docs.rs b/src/controller/http/docs.rs
@@ -146,7 +146,7 @@ fn api_schema_route<S>(context: &AppContext<S>) -> String {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::app::MockTestApp;
+    use crate::app::MockApp;
     use crate::app_context::MockAppContext;
     use crate::config::app_config::AppConfig;
     use rstest::rstest;
@@ -176,7 +176,7 @@ mod tests {
             .scalar
             .route
             .clone_from(&route);
-        let mut context = MockAppContext::<MockTestApp>::default();
+        let mut context = MockAppContext::<MockApp>::default();
         context.expect_config().return_const(config);
 
         assert_eq!(scalar_enabled(&context), enabled);
@@ -209,7 +209,7 @@ mod tests {
             .redoc
             .route
             .clone_from(&route);
-        let mut context = MockAppContext::<MockTestApp>::default();
+        let mut context = MockAppContext::<MockApp>::default();
         context.expect_config().return_const(config);
 
         assert_eq!(redoc_enabled(&context), enabled);
@@ -242,7 +242,7 @@ mod tests {
             .api_schema
             .route
             .clone_from(&route);
-        let mut context = MockAppContext::<MockTestApp>::default();
+        let mut context = MockAppContext::<MockApp>::default();
         context.expect_config().return_const(config);
 
         assert_eq!(api_schema_enabled(&context), enabled);

diff --git a/src/controller/http/health.rs b/src/controller/http/health.rs
@@ -249,7 +249,7 @@ fn health_get_docs(op: TransformOperation) -> TransformOperation {
 
 #[cfg(test)]
 mod tests {
-    use crate::app::MockTestApp;
+    use crate::app::MockApp;
     use crate::app_context::MockAppContext;
     use crate::config::app_config::AppConfig;
     use rstest::rstest;
@@ -279,7 +279,7 @@ mod tests {
             .health
             .route
             .clone_from(&route);
-        let mut context = MockAppContext::<MockTestApp>::default();
+        let mut context = MockAppContext::<MockApp>::default();
         context.expect_config().return_const(config);
 
         assert_eq!(super::enabled(&context), enabled);

diff --git a/src/controller/http/ping.rs b/src/controller/http/ping.rs
@@ -88,7 +88,7 @@ fn ping_get_docs(op: TransformOperation) -> TransformOperation {
 
 #[cfg(test)]
 mod tests {
-    use crate::app::MockTestApp;
+    use crate::app::MockApp;
     use crate::app_context::MockAppContext;
     use crate::config::app_config::AppConfig;
     use rstest::rstest;
@@ -118,7 +118,7 @@ mod tests {
             .ping
             .route
             .clone_from(&route);
-        let mut context = MockAppContext::<MockTestApp>::default();
+        let mut context = MockAppContext::<MockApp>::default();
         context.expect_config().return_const(config);
 
         assert_eq!(super::enabled(&context), enabled);

diff --git a/src/service/http/builder.rs b/src/service/http/builder.rs
@@ -212,7 +212,7 @@ impl<A: App> AppServiceBuilder<A, HttpService> for HttpServiceBuilder<A> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::app::MockTestApp;
+    use crate::app::MockApp;
     use crate::app_context::MockAppContext;
     use crate::service::http::initializer::MockInitializer;
     use crate::service::http::middleware::MockMiddleware;
@@ -225,7 +225,7 @@ mod tests {
         context
             .expect_clone()
             .returning(MockAppContext::<()>::default);
-        let builder = HttpServiceBuilder::<MockTestApp>::empty(&context);
+        let builder = HttpServiceBuilder::<MockApp>::empty(&context);
 
         let mut middleware = MockMiddleware::default();
         middleware.expect_enabled().returning(|_| true);
@@ -247,7 +247,7 @@ mod tests {
         context
             .expect_clone()
             .returning(MockAppContext::<()>::default);
-        let builder = HttpServiceBuilder::<MockTestApp>::empty(&context);
+        let builder = HttpServiceBuilder::<MockApp>::empty(&context);
 
         let mut middleware = MockMiddleware::default();
         middleware.expect_enabled().returning(|_| false);
@@ -268,7 +268,7 @@ mod tests {
         context
             .expect_clone()
             .returning(MockAppContext::<()>::default);
-        let builder = HttpServiceBuilder::<MockTestApp>::empty(&context);
+        let builder = HttpServiceBuilder::<MockApp>::empty(&context);
 
         let mut middleware = MockMiddleware::default();
         middleware.expect_name().returning(|| "test".to_string());
@@ -289,7 +289,7 @@ mod tests {
         context
             .expect_clone()
             .returning(MockAppContext::<()>::default);
-        let builder = HttpServiceBuilder::<MockTestApp>::empty(&context);
+        let builder = HttpServiceBuilder::<MockApp>::empty(&context);
 
         let mut initializer = MockInitializer::default();
         initializer.expect_enabled().returning(|_| true);
@@ -311,7 +311,7 @@ mod tests {
         context
             .expect_clone()
             .returning(MockAppContext::<()>::default);
-        let builder = HttpServiceBuilder::<MockTestApp>::empty(&context);
+        let builder = HttpServiceBuilder::<MockApp>::empty(&context);
 
         let mut initializer = MockInitializer::default();
         initializer.expect_enabled().returning(|_| false);
@@ -332,7 +332,7 @@ mod tests {
         context
             .expect_clone()
             .returning(MockAppContext::<()>::default);
-        let builder = HttpServiceBuilder::<MockTestApp>::empty(&context);
+        let builder = HttpServiceBuilder::<MockApp>::empty(&context);
 
         let mut initializer = MockInitializer::default();
         initializer.expect_name().returning(|| "test".to_string());

diff --git a/src/service/mod.rs b/src/service/mod.rs
@@ -79,20 +79,20 @@ where
 
 #[cfg(test)]
 mod tests {
-    use crate::app::MockTestApp;
+    use crate::app::MockApp;
     use crate::app_context::MockAppContext;
     use crate::service::{AppServiceBuilder, MockAppService};
     use async_trait::async_trait;
     use rstest::rstest;
 
     struct TestAppServiceBuilder;
     #[async_trait]
-    impl AppServiceBuilder<MockTestApp, MockAppService<MockTestApp>> for TestAppServiceBuilder {
+    impl AppServiceBuilder<MockApp, MockAppService<MockApp>> for TestAppServiceBuilder {
         #[cfg_attr(coverage_nightly, coverage(off))]
         async fn build(
             self,
             _context: &MockAppContext<()>,
-        ) -> anyhow::Result<MockAppService<MockTestApp>> {
+        ) -> anyhow::Result<MockAppService<MockApp>> {
             Ok(MockAppService::default())
         }
     }
@@ -106,7 +106,7 @@ mod tests {
         let mut context = MockAppContext::default();
         context.expect_clone().returning(MockAppContext::default);
 
-        let enabled_ctx = MockAppService::<MockTestApp>::enabled_context();
+        let enabled_ctx = MockAppService::<MockApp>::enabled_context();
         enabled_ctx.expect().returning(move |_| service_enabled);
 
         // Act

diff --git a/src/service/registry.rs b/src/service/registry.rs
@@ -70,7 +70,7 @@ impl<A: App> ServiceRegistry<A> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::app::MockTestApp;
+    use crate::app::MockApp;
     use crate::app_context::MockAppContext;
     use crate::service::{MockAppService, MockAppServiceBuilder};
     use rstest::rstest;
@@ -84,14 +84,14 @@ mod tests {
         let mut context = MockAppContext::default();
         context.expect_clone().returning(MockAppContext::default);
 
-        let service: MockAppService<MockTestApp> = MockAppService::default();
-        let enabled_ctx = MockAppService::<MockTestApp>::enabled_context();
+        let service: MockAppService<MockApp> = MockAppService::default();
+        let enabled_ctx = MockAppService::<MockApp>::enabled_context();
         enabled_ctx.expect().returning(move |_| service_enabled);
-        let name_ctx = MockAppService::<MockTestApp>::name_context();
+        let name_ctx = MockAppService::<MockApp>::name_context();
         name_ctx.expect().returning(|| "test".to_string());
 
         // Act
-        let mut subject: ServiceRegistry<MockTestApp> = ServiceRegistry::new(&context);
+        let mut subject: ServiceRegistry<MockApp> = ServiceRegistry::new(&context);
         subject.register_service(service).unwrap();
 
         // Assert
@@ -115,9 +115,9 @@ mod tests {
         let mut context = MockAppContext::default();
         context.expect_clone().returning(MockAppContext::default);
 
-        let enabled_ctx = MockAppService::<MockTestApp>::enabled_context();
+        let enabled_ctx = MockAppService::<MockApp>::enabled_context();
         enabled_ctx.expect().returning(move |_| service_enabled);
-        let name_ctx = MockAppService::<MockTestApp>::name_context();
+        let name_ctx = MockAppService::<MockApp>::name_context();
         name_ctx.expect().returning(|| "test".to_string());
 
         let mut builder = MockAppServiceBuilder::default();
@@ -131,7 +131,7 @@ mod tests {
         }
 
         // Act
-        let mut subject: ServiceRegistry<MockTestApp> = ServiceRegistry::new(&context);
+        let mut subject: ServiceRegistry<MockApp> = ServiceRegistry::new(&context);
         subject.register_builder(builder).await.unwrap();
 
         // Assert

diff --git a/src/service/worker/sidekiq/builder.rs b/src/service/worker/sidekiq/builder.rs
@@ -333,6 +333,7 @@ async fn remove_stale_periodic_jobs<S, C: RedisCommands>(
 
 /// Trait to help with mocking responses from Redis.
 // Todo: Make available to other parts of the project?
+#[cfg_attr(test, mockall::automock)]
 #[async_trait]
 trait RedisCommands {
     async fn zrange(
@@ -369,7 +370,7 @@ impl<'a> RedisCommands for PooledConnection<'a, RedisConnectionManager> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::app::MockTestApp;
+    use crate::app::MockApp;
     use crate::app_context::MockAppContext;
     use crate::config::app_config::AppConfig;
     use crate::service::worker::sidekiq::MockProcessor;
@@ -453,7 +454,7 @@ mod tests {
         }
 
         #[async_trait]
-        impl AppWorker<MockTestApp, ()> for TestAppWorker
+        impl AppWorker<MockApp, ()> for TestAppWorker
         {
             fn build(context: &MockAppContext<()>) -> Self;
         }
@@ -464,7 +465,7 @@ mod tests {
         enabled: bool,
         register_count: usize,
         periodic_count: usize,
-    ) -> SidekiqWorkerServiceBuilder<MockTestApp> {
+    ) -> SidekiqWorkerServiceBuilder<MockApp> {
         let mut config = AppConfig::empty(None).unwrap();
         config.service.default_enable = enabled;
         config.service.sidekiq.custom.num_workers = 1;
@@ -476,7 +477,7 @@ mod tests {
         let pool = Pool::builder().build_unchecked(redis_fetch);
         context.expect_redis_fetch().return_const(Some(pool));
 
-        let mut processor = MockProcessor::<MockTestApp>::default();
+        let mut processor = MockProcessor::<MockApp>::default();
         processor
             .expect_register::<(), MockTestAppWorker>()
             .times(register_count)
@@ -486,14 +487,14 @@ mod tests {
             .times(periodic_count)
             .returning(|_, _| Ok(()));
 
-        SidekiqWorkerServiceBuilder::<MockTestApp>::new(context, Some(processor))
+        SidekiqWorkerServiceBuilder::<MockApp>::new(context, Some(processor))
             .await
             .unwrap()
     }
 
     #[cfg_attr(coverage_nightly, coverage(off))]
     fn validate_registered_workers(
-        builder: &SidekiqWorkerServiceBuilder<MockTestApp>,
+        builder: &SidekiqWorkerServiceBuilder<MockApp>,
         enabled: bool,
         size: usize,
         class_names: Vec<String>,
@@ -516,7 +517,7 @@ mod tests {
 
     #[cfg_attr(coverage_nightly, coverage(off))]
     fn validate_registered_periodic_workers(
-        builder: &SidekiqWorkerServiceBuilder<MockTestApp>,
+        builder: &SidekiqWorkerServiceBuilder<MockApp>,
         enabled: bool,
         size: usize,
         job_names: Vec<String>,
@@ -594,10 +595,10 @@ mod tests {
             config.service.sidekiq.custom.periodic.stale_cleanup = StaleCleanUpBehavior::Manual;
         }
 
-        let mut context = MockAppContext::<MockTestApp>::default();
+        let mut context = MockAppContext::<MockApp>::default();
         context.expect_config().return_const(config);
 
-        let mut redis = MockTestRedisCommands::default();
+        let mut redis = MockRedisCommands::default();
         redis
             .expect_zrange()
             .times(1)
@@ -619,22 +620,4 @@ mod tests {
             .await
             .unwrap();
     }
-
-    mockall::mock! {
-        TestRedisCommands {}
-
-        #[async_trait]
-        impl RedisCommands for TestRedisCommands {
-            async fn zrange(
-                & mut self,
-                key: String,
-                lower: isize,
-                upper: isize,
-            ) -> Result<Vec<String>, RedisError>;
-
-            async fn zrem<V>(&mut self, key: String, value: V) -> Result<bool, RedisError>
-            where
-                V: ToRedisArgs + Send + Sync + 'static;
-        }
-    }
 }