Skip to content

Commit

Permalink
Remove mock AppContext and use a concrete version in tests instead
Browse files Browse the repository at this point in the history
There are some inconveniences of using a mock `AppContext`:

1. Need to use `mockall_double` when importing `AppContext` whenever
   it's needed for tests. This isn't a huge deal but is a little
   annoying.
2. Need to update the `mock!` any time definitions in the `AppContext`
   impl change
3. Code completion doesn't seem to be working -- `context.config()`
   doesn't show up, nor do the fields on the config.

However there is at least one benefit:
1. In order to create a concrete redis pool, the creation code needs to
   run in an async context. This means that any test that needs the
   `AppContext` needs to run with `#[tokio::test]`. This isn't a huge
   deal, but is slightly inconvenient. However, using a mock
   `AppContext` removes this requirement (aside from the few tests that
   still need a concrete redis pool).

To get the above benefit of a mock `AppContext` without the listed cons,
we can mock just the `AppContextInner` that we already created for the
`AppContext`. Then, we can still create the `AppContext` in tests
without needing to create a concrete redis pool, code completiong works,
and we don't need to sprinkle `mockall_double` everywhere (for the
`AppContext` at least).

Also, make `mockall_double` a dev dependency

Closes #154
  • Loading branch information
spencewenski committed May 23, 2024
1 parent 0fc12e9 commit 31b1828
Show file tree
Hide file tree
Showing 38 changed files with 188 additions and 261 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,14 @@ const_format = "0.2.32"
typed-builder = "0.18.2"
num-traits = "0.2.19"
log = "0.4.21"
mockall_double = "0.3.1"
futures = "0.3.30"
validator = { version = "0.18.1", features = ["derive"] }

[dev-dependencies]
cargo-husky = { version = "1.5.0", default-features = false, features = ["user-hooks"] }
insta = { version = "1.39.0", features = ["toml"] }
mockall = "0.12.1"
mockall_double = "0.3.1"
rstest = "0.19.0"

[workspace]
Expand Down
3 changes: 1 addition & 2 deletions src/app.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#[mockall_double::double]
use crate::app_context::AppContext;
#[cfg(feature = "cli")]
use crate::cli::parse_cli;
Expand Down Expand Up @@ -51,7 +50,7 @@ where
#[cfg(not(test))]
let context = AppContext::<()>::new::<A>(config).await?;
#[cfg(test)]
let context = AppContext::<()>::default();
let context = AppContext::<()>::test(Some(config), None)?;

let state = A::with_state(&context).await?;
let context = context.with_custom(state);
Expand Down
163 changes: 97 additions & 66 deletions src/app_context.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
use crate::app::App;
use crate::config::app_config::AppConfig;
#[cfg(feature = "db-sql")]
use sea_orm::Database;
#[cfg(feature = "db-sql")]
use sea_orm::DatabaseConnection;
use std::sync::Arc;
#[cfg(feature = "sidekiq")]
use tracing::info;

#[cfg(not(test))]
type Inner = AppContextInner;
#[cfg(test)]
type Inner = MockAppContextInner;

#[derive(Clone)]
pub struct AppContext<T = ()> {
inner: Arc<AppContextInner>,
inner: Arc<Inner>,
custom: Arc<T>,
}

