fix: Add a test that makes sure the fetch requests are POST

fixes #865

When writing a unit test to assert subgraph requests are POST, I noticed the test would never fail. this is because withf only acts as a request router, and panicking (or in our case asserting) within wouldnt cause the router to fail.

This commit rewrites a couple of subgraph tests so they properly notify whether they completed successfully or failed.

apollo-router-core/src/query_planner/mod.rs

@@ -530,9 +530,11 @@ mod log {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::collections::HashMap;
+    use http::Method;
     use std::str::FromStr;
+    use std::sync::atomic::Ordering;
     use std::sync::Arc;
+    use std::{collections::HashMap, sync::atomic::AtomicBool};
     use tower::{ServiceBuilder, ServiceExt};
     macro_rules! test_query_plan {
         () => {
@@ -569,16 +571,18 @@ mod tests {
             root: serde_json::from_str(test_query_plan!()).unwrap(),
         };
 
+        let succeeded: Arc<AtomicBool> = Default::default();
+        let inner_succeeded = Arc::clone(&succeeded);
+
         let mut mock_products_service = plugin_utils::MockSubgraphService::new();
         mock_products_service
             .expect_call()
             .times(1)
-            .withf(|request| {
-                assert_eq!(
-                    request.http_request.body().operation_name,
-                    Some("topProducts_product_0".into())
-                );
-                true
+            .withf(move |request| {
+                let matches = request.http_request.body().operation_name
+                    == Some("topProducts_product_0".into());
+                inner_succeeded.store(matches, Ordering::SeqCst);
+                matches
             });
         query_plan
             .execute(
@@ -592,5 +596,44 @@ mod tests {
                 &Schema::from_str(test_schema!()).unwrap(),
             )
             .await;
+
+        assert!(succeeded.load(Ordering::SeqCst), "incorrect operation name");
+    }
+
+    #[tokio::test]
+    async fn fetch_makes_post_requests() {
+        let query_plan: QueryPlan = QueryPlan {
+            root: serde_json::from_str(test_query_plan!()).unwrap(),
+        };
+
+        let succeeded: Arc<AtomicBool> = Default::default();
+        let inner_succeeded = Arc::clone(&succeeded);
+
+        let mut mock_products_service = plugin_utils::MockSubgraphService::new();
+        mock_products_service
+            .expect_call()
+            .times(1)
+            .withf(move |request| {
+                let matches = request.http_request.method() == Method::POST;
+                inner_succeeded.store(matches, Ordering::SeqCst);
+                matches
+            });
+        query_plan
+            .execute(
+                &Context::new().with_request(Arc::new(http_compat::Request::mock())),
+                &ServiceRegistry::new(HashMap::from([(
+                    "product".into(),
+                    ServiceBuilder::new()
+                        .buffer(1)
+                        .service(mock_products_service.build().boxed()),
+                )])),
+                &Schema::from_str(test_schema!()).unwrap(),
+            )
+            .await;
+
+        assert!(
+            succeeded.load(Ordering::SeqCst),
+            "subgraph requests must be http post"
+        );
     }
 }

apollo-router/src/plugins/override_url.rs

@@ -63,13 +63,7 @@ mod tests {
         let mut mock_service = MockSubgraphService::new();
         mock_service
             .expect_call()
-            .withf(|req| {
-                assert_eq!(
-                    req.http_request.url(),
-                    &Uri::from_str("http://localhost:8001").unwrap()
-                );
-                true
-            })
+            .withf(|req| req.http_request.url() == &Uri::from_str("http://localhost:8001").unwrap())
             .times(1)
             .returning(move |req: SubgraphRequest| {
                 Ok(plugin_utils::SubgraphResponse::builder()