From 2a746aaac12de9975e18aea58c1df4909d3a1895 Mon Sep 17 00:00:00 2001 From: jonathanrainer Date: Sun, 24 Nov 2024 09:25:53 +0000 Subject: [PATCH] ROVER-245 Fix failing test and clippies --- src/command/lsp/mod.rs | 7 +++---- .../supergraph/config/resolve/subgraph.rs | 6 +++--- src/composition/watchers/composition.rs | 18 +++++++++++++++--- src/utils/supergraph_config.rs | 8 ++++---- tests/e2e/mod.rs | 1 - 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/command/lsp/mod.rs b/src/command/lsp/mod.rs index 405779d36..f1ff6cf1c 100644 --- a/src/command/lsp/mod.rs +++ b/src/command/lsp/mod.rs @@ -27,7 +27,6 @@ use serde::Serialize; use std::collections::HashMap; use std::env::temp_dir; use std::io::stdin; -use tokio::task::JoinHandle; use tower_lsp::lsp_types::{Diagnostic, Range}; use tower_lsp::Server; use tracing::debug; @@ -105,7 +104,7 @@ async fn run_lsp(client_config: StudioClientConfig, lsp_opts: LspOpts) -> RoverR HashMap::from_iter( lazily_resolved_supergraph_config .subgraphs() - .into_iter() + .iter() .map(|(a, b)| (a.to_string(), b.schema().clone())), ), ); @@ -113,7 +112,7 @@ async fn run_lsp(client_config: StudioClientConfig, lsp_opts: LspOpts) -> RoverR let language_server = service.inner().clone(); // Spawn a separate thread to handle composition and passing that data to the language server - let _: JoinHandle> = tokio::spawn(async move { + tokio::spawn(async move { // Create a supergraph binary // TODO: Check defaulting behaviour here and see if we need to centralise let federation_version = lazily_resolved_supergraph_config @@ -220,7 +219,7 @@ async fn run_lsp(client_config: StudioClientConfig, lsp_opts: LspOpts) -> RoverR } } } - Ok(()) + Ok::<(), Error>(()) }); let stdin = tokio::io::stdin(); diff --git a/src/composition/supergraph/config/resolve/subgraph.rs b/src/composition/supergraph/config/resolve/subgraph.rs index 597789764..9fc01a892 100644 --- a/src/composition/supergraph/config/resolve/subgraph.rs +++ b/src/composition/supergraph/config/resolve/subgraph.rs @@ -216,7 +216,7 @@ impl FullyResolvedSubgraph { routing_url: Option, file: &Utf8PathBuf, ) -> Result { - let schema = Fs::read_file(&file).map_err(|err| ResolveSubgraphError::Fs(Box::new(err)))?; + let schema = Fs::read_file(file).map_err(|err| ResolveSubgraphError::Fs(Box::new(err)))?; let is_fed_two = schema_contains_link_directive(&schema); Ok(FullyResolvedSubgraph { routing_url: routing_url.clone(), @@ -228,12 +228,12 @@ impl FullyResolvedSubgraph { async fn resolve_subgraph( fetch_remote_subgraph_impl: &impl FetchRemoteSubgraph, routing_url: Option, - graph_ref: &String, + graph_ref: &str, subgraph: &String, ) -> Result { let graph_ref = GraphRef::from_str(graph_ref).map_err(|err| ResolveSubgraphError::InvalidGraphRef { - graph_ref: graph_ref.clone(), + graph_ref: graph_ref.to_owned(), source: Box::new(err), })?; let remote_subgraph = fetch_remote_subgraph_impl diff --git a/src/composition/watchers/composition.rs b/src/composition/watchers/composition.rs index caf36483e..5a9ce87a4 100644 --- a/src/composition/watchers/composition.rs +++ b/src/composition/watchers/composition.rs @@ -144,6 +144,7 @@ mod tests { use anyhow::Result; use apollo_federation_types::config::FederationVersion; + use apollo_federation_types::config::SchemaSource::Sdl; use camino::Utf8PathBuf; use futures::{ stream::{once, BoxStream}, @@ -155,6 +156,8 @@ mod tests { use speculoos::prelude::*; use tracing_test::traced_test; + use super::CompositionWatcher; + use crate::composition::CompositionSubgraphAdded; use crate::{ composition::{ events::CompositionEvent, @@ -172,8 +175,6 @@ mod tests { }, }; - use super::CompositionWatcher; - #[rstest] #[case::success(false, serde_json::to_string(&default_composition_json()).unwrap())] #[case::error(true, "invalid".to_string())] @@ -216,7 +217,7 @@ mod tests { indoc::indoc! { r#"subgraphs: {}: - routing_url: null + routing_url: localhost schema: sdl: '{}' federation_version: null @@ -253,6 +254,17 @@ mod tests { let (mut composition_messages, composition_subtask) = Subtask::new(composition_handler); let abort_handle = composition_subtask.run(subgraph_change_events); + // Assert we always get a subgraph added event. + let next_message = composition_messages.next().await; + assert_that!(next_message) + .is_some() + .is_equal_to(CompositionEvent::SubgraphAdded(CompositionSubgraphAdded { + name: String::from("subgraph-name"), + schema_source: Sdl { + sdl: String::from("type Query { test: String! }"), + }, + })); + // Assert we always get a composition started event. let next_message = composition_messages.next().await; assert_that!(next_message) diff --git a/src/utils/supergraph_config.rs b/src/utils/supergraph_config.rs index a8f64e4e2..09820cbcc 100644 --- a/src/utils/supergraph_config.rs +++ b/src/utils/supergraph_config.rs @@ -487,7 +487,7 @@ mod test_get_supergraph_config { let supergraph_config_path = third_level_folder.path().join("supergraph.yaml"); fs::write( supergraph_config_path.clone(), - &supergraph_config.into_bytes(), + supergraph_config.into_bytes(), ) .expect("Could not write supergraph.yaml"); @@ -1185,7 +1185,7 @@ subgraphs: routing_url: https://people.example.com schema: file: ./people.graphql"#, - latest_fed2_version.to_string() + latest_fed2_version ); let tmp_home = TempDir::new().unwrap(); let mut config_path = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); @@ -1225,7 +1225,7 @@ subgraphs: routing_url: https://people.example.com schema: file: ../../people.graphql"#, - latest_fed2_version.to_string() + latest_fed2_version ); let tmp_home = TempDir::new().unwrap(); let tmp_dir = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); @@ -1279,7 +1279,7 @@ subgraphs: routing_url: https://people.example.com schema: file: ../../people.graphql"#, - latest_fed2_version.to_string() + latest_fed2_version ); let tmp_home = TempDir::new().unwrap(); let tmp_dir = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); diff --git a/tests/e2e/mod.rs b/tests/e2e/mod.rs index 90e14a72e..04ab41623 100644 --- a/tests/e2e/mod.rs +++ b/tests/e2e/mod.rs @@ -2,7 +2,6 @@ use std::collections::HashMap; use std::env; use std::io::BufRead; use std::io::BufReader; -use std::path::Path; use std::path::PathBuf; use std::process::ChildStderr; use std::time::Duration;