Expand All @@ -23,54 +24,84 @@ impl<T> AppContext<T> {
where
A: App,
{
#[cfg(feature = "db-sql")]
let db = Database::connect(A::db_connection_options(&config)?).await?;
#[cfg(test)]
// The `config.clone()` here is technically not necessary. However, without it, RustRover
// is giving a "value used after move" error when creating an actual `AppContext` below.
let context = { Self::test(Some(config.clone()), None)? };

#[cfg(feature = "sidekiq")]
let (redis_enqueue, redis_fetch) = {
let sidekiq_config = &config.service.sidekiq;
let redis_config = &sidekiq_config.custom.redis;
let redis = sidekiq::RedisConnectionManager::new(redis_config.uri.to_string())?;
let redis_enqueue = {
let pool = bb8::Pool::builder().min_idle(redis_config.enqueue_pool.min_idle);
let pool = redis_config
.enqueue_pool
.max_connections
.iter()
.fold(pool, |pool, max_conns| pool.max_size(*max_conns));
pool.build(redis.clone()).await?
};
let redis_fetch = if redis_config
.fetch_pool
.max_connections
.iter()
.any(|max_conns| *max_conns == 0)
{
info!(
"Redis fetch pool configured with size of zero, will not start the Sidekiq processor"
);
None
} else {
let pool = bb8::Pool::builder().min_idle(redis_config.fetch_pool.min_idle);
let pool = redis_config
#[cfg(not(test))]
let context = {
#[cfg(feature = "db-sql")]
let db = sea_orm::Database::connect(A::db_connection_options(&config)?).await?;

#[cfg(feature = "sidekiq")]
let (redis_enqueue, redis_fetch) = {
let sidekiq_config = &config.service.sidekiq;
let redis_config = &sidekiq_config.custom.redis;
let redis = sidekiq::RedisConnectionManager::new(redis_config.uri.to_string())?;
let redis_enqueue = {
let pool = bb8::Pool::builder().min_idle(redis_config.enqueue_pool.min_idle);
let pool = redis_config
.enqueue_pool
.max_connections
.iter()
.fold(pool, |pool, max_conns| pool.max_size(*max_conns));
pool.build(redis.clone()).await?
};
let redis_fetch = if redis_config
.fetch_pool
.max_connections
.iter()
.fold(pool, |pool, max_conns| pool.max_size(*max_conns));
Some(pool.build(redis.clone()).await?)
.any(|max_conns| *max_conns == 0)
{
tracing::info!("Redis fetch pool configured with size of zero, will not start the Sidekiq processor");
None
} else {
let pool = bb8::Pool::builder().min_idle(redis_config.fetch_pool.min_idle);
let pool = redis_config
.fetch_pool
.max_connections
.iter()
.fold(pool, |pool, max_conns| pool.max_size(*max_conns));
Some(pool.build(redis.clone()).await?)
};
(redis_enqueue, redis_fetch)
};
(redis_enqueue, redis_fetch)
let inner = AppContextInner {
config,
#[cfg(feature = "db-sql")]
db,
#[cfg(feature = "sidekiq")]
redis_enqueue,
#[cfg(feature = "sidekiq")]
redis_fetch,
};
AppContext {
inner: Arc::new(inner),
custom: Arc::new(()),
}
};

let inner = AppContextInner {
config,
#[cfg(feature = "db-sql")]
db,
#[cfg(feature = "sidekiq")]
redis_enqueue,
#[cfg(feature = "sidekiq")]
redis_fetch,
};
Ok(context)
}

#[cfg(test)]
pub(crate) fn test(
config: Option<AppConfig>,
#[cfg(not(feature = "sidekiq"))] _redis: Option<()>,
#[cfg(feature = "sidekiq")] redis: Option<sidekiq::RedisPool>,
) -> anyhow::Result<AppContext<()>> {
let mut inner = MockAppContextInner::default();
inner
.expect_config()
.return_const(config.unwrap_or(AppConfig::test(None)?));
#[cfg(feature = "sidekiq")]
if let Some(redis) = redis {
inner.expect_redis_enqueue().return_const(redis.clone());
inner.expect_redis_fetch().return_const(Some(redis));
} else {
inner.expect_redis_fetch().return_const(None);
}
Ok(AppContext {
inner: Arc::new(inner),
custom: Arc::new(()),
Expand All @@ -85,30 +116,29 @@ impl<T> AppContext<T> {
}

pub fn config(&self) -> &AppConfig {
&self.inner.config
self.inner.config()
}

#[cfg(feature = "db-sql")]
pub fn db(&self) -> &DatabaseConnection {
&self.inner.db
self.inner.db()
}

#[cfg(feature = "sidekiq")]
pub fn redis_enqueue(&self) -> &sidekiq::RedisPool {
&self.inner.redis_enqueue
self.inner.redis_enqueue()
}

#[cfg(feature = "sidekiq")]
pub fn redis_fetch(&self) -> &Option<sidekiq::RedisPool> {
&self.inner.redis_fetch
self.inner.redis_fetch()
}

pub fn custom(&self) -> &T {
&self.custom
}
}

#[derive(Debug)]
struct AppContextInner {
config: AppConfig,
#[cfg(feature = "db-sql")]
Expand All @@ -122,24 +152,25 @@ struct AppContextInner {
redis_fetch: Option<sidekiq::RedisPool>,
}

#[cfg(test)]
mockall::mock! {
pub AppContext<T: 'static = ()> {
pub fn config(&self) -> &AppConfig;

#[cfg(feature = "db-sql")]
pub fn db(&self) -> &DatabaseConnection;

#[cfg(feature = "sidekiq")]
pub fn redis_enqueue(&self) -> &sidekiq::RedisPool;
#[cfg_attr(test, mockall::automock)]
#[cfg_attr(test, allow(dead_code))]
impl AppContextInner {
fn config(&self) -> &AppConfig {
&self.config
}

#[cfg(feature = "sidekiq")]
pub fn redis_fetch(&self) -> &Option<sidekiq::RedisPool>;
#[cfg(feature = "db-sql")]
fn db(&self) -> &DatabaseConnection {
&self.db
}

pub fn with_custom<NewT: 'static>(self, custom: NewT) -> MockAppContext<NewT>;
#[cfg(feature = "sidekiq")]
fn redis_enqueue(&self) -> &sidekiq::RedisPool {
&self.redis_enqueue
}

impl<T> Clone for AppContext<T> {
fn clone(&self) -> Self;
#[cfg(feature = "sidekiq")]
fn redis_fetch(&self) -> &Option<sidekiq::RedisPool> {
&self.redis_fetch
}
}
1 change: 0 additions & 1 deletion src/cli/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::app::App;
#[cfg(test)]
use crate::app::MockApp;
#[mockall_double::double]
use crate::app_context::AppContext;
use crate::cli::roadster::{RoadsterCli, RunRoadsterCommand};
use async_trait::async_trait;
Expand Down
1 change: 0 additions & 1 deletion src/cli/roadster/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use serde_derive::Serialize;
use tracing::warn;

use crate::app::App;
#[mockall_double::double]
use crate::app_context::AppContext;
use crate::cli::roadster::{RoadsterCli, RunRoadsterCommand};

Expand Down
1 change: 0 additions & 1 deletion src/cli/roadster/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::app::App;
#[mockall_double::double]
use crate::app_context::AppContext;
#[cfg(feature = "open-api")]
use crate::cli::roadster::list_routes::ListRoutesArgs;
Expand Down
1 change: 0 additions & 1 deletion src/cli/roadster/print_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use strum_macros::{EnumString, IntoStaticStr};
use tracing::info;

use crate::app::App;
#[mockall_double::double]
use crate::app_context::AppContext;
use crate::cli::roadster::{RoadsterCli, RunRoadsterCommand};

Expand Down
4 changes: 2 additions & 2 deletions src/config/app_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl AppConfig {
}

#[cfg(test)]
pub(crate) fn empty(config_str: Option<&str>) -> anyhow::Result<Self> {
pub(crate) fn test(config_str: Option<&str>) -> anyhow::Result<Self> {
let config = config_str.unwrap_or(
r#"
environment = "test"
Expand Down Expand Up @@ -242,6 +242,6 @@ mod tests {
#[test]
#[cfg_attr(coverage_nightly, coverage(off))]
fn empty() {
AppConfig::empty(None).unwrap();
AppConfig::test(None).unwrap();
}
}
1 change: 0 additions & 1 deletion src/config/service/http/initializer.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#[mockall_double::double]
use crate::app_context::AppContext;
use crate::config::app_config::CustomConfig;
use crate::service::http::initializer::normalize_path::NormalizePathConfig;
Expand Down
7 changes: 2 additions & 5 deletions src/config/service/http/middleware.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#[mockall_double::double]
use crate::app_context::AppContext;
use crate::config::app_config::CustomConfig;
use crate::service::http::middleware::catch_panic::CatchPanicConfig;
Expand Down Expand Up @@ -171,7 +170,6 @@ impl<T: Default> MiddlewareConfig<T> {
#[cfg(test)]
mod tests {
use super::*;
use crate::app_context::MockAppContext;
use crate::config::app_config::AppConfig;
use rstest::rstest;
use serde_json::Value;
Expand All @@ -190,11 +188,10 @@ mod tests {
#[case] expected_enabled: bool,
) {
// Arrange
let mut config = AppConfig::empty(None).unwrap();
let mut config = AppConfig::test(None).unwrap();
config.service.http.custom.middleware.default_enable = default_enable;

let mut context = MockAppContext::<()>::default();
context.expect_config().return_const(config);
let context = AppContext::<()>::test(Some(config), None).unwrap();

let common_config = CommonConfig {
enable,
Expand Down
1 change: 0 additions & 1 deletion src/config/service/http/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#[mockall_double::double]
use crate::app_context::AppContext;
use crate::config::service::http::initializer::Initializer;
use crate::config::service::http::middleware::Middleware;
Expand Down
1 change: 0 additions & 1 deletion src/config/service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
pub mod http;
pub mod worker;

#[mockall_double::double]
use crate::app_context::AppContext;
#[cfg(feature = "http")]
use crate::config::service::http::HttpServiceConfig;
Expand Down
Loading

0 comments on commit 31b1828

Please sign in to comment.