ROVER-279 Fix clippies and tests

apollographql · Jan 8, 2025 · 410d39d · 410d39d
1 parent 9738f02
commit 410d39d
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 41 deletions.
diff --git a/src/composition/supergraph/config/resolver/mod.rs b/src/composition/supergraph/config/resolver/mod.rs
@@ -25,12 +25,10 @@ use rover_client::shared::GraphRef;
 use tower::{MakeService, Service, ServiceExt};
 use url::Url;
 
-use crate::{
-    cli::Rover,
-    utils::{effect::read_stdin::ReadStdin, parsers::FileDescriptorType},
-    RoverError,
+use self::{
+    fetch_remote_subgraph::FetchRemoteSubgraphFactory,
+    fetch_remote_subgraphs::FetchRemoteSubgraphsRequest, state::ResolveSubgraphs,
 };
-
 use super::{
     error::ResolveSubgraphError,
     federation::{
@@ -41,10 +39,10 @@ use super::{
     lazy::LazilyResolvedSupergraphConfig,
     unresolved::UnresolvedSupergraphConfig,
 };
-
-use self::{
-    fetch_remote_subgraph::FetchRemoteSubgraphFactory,
-    fetch_remote_subgraphs::FetchRemoteSubgraphsRequest, state::ResolveSubgraphs,
+use crate::{
+    cli::Rover,
+    utils::{effect::read_stdin::ReadStdin, parsers::FileDescriptorType},
+    RoverError,
 };
 
 pub mod fetch_remote_subgraph;
@@ -439,6 +437,7 @@ fn maybe_name_from_dir() -> Option<String> {
 
 #[cfg(test)]
 mod tests {
+    use std::sync::Arc;
     use std::{collections::BTreeMap, str::FromStr};
 
     use anyhow::Result;
@@ -458,6 +457,14 @@ mod tests {
     use tower::{ServiceBuilder, ServiceExt};
     use tower_test::mock::Handle;
 
+    use super::{
+        fetch_remote_subgraph::{
+            FetchRemoteSubgraphError, FetchRemoteSubgraphFactory, FetchRemoteSubgraphRequest,
+            MakeFetchRemoteSubgraphError, RemoteSubgraph,
+        },
+        fetch_remote_subgraphs::{FetchRemoteSubgraphsRequest, MakeFetchRemoteSubgraphsError},
+        MockPrompt, SupergraphConfigResolver,
+    };
     use crate::{
         composition::supergraph::config::{
             error::ResolveSubgraphError,
@@ -475,15 +482,6 @@ mod tests {
         },
     };
 
-    use super::{
-        fetch_remote_subgraph::{
-            FetchRemoteSubgraphError, FetchRemoteSubgraphFactory, FetchRemoteSubgraphRequest,
-            MakeFetchRemoteSubgraphError, RemoteSubgraph,
-        },
-        fetch_remote_subgraphs::{FetchRemoteSubgraphsRequest, MakeFetchRemoteSubgraphsError},
-        MockPrompt, SupergraphConfigResolver,
-    };
-
     /// Test showing that federation version is selected from the user-specified fed version
     /// over local supergraph config, remote composition version, or version inferred from
     /// resolved SDLs
@@ -695,7 +693,7 @@ mod tests {
                             .boxed_clone()
                             .map_err(|err| ResolveSubgraphError::IntrospectionError {
                                 subgraph_name: "dont-call-me".to_string(),
-                                source: err,
+                                source: Arc::new(err),
                             })
                             .service(resolve_introspect_subgraph_service.into_inner()))
                     }
@@ -928,7 +926,7 @@ mod tests {
                             .boxed_clone()
                             .map_err(|err| ResolveSubgraphError::IntrospectionError {
                                 subgraph_name: "dont-call-me".to_string(),
-                                source: err,
+                                source: Arc::new(err),
                             })
                             .service(resolve_introspect_subgraph_service.into_inner()))
                     }
@@ -1161,7 +1159,7 @@ mod tests {
                             .boxed_clone()
                             .map_err(|err| ResolveSubgraphError::IntrospectionError {
                                 subgraph_name: "dont-call-me".to_string(),
-                                source: err,
+                                source: Arc::new(err),
                             })
                             .service(resolve_introspect_subgraph_service.into_inner()))
                     }

diff --git a/src/composition/supergraph/config/unresolved/supergraph.rs b/src/composition/supergraph/config/unresolved/supergraph.rs
@@ -48,6 +48,7 @@ impl UnresolvedSupergraphConfig {
 
 #[cfg(test)]
 mod tests {
+    use std::sync::Arc;
     use std::{
         collections::{BTreeMap, HashSet},
         str::FromStr,
@@ -436,7 +437,7 @@ mod tests {
                         .boxed_clone()
                         .map_err(move |err| ResolveSubgraphError::IntrospectionError {
                             subgraph_name: introspect_subgraph_name.to_string(),
-                            source: err,
+                            source: Arc::new(err),
                         })
                         .service(resolve_introspect_subgraph_service.into_inner()),
                 );
@@ -464,7 +465,7 @@ mod tests {
                     let introspect_subgraph_name = introspect_subgraph_name.to_string();
                     move |err| ResolveSubgraphError::IntrospectionError {
                         subgraph_name: introspect_subgraph_name.to_string(),
-                        source: err,
+                        source: Arc::new(err),
                     }
                 })
                 .service(resolve_introspect_subgraph_factory.into_inner()),
@@ -732,7 +733,7 @@ mod tests {
                         .boxed_clone()
                         .map_err(move |err| ResolveSubgraphError::IntrospectionError {
                             subgraph_name: introspect_subgraph_name.to_string(),
-                            source: err,
+                            source: Arc::new(err),
                         })
                         .service(resolve_introspect_subgraph_service.into_inner()),
                 );
@@ -745,7 +746,7 @@ mod tests {
                     let introspect_subgraph_name = introspect_subgraph_name.to_string();
                     move |err| ResolveSubgraphError::IntrospectionError {
                         subgraph_name: introspect_subgraph_name.to_string(),
-                        source: err,
+                        source: Arc::new(err),
                     }
                 })
                 .service(resolve_introspect_subgraph_factory.into_inner()),

diff --git a/src/composition/watchers/federation.rs b/src/composition/watchers/federation.rs
@@ -26,17 +26,15 @@ impl SubtaskHandleStream for FederationWatcher {
     ) -> AbortHandle {
         tokio::task::spawn(async move {
             while let Some(recv_res) = input.next().await {
-                match recv_res {
-                    Err(SupergraphConfigSerialisationError::DeserializingConfigError {
-                        source,
-                    }) => {
-                        let _ = sender
-                            .send(CompositionEvent::Error(
-                                CompositionError::InvalidSupergraphConfig(source.message()),
-                            ))
-                            .tap_err(|err| error!("{:?}", err));
-                    }
-                    _ => (),
+                if let Err(SupergraphConfigSerialisationError::DeserializingConfigError {
+                    source,
+                }) = recv_res
+                {
+                    let _ = sender
+                        .send(CompositionEvent::Error(
+                            CompositionError::InvalidSupergraphConfig(source.message()),
+                        ))
+                        .tap_err(|err| error!("{:?}", err));
                 }
             }
         })

diff --git a/src/composition/watchers/subgraphs.rs b/src/composition/watchers/subgraphs.rs
@@ -407,6 +407,8 @@ impl SubgraphHandles {
 
 #[cfg(test)]
 mod tests {
+    use std::sync::Arc;
+
     use apollo_federation_types::config::SchemaSource;
     use camino::Utf8PathBuf;
     use speculoos::prelude::*;
@@ -494,7 +496,7 @@ mod tests {
                 send_response.send_response(
                     ServiceBuilder::new()
                         .boxed_clone()
-                        .map_err(ResolveSubgraphError::ServiceReady)
+                        .map_err(|e| ResolveSubgraphError::ServiceReady(Arc::new(e)))
                         .service(resolve_introspect_subgraph_service.into_inner()),
                 );
             }
@@ -503,7 +505,7 @@ mod tests {
         let resolve_introspect_subgraph_factory: ResolveIntrospectSubgraphFactory =
             ServiceBuilder::new()
                 .boxed_clone()
-                .map_err(ResolveSubgraphError::ServiceReady)
+                .map_err(|e| ResolveSubgraphError::ServiceReady(Arc::new(e)))
                 .service(resolve_introspect_subgraph_factory.into_inner());
 
         let (fetch_remote_subgraph_service, mut fetch_remote_subgraph_service_handle) =

diff --git a/src/composition/watchers/watcher/file.rs b/src/composition/watchers/watcher/file.rs
@@ -162,17 +162,15 @@ mod tests {
     use std::{fs::OpenOptions, io::Write, time::Duration};
 
     use apollo_federation_types::config::{SchemaSource, SubgraphConfig};
-    use assert_fs::TempDir;
     use speculoos::prelude::*;
     use tokio::time::timeout;
     use tower::ServiceExt;
     use tracing_test::traced_test;
 
+    use super::*;
     use crate::composition::supergraph::config::full::file::ResolveFileSubgraph;
     use crate::composition::supergraph::config::unresolved::UnresolvedSubgraph;
 
-    use super::*;
-
     #[tokio::test]
     #[traced_test(level = "error")]
     async fn it_watches() {