From 18d8fda52e2c419b9aeda7b3546704208d761f1f Mon Sep 17 00:00:00 2001 From: Andi Pieper Date: Mon, 19 Feb 2024 16:35:38 +0100 Subject: [PATCH] fix(tests): add 10ms delay after we are done with stubr (#408) * add 10ms delay after we are done with stubr, make sure we drop it ourselves in all cases, removed old check/wait functions around stubr * re-connect separated tests --- Cargo.lock | 1 + Cargo.toml | 1 + tests/api/ai_explain.rs | 7 ++--- tests/api/ai_help.rs | 6 ++-- tests/api/ai_help_history.rs | 7 ++--- tests/api/auth.rs | 8 +++-- tests/api/fxa_webhooks.rs | 51 +++++++++++++++---------------- tests/api/multiple_collections.rs | 51 ++++++++++++++++--------------- tests/api/newsletter.rs | 29 ++++++++++-------- tests/api/ping.rs | 23 ++++---------- tests/api/play.rs | 8 ++--- tests/api/root.rs | 5 ++- tests/api/search.rs | 5 ++- tests/api/settings.rs | 6 ++-- tests/api/updates.rs | 13 ++++---- tests/api/whoami.rs | 29 ++++++------------ tests/fxa/callback.rs | 25 +++++---------- tests/helpers/app.rs | 13 ++++++++ tests/helpers/http_client.rs | 14 --------- tests/helpers/mod.rs | 21 +------------ 20 files changed, 137 insertions(+), 186 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b6d2deae..3fc23e6b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3736,6 +3736,7 @@ dependencies = [ "stubr-attributes", "thiserror", "tiktoken-rs", + "tokio", "url", "uuid", "validator", diff --git a/Cargo.toml b/Cargo.toml index 66f2eb67..b2f5b2e2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -90,3 +90,4 @@ sha2 = "0.10" stubr = "0.6" stubr-attributes = "0.6" assert-json-diff = "2" +tokio = { version = "1", features = ["io-util"] } diff --git a/tests/api/ai_explain.rs b/tests/api/ai_explain.rs index 8c538ff4..2dbc3364 100644 --- a/tests/api/ai_explain.rs +++ b/tests/api/ai_explain.rs @@ -1,6 +1,5 @@ -use crate::helpers::app::test_app_with_login; +use crate::helpers::app::{drop_stubr, test_app_with_login}; use crate::helpers::db::{get_pool, reset}; -use crate::helpers::wait_for_stubr; use actix_web::test; use anyhow::Error; use diesel::{QueryDsl, RunQueryDsl}; @@ -49,7 +48,6 @@ fn add_explain_cache() -> Result<(), Error> { async fn test_explain() -> Result<(), Error> { let pool = reset()?; add_explain_cache()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await.unwrap(); let service = test::init_service(app).await; let request = test::TestRequest::post() @@ -123,7 +121,6 @@ async fn test_explain() -> Result<(), Error> { .first(&mut conn)?; assert_eq!(row.thumbs_up, 1); assert_eq!(row.thumbs_down, 1); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/api/ai_help.rs b/tests/api/ai_help.rs index 195cee57..d06c7524 100644 --- a/tests/api/ai_help.rs +++ b/tests/api/ai_help.rs @@ -1,7 +1,7 @@ use std::time::Duration; use crate::helpers::api_assertions::assert_ok_with_json_containing; -use crate::helpers::app::init_test; +use crate::helpers::app::{drop_stubr, init_test}; use actix_http::StatusCode; use actix_rt::time::sleep; use anyhow::Error; @@ -58,7 +58,7 @@ async fn test_quota() -> Result<(), Error> { ) .await; assert_eq!(ai_help.status(), StatusCode::PAYMENT_REQUIRED); - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -142,6 +142,6 @@ async fn test_quota_rest() -> Result<(), Error> { assert_eq!(ai_help.status(), StatusCode::NOT_IMPLEMENTED); let quota = client.get("/api/v1/plus/ai/ask/quota", None).await; assert_ok_with_json_containing(quota, json!({"quota": { "count": 1, "limit": 5}})).await; - drop(stubr); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/api/ai_help_history.rs b/tests/api/ai_help_history.rs index 3ddf7fb6..80b20631 100644 --- a/tests/api/ai_help_history.rs +++ b/tests/api/ai_help_history.rs @@ -1,7 +1,6 @@ -use crate::helpers::app::test_app_with_login; +use crate::helpers::app::{drop_stubr, test_app_with_login}; use crate::helpers::db::{get_pool, reset}; use crate::helpers::http_client::TestHttpClient; -use crate::helpers::wait_for_stubr; use actix_web::test; use anyhow::Error; use async_openai::types::ChatCompletionRequestMessage; @@ -64,7 +63,6 @@ fn add_history_log() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn test_history() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await.unwrap(); let service = test::init_service(app).await; let mut logged_in_client = TestHttpClient::new(service).await; @@ -93,8 +91,7 @@ async fn test_history() -> Result<(), Error> { test::read_body(history).await.as_ref() )) ); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/api/auth.rs b/tests/api/auth.rs index 57469cbd..53f3c859 100644 --- a/tests/api/auth.rs +++ b/tests/api/auth.rs @@ -4,14 +4,15 @@ use actix_web::test; use anyhow::Error; use url::Url; -use crate::helpers::{app::test_app_with_login, db::reset, http_client::check_stubr_initialized}; +use crate::helpers::{ + app::{drop_stubr, test_app_with_login}, + db::reset, +}; #[actix_rt::test] #[stubr::mock(port = 4321)] async fn test_next() -> Result<(), Error> { let pool = reset()?; - let _stubr_ok = check_stubr_initialized().await; - let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; @@ -53,5 +54,6 @@ async fn test_next() -> Result<(), Error> { .and_then(|l| l.to_str().ok()), Some("http://localhost:8000/foo") ); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/api/fxa_webhooks.rs b/tests/api/fxa_webhooks.rs index 20182b11..db1178fb 100644 --- a/tests/api/fxa_webhooks.rs +++ b/tests/api/fxa_webhooks.rs @@ -1,12 +1,12 @@ use std::thread; use std::time::Duration; +use crate::helpers::app::drop_stubr; use crate::helpers::app::test_app_with_login; use crate::helpers::db::reset; use crate::helpers::http_client::PostPayload; use crate::helpers::http_client::TestHttpClient; use crate::helpers::read_json; -use crate::helpers::wait_for_stubr; use actix_http::StatusCode; use actix_web::test; use anyhow::anyhow; @@ -19,6 +19,7 @@ use rumba::db::types::FxaEventStatus; use rumba::db::Pool; use serde_json::json; use stubr::{Config, Stubr}; +use tokio::time::sleep; const TEN_MS: std::time::Duration = Duration::from_millis(10); @@ -68,7 +69,6 @@ async fn subscription_state_change_to_10m_test() -> Result<(), Error> { let set_token = include_str!("../data/set_tokens/set_token_subscription_state_change_to_10m.txt"); let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; let mut logged_in_client = TestHttpClient::new(service).await; @@ -111,8 +111,7 @@ async fn subscription_state_change_to_10m_test() -> Result<(), Error> { FxaEvent::SubscriptionStateChange, FxaEventStatus::Ignored, )?; - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -121,7 +120,9 @@ async fn subscription_state_change_to_10m_test() -> Result<(), Error> { async fn subscription_state_change_to_core_test_empty_subscription() -> Result<(), Error> { let set_token = include_str!("../data/set_tokens/set_token_subscription_state_change_to_core.txt"); - subscription_state_change_to_core_test(set_token).await + subscription_state_change_to_core_test(set_token).await?; + drop_stubr(stubr).await; + Ok(()) } #[actix_rt::test] @@ -129,12 +130,13 @@ async fn subscription_state_change_to_core_test_empty_subscription() -> Result<( async fn subscription_state_change_to_core_test_inactive() -> Result<(), Error> { let set_token = include_str!("../data/set_tokens/set_token_subscription_state_change_to_core_inactive.txt"); - subscription_state_change_to_core_test(set_token).await + subscription_state_change_to_core_test(set_token).await?; + drop_stubr(stubr).await; + Ok(()) } async fn subscription_state_change_to_core_test(set_token: &str) -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; let mut logged_in_client = TestHttpClient::new(service).await; @@ -167,14 +169,11 @@ async fn subscription_state_change_to_core_test(set_token: &str) -> Result<(), E FxaEvent::SubscriptionStateChange, FxaEventStatus::Processed, )?; - Ok(()) } #[actix_rt::test] async fn delete_user_test() -> Result<(), Error> { - let set_token = include_str!("../data/set_tokens/set_token_delete_user.txt"); - let pool = reset()?; let stubr = Stubr::start_blocking_with( vec!["tests/stubs", "tests/test_specific_stubs/collections"], Config { @@ -185,7 +184,8 @@ async fn delete_user_test() -> Result<(), Error> { verify: false, }, ); - wait_for_stubr().await?; + let set_token = include_str!("../data/set_tokens/set_token_delete_user.txt"); + let pool = reset()?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; @@ -248,8 +248,7 @@ async fn delete_user_test() -> Result<(), Error> { FxaEvent::DeleteUser, FxaEventStatus::Processed, )?; - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -258,7 +257,6 @@ async fn delete_user_test() -> Result<(), Error> { async fn invalid_set_test() -> Result<(), Error> { let set_token = include_str!("../data/set_tokens/set_token_delete_user_invalid.txt"); let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; let mut logged_in_client = TestHttpClient::new(service).await; @@ -280,12 +278,12 @@ async fn invalid_set_test() -> Result<(), Error> { .select(schema::raw_webhook_events_tokens::token) .first::(&mut conn)?; assert_eq!(failed_token, set_token); - drop(stubr); + drop_stubr(stubr).await; Ok(()) } #[actix_rt::test] -async fn change_profile_test() -> Result<(), Error> { +async fn whoami_test() -> Result<(), Error> { let stubr = Stubr::start_blocking_with( vec!["tests/stubs"], Config { @@ -296,13 +294,14 @@ async fn change_profile_test() -> Result<(), Error> { verify: false, }, ); - wait_for_stubr().await?; - let set_token = include_str!("../data/set_tokens/set_token_profile_change.txt"); let pool = reset()?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; let mut logged_in_client = TestHttpClient::new(service).await; + let res = logged_in_client.trigger_webhook(set_token).await; + assert!(res.response().status().is_success()); + let whoami = logged_in_client .get("/api/v1/whoami", Some(vec![("X-Appengine-Country", "IS")])) .await; @@ -310,8 +309,7 @@ async fn change_profile_test() -> Result<(), Error> { let json = read_json(whoami).await; assert_eq!(json["username"], "TEST_SUB"); assert_eq!(json["email"], "test@test.com"); - - drop(stubr); + drop_stubr(stubr).await; let stubr = Stubr::start_blocking_with( vec!["tests/stubs", "tests/test_specific_stubs/fxa_webhooks"], @@ -323,9 +321,11 @@ async fn change_profile_test() -> Result<(), Error> { verify: false, }, ); - wait_for_stubr().await?; - - thread::sleep(TEN_MS); + let set_token = include_str!("../data/set_tokens/set_token_profile_change.txt"); + let pool = reset()?; + let app = test_app_with_login(&pool).await?; + let service = test::init_service(app).await; + let mut logged_in_client = TestHttpClient::new(service).await; let res = logged_in_client.trigger_webhook(set_token).await; assert!(res.response().status().is_success()); @@ -341,7 +341,7 @@ async fn change_profile_test() -> Result<(), Error> { if json["email"] == "foo@bar.com" { break; } - thread::sleep(TEN_MS); + sleep(TEN_MS).await; tries -= 1; } @@ -355,7 +355,6 @@ async fn change_profile_test() -> Result<(), Error> { FxaEvent::ProfileChange, FxaEventStatus::Processed, )?; - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/api/multiple_collections.rs b/tests/api/multiple_collections.rs index 98f37ac9..553192e8 100644 --- a/tests/api/multiple_collections.rs +++ b/tests/api/multiple_collections.rs @@ -3,7 +3,7 @@ use crate::helpers::api_assertions::{ assert_conflict_with_json_containing, assert_created, assert_created_returning_json, assert_created_with_json_containing, assert_ok, assert_ok_with_json_containing, }; -use crate::helpers::app::init_test; +use crate::helpers::app::{drop_stubr, init_test}; use crate::helpers::http_client::PostPayload; use crate::helpers::read_json; @@ -61,7 +61,7 @@ async fn test_create_and_get_collection() -> Result<(), Error> { ), ) .await; - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -152,8 +152,7 @@ async fn test_add_items_to_collection() -> Result<(), Error> { }), ) .await; - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -216,7 +215,7 @@ async fn test_collection_name_conflicts() -> Result<(), Error> { ) .await; assert_created_with_json_containing(res, json!({"name":"Test 2"})).await; - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -297,7 +296,7 @@ async fn test_collection_item_conflicts() -> Result<(), Error> { }), ) .await; - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -366,7 +365,7 @@ async fn test_edit_item_in_collection() -> Result<(), Error> { }), ) .await; - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -424,8 +423,7 @@ async fn test_delete_item_in_collection() -> Result<(), Error> { json!({"id": collection_1,"article_count": 0, "items": []}), ) .await; - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -510,7 +508,7 @@ async fn test_delete_collection() -> Result<(), Error> { .await; assert_created(res); - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -562,14 +560,13 @@ async fn test_no_modify_delete_default() -> Result<(), Error> { json!({"error": "Cannot delete default collection"}), ) .await; - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } #[actix_rt::test] async fn test_long_collection_name_is_bad_request() -> Result<(), Error> { - let (mut client, _stubr) = + let (mut client, stubr) = init_test(vec!["tests/stubs", "tests/test_specific_stubs/collections"]).await?; let base_url = "/api/v2/collections/"; let two_hundred_twenty = "This is a really long title that has over 1024 characeters 5 times this is 1100. What were we thinking creeating such a long title really? 1024 really is a lot of character. To make this easier let's repeat this 5 times.".to_owned(); @@ -588,12 +585,13 @@ async fn test_long_collection_name_is_bad_request() -> Result<(), Error> { ) .await; assert_bad_request_with_json_containing(res, json!({"code":400,"error":"Validation Error","message":"Error validating input name: 'name' must be between 1 and 1024 chars"})).await; + drop_stubr(stubr).await; Ok(()) } #[actix_rt::test] async fn test_very_long_collection_description_is_bad_request() -> Result<(), Error> { - let (mut client, _stubr) = + let (mut client, stubr) = init_test(vec!["tests/stubs", "tests/test_specific_stubs/collections"]).await?; let base_url = "/api/v2/collections/"; @@ -614,12 +612,13 @@ async fn test_very_long_collection_description_is_bad_request() -> Result<(), Er ) .await; assert_bad_request_with_json_containing(res, json!({"code":400,"error":"Validation Error","message":"Error validating input description: 'description' must not be longer than 65536 chars"})).await; + drop_stubr(stubr).await; Ok(()) } #[actix_rt::test] async fn test_long_collection_item_title_is_bad_request() -> Result<(), Error> { - let (mut client, _stubr) = + let (mut client, stubr) = init_test(vec!["tests/stubs", "tests/test_specific_stubs/collections"]).await?; let base_url = "/api/v2/collections/"; @@ -653,12 +652,13 @@ async fn test_long_collection_item_title_is_bad_request() -> Result<(), Error> { ) .await; assert_bad_request_with_json_containing(res, json!({"code":400,"error":"Validation Error","message":"Error validating input title: 'title' must be between 1 and 1024 chars"})).await; + drop_stubr(stubr).await; Ok(()) } #[actix_rt::test] async fn test_very_long_collection_item_notes_is_bad_request() -> Result<(), Error> { - let (mut client, _stubr) = + let (mut client, stubr) = init_test(vec!["tests/stubs", "tests/test_specific_stubs/collections"]).await?; let base_url = "/api/v2/collections/"; @@ -699,12 +699,13 @@ async fn test_very_long_collection_item_notes_is_bad_request() -> Result<(), Err ) .await; assert_bad_request_with_json_containing(res, json!({"code":400,"error":"Validation Error","message":"Error validating input notes: 'notes' must not be longer than 65536 chars"})).await; + drop_stubr(stubr).await; Ok(()) } #[actix_rt::test] async fn test_collection_subscription_limits() -> Result<(), Error> { - let (mut client, _stubr) = init_test(vec![ + let (mut client, stubr) = init_test(vec![ "tests/stubs", "tests/test_specific_stubs/collections", "tests/test_specific_stubs/collections_core_user", @@ -765,13 +766,13 @@ async fn test_collection_subscription_limits() -> Result<(), Error> { ) .await; assert_created(res); - drop(_stubr); + drop_stubr(stubr).await; Ok(()) } #[actix_rt::test] async fn test_paying_user_no_collection_limit() -> Result<(), Error> { - let (mut client, _stubr) = + let (mut client, stubr) = init_test(vec!["tests/stubs", "tests/test_specific_stubs/collections"]).await?; let base_url = "/api/v2/collections/"; @@ -789,7 +790,7 @@ async fn test_paying_user_no_collection_limit() -> Result<(), Error> { .await; assert_created(res); } - drop(_stubr); + drop_stubr(stubr).await; Ok(()) } #[actix_rt::test] @@ -854,7 +855,7 @@ async fn test_delete_collection_items_also_deleted() -> Result<(), Error> { ) .await; assert_ok_with_json_containing(res, json!({"results": [] })).await; - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -905,13 +906,15 @@ async fn test_create_and_get_many_collections() -> Result<(), Error> { body.as_array().unwrap().iter().reduce(|acc, next| { let t1 = DateTime::parse_from_rfc3339(acc["created_at"].as_str().unwrap()) .unwrap() - .timestamp_nanos(); + .timestamp_nanos_opt() + .unwrap(); let t2 = DateTime::parse_from_rfc3339(next["created_at"].as_str().unwrap()) .unwrap() - .timestamp_nanos(); + .timestamp_nanos_opt() + .unwrap(); assert!(t1 < t2); next }); - drop(stubr); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/api/newsletter.rs b/tests/api/newsletter.rs index 6bdd889b..c4655084 100644 --- a/tests/api/newsletter.rs +++ b/tests/api/newsletter.rs @@ -1,7 +1,7 @@ -use crate::helpers::app::{init_test, test_app_with_login}; +use crate::helpers::app::{drop_stubr, init_test, test_app_with_login}; use crate::helpers::db::reset; use crate::helpers::http_client::TestHttpClient; -use crate::helpers::{read_json, wait_for_stubr}; +use crate::helpers::read_json; use actix_web::test; use anyhow::Error; use serde_json::json; @@ -9,9 +9,8 @@ use stubr::{Config, Stubr}; #[actix_rt::test] #[stubr::mock(port = 4321)] -async fn settings_newsletter_test() -> Result<(), Error> { +async fn settings_newsletter_subscribe() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; let mut logged_in_client = TestHttpClient::new(service).await; @@ -39,8 +38,12 @@ async fn settings_newsletter_test() -> Result<(), Error> { assert_eq!(newsletter.status(), 201); let json = read_json(newsletter).await; assert_eq!(json["subscribed"], true); + drop_stubr(stubr).await; + Ok(()) +} - drop(stubr); +#[actix_rt::test] +async fn settings_newsletter_test() -> Result<(), Error> { let stubr = Stubr::start_blocking_with( vec![ "tests/stubs", @@ -54,7 +57,11 @@ async fn settings_newsletter_test() -> Result<(), Error> { verify: false, }, ); - wait_for_stubr().await?; + let pool = reset()?; + let app = test_app_with_login(&pool).await?; + let service = test::init_service(app).await; + let mut logged_in_client = TestHttpClient::new(service).await; + let newsletter = logged_in_client.get("/api/v1/plus/newsletter/", None).await; assert_eq!(newsletter.status(), 200); @@ -67,8 +74,7 @@ async fn settings_newsletter_test() -> Result<(), Error> { assert!(whoami.response().status().is_success()); let json = read_json(whoami).await; assert_eq!(json["settings"]["mdnplus_newsletter"], true); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -76,7 +82,6 @@ async fn settings_newsletter_test() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn anonymous_newsletter_test() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await.unwrap(); let service = test::init_service(app).await; let request = test::TestRequest::post() @@ -86,8 +91,7 @@ async fn anonymous_newsletter_test() -> Result<(), Error> { let newsletter_res = test::call_service(&service, request).await; assert!(newsletter_res.status().is_success()); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -109,7 +113,6 @@ async fn anonymous_newsletter_error_test() -> Result<(), Error> { let newsletter_res = test::call_service(&service, request).await; assert!(newsletter_res.status().is_server_error()); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/api/ping.rs b/tests/api/ping.rs index 0bf71beb..130e8c21 100644 --- a/tests/api/ping.rs +++ b/tests/api/ping.rs @@ -1,7 +1,6 @@ -use crate::helpers::app::test_app_with_login; +use crate::helpers::app::{drop_stubr, test_app_with_login}; use crate::helpers::db::reset; use crate::helpers::http_client::{PostPayload, TestHttpClient}; -use crate::helpers::wait_for_stubr; use actix_web::test; use anyhow::Error; use diesel::prelude::*; @@ -12,7 +11,6 @@ use serde_json::{json, Value}; #[stubr::mock(port = 4321)] async fn test_empty() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; @@ -28,8 +26,7 @@ async fn test_empty() -> Result<(), Error> { .select(schema::activity_pings::activity) .first::(&mut conn)?; assert_eq!(activity_data, json!({ "subscription_type": "mdn_plus_5m" })); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -37,7 +34,6 @@ async fn test_empty() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn test_garbage() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; @@ -57,8 +53,7 @@ async fn test_garbage() -> Result<(), Error> { .select(schema::activity_pings::activity) .first::(&mut conn)?; assert_eq!(activity_data, json!({ "subscription_type": "mdn_plus_5m" })); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -66,7 +61,6 @@ async fn test_garbage() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn test_offline_disabled() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; @@ -86,8 +80,7 @@ async fn test_offline_disabled() -> Result<(), Error> { .select(schema::activity_pings::activity) .first::(&mut conn)?; assert_eq!(activity_data, json!({ "subscription_type": "mdn_plus_5m" })); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -95,7 +88,6 @@ async fn test_offline_disabled() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn test_offline_enabled() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; @@ -118,8 +110,7 @@ async fn test_offline_enabled() -> Result<(), Error> { activity_data, json!({ "subscription_type": "mdn_plus_5m", "offline": true }) ); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -127,7 +118,6 @@ async fn test_offline_enabled() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn test_offline_upsert() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; @@ -189,7 +179,6 @@ async fn test_offline_upsert() -> Result<(), Error> { .count() .first::(&mut conn)?; assert_eq!(count, 1); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/api/play.rs b/tests/api/play.rs index 74f936f7..aadfba44 100644 --- a/tests/api/play.rs +++ b/tests/api/play.rs @@ -1,7 +1,7 @@ -use crate::helpers::app::test_app_with_login; +use crate::helpers::app::{drop_stubr, test_app_with_login}; use crate::helpers::db::reset; use crate::helpers::http_client::TestHttpClient; -use crate::helpers::{read_json, wait_for_stubr}; +use crate::helpers::read_json; use actix_http::StatusCode; use actix_web::test; use anyhow::Error; @@ -16,7 +16,6 @@ use serde_json::json; #[stubr::mock(port = 4321)] async fn test_playground() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; let mut client = TestHttpClient::new(service).await; @@ -62,6 +61,7 @@ async fn test_playground() -> Result<(), Error> { let playground: PlaygroundQuery = schema::playground::table.first(&mut conn)?; assert_eq!(playground.user_id, None); assert_eq!(playground.deleted_user_id, Some(user_id)); + drop_stubr(stubr).await; Ok(()) } @@ -69,12 +69,12 @@ async fn test_playground() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn test_invalid_id() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; let mut client = TestHttpClient::new(service).await; let res = client.get("/api/v1/play/sssieddidxsx", None).await; // This used to panic, now it should just 400 assert_eq!(res.status(), StatusCode::BAD_REQUEST); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/api/root.rs b/tests/api/root.rs index 693d8f68..2084f143 100644 --- a/tests/api/root.rs +++ b/tests/api/root.rs @@ -1,9 +1,9 @@ use crate::helpers::api_assertions::{ assert_created_with_json_containing, assert_ok_with_json_containing, }; +use crate::helpers::app::drop_stubr; use crate::helpers::db::{get_pool, reset}; use crate::helpers::http_client::TestHttpClient; -use crate::helpers::wait_for_stubr; use crate::helpers::{app::test_app_with_login, http_client::PostPayload}; use actix_http::StatusCode; use actix_web::test; @@ -16,7 +16,6 @@ use serde_json::json; #[stubr::mock(port = 4321)] async fn find_user() -> Result<(), Error> { reset()?; - wait_for_stubr().await?; let pool = get_pool(); let mut conn = pool.get()?; @@ -207,6 +206,6 @@ async fn find_user() -> Result<(), Error> { }), ) .await; - drop(stubr); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/api/search.rs b/tests/api/search.rs index 38a3b985..143cc7a9 100644 --- a/tests/api/search.rs +++ b/tests/api/search.rs @@ -1,5 +1,5 @@ +use crate::helpers::app::{drop_stubr, test_app_only_search}; use crate::helpers::read_json; -use crate::helpers::{app::test_app_only_search, wait_for_stubr}; use actix_http::body::BoxBody; use actix_web::{http::header, test}; use anyhow::Error; @@ -16,12 +16,11 @@ async fn do_request(path: &str) -> Result Result<(), Error> { assert_eq!(json["is_authenticated"], true); assert_eq!(json["is_subscriber"], false); assert_eq!(json["settings"]["no_ads"], false); - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -76,6 +76,6 @@ async fn test_subscriber_settings() -> Result<(), Error> { assert_eq!(json["is_authenticated"], true); assert_eq!(json["is_subscriber"], true); assert_eq!(json["settings"]["no_ads"], true); - drop(stubr); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/api/updates.rs b/tests/api/updates.rs index c5087039..f77c07fb 100644 --- a/tests/api/updates.rs +++ b/tests/api/updates.rs @@ -1,10 +1,10 @@ use std::fs; use std::time::Duration; -use crate::helpers::app::test_app_with_login; +use crate::helpers::app::{drop_stubr, test_app_with_login}; use crate::helpers::db::reset; use crate::helpers::http_client::{PostPayload, TestHttpClient}; -use crate::helpers::{read_json, wait_for_stubr}; +use crate::helpers::read_json; use actix_rt::time::{sleep, timeout}; use actix_web::dev::Service; use actix_web::test; @@ -19,7 +19,6 @@ use stubr::{Config, Stubr}; macro_rules! test_setup { () => {{ - let mut pool = reset()?; let stubr = Stubr::start_blocking_with( vec!["tests/stubs", "tests/test_specific_stubs/bcd_updates"], Config { @@ -30,7 +29,7 @@ macro_rules! test_setup { verify: false, }, ); - wait_for_stubr().await?; + let mut pool = reset()?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; let mut logged_in_client = TestHttpClient::new(service).await; @@ -41,7 +40,7 @@ macro_rules! test_setup { #[actix_rt::test] async fn test_bcd_updates_basic_pagination() -> Result<(), Error> { - let (mut logged_in_client, _stubr) = test_setup!(); + let (mut logged_in_client, stubr) = test_setup!(); let res = logged_in_client.get("/api/v2/updates/", None).await; let json = read_json(res).await; // 3 browsers x 3 browser releases = 9 total rows, (5 per page so 2 pages total) @@ -134,12 +133,13 @@ async fn test_bcd_updates_basic_pagination() -> Result<(), Error> { } } ))); + drop_stubr(stubr).await; Ok(()) } #[actix_rt::test] async fn test_bcd_updates_filter_by_collections() -> Result<(), Error> { - let (mut logged_in_client, _stubr) = test_setup!(); + let (mut logged_in_client, stubr) = test_setup!(); let res = logged_in_client.get("/api/v2/collections/", None).await; let vals = read_json(res).await; let default_id = &vals.as_array().unwrap()[0]["id"]; @@ -159,6 +159,7 @@ async fn test_bcd_updates_filter_by_collections() -> Result<(), Error> { .expect("Json snapshot opened"); let snapshot_json: Value = serde_json::from_reader(file).expect("Error reading snapshot json"); assert_eq!(res_json, snapshot_json); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/api/whoami.rs b/tests/api/whoami.rs index d40a34d9..bfe25a44 100644 --- a/tests/api/whoami.rs +++ b/tests/api/whoami.rs @@ -1,7 +1,8 @@ +use crate::helpers::app::drop_stubr; use crate::helpers::db::reset; use crate::helpers::http_client::TestHttpClient; +use crate::helpers::read_json; use crate::helpers::{app::test_app_with_login, http_client::PostPayload}; -use crate::helpers::{read_json, wait_for_stubr}; use actix_web::test; use anyhow::Error; use serde_json::json; @@ -11,7 +12,6 @@ use stubr::{Config, Stubr}; #[stubr::mock(port = 4321)] async fn whoami_anonymous_test() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await.unwrap(); let service = test::init_service(app).await; let request = test::TestRequest::get().uri("/api/v1/whoami").to_request(); @@ -22,7 +22,7 @@ async fn whoami_anonymous_test() -> Result<(), Error> { let json = read_json(whoami).await; assert_eq!(json["geo"]["country"], "Unknown"); assert_eq!(json["geo"]["country_iso"], "ZZ"); - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -30,7 +30,6 @@ async fn whoami_anonymous_test() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn whoami_anonymous_test_gcp() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await.unwrap(); let service = test::init_service(app).await; let request = test::TestRequest::get() @@ -44,7 +43,7 @@ async fn whoami_anonymous_test_gcp() -> Result<(), Error> { let json = read_json(whoami).await; assert_eq!(json["geo"]["country"], "Iceland"); assert_eq!(json["geo"]["country_iso"], "IS"); - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -52,7 +51,6 @@ async fn whoami_anonymous_test_gcp() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn whoami_anonymous_test_aws() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await.unwrap(); let service = test::init_service(app).await; let request = test::TestRequest::get() @@ -66,7 +64,7 @@ async fn whoami_anonymous_test_aws() -> Result<(), Error> { let json = read_json(whoami).await; assert_eq!(json["geo"]["country"], "Iceland"); assert_eq!(json["geo"]["country_iso"], "IS"); - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -74,7 +72,6 @@ async fn whoami_anonymous_test_aws() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn whoami_legacy_logged_in_test() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; let mut logged_in_client = TestHttpClient::new(service).await; @@ -102,8 +99,7 @@ async fn whoami_legacy_logged_in_test() -> Result<(), Error> { assert_eq!(json["geo"]["country_iso"], "IS"); // old sessions do not work anymore assert!(json["username"].is_null()); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -111,7 +107,6 @@ async fn whoami_legacy_logged_in_test() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn whoami_logged_in_test() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; let mut logged_in_client = TestHttpClient::new(service).await; @@ -135,7 +130,7 @@ async fn whoami_logged_in_test() -> Result<(), Error> { json["subscription_type"], "mdn_plus_5m", "Subscription type wrong" ); - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -143,7 +138,6 @@ async fn whoami_logged_in_test() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn whoami_settings_test() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; let mut logged_in_client = TestHttpClient::new(service).await; @@ -186,15 +180,12 @@ async fn whoami_settings_test() -> Result<(), Error> { let json = read_json(whoami).await; assert_eq!(json["settings"]["locale_override"], "zh-TW"); assert_eq!(json["settings"]["mdnplus_newsletter"], false); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } #[actix_rt::test] async fn whoami_multiple_subscriptions_test() -> Result<(), Error> { - let pool = reset()?; - let stubr = Stubr::start_blocking_with( vec!["tests/stubs", "tests/test_specific_stubs/whoami"], Config { @@ -205,7 +196,7 @@ async fn whoami_multiple_subscriptions_test() -> Result<(), Error> { verify: false, }, ); - wait_for_stubr().await?; + let pool = reset()?; let app = test_app_with_login(&pool).await?; let service = test::init_service(app).await; @@ -227,6 +218,6 @@ async fn whoami_multiple_subscriptions_test() -> Result<(), Error> { ); assert_eq!(json["is_subscriber"], true); assert_eq!(json["subscription_type"], "mdn_plus_5y"); - drop(stubr); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/fxa/callback.rs b/tests/fxa/callback.rs index c62e9268..6bd4c464 100644 --- a/tests/fxa/callback.rs +++ b/tests/fxa/callback.rs @@ -5,9 +5,8 @@ use std::collections::HashMap; use url::Url; -use crate::helpers::app::test_app_with_login; +use crate::helpers::app::{drop_stubr, test_app_with_login}; use crate::helpers::db::reset; -use crate::helpers::wait_for_stubr; #[actix_rt::test] #[stubr::mock(port = 4321)] @@ -49,8 +48,7 @@ async fn basic() -> Result<(), Error> { let res = test::call_service(&app, base.to_request()).await; assert!(res.status().is_redirection()); assert_eq!(res.headers().get("Location").unwrap(), "/"); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -58,7 +56,6 @@ async fn basic() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn next() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await.unwrap(); let app = test::init_service(app).await; @@ -98,8 +95,7 @@ async fn next() -> Result<(), Error> { res.headers().get("Location").unwrap(), "http://localhost:8000/foo" ); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -107,7 +103,6 @@ async fn next() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn next_absolute() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await.unwrap(); let app = test::init_service(app).await; @@ -144,8 +139,7 @@ async fn next_absolute() -> Result<(), Error> { let res = test::call_service(&app, base.to_request()).await; assert!(res.status().is_redirection()); assert_eq!(res.headers().get("Location").unwrap(), "/"); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -153,7 +147,6 @@ async fn next_absolute() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn next_absolute_without_protocol() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await.unwrap(); let app = test::init_service(app).await; @@ -190,8 +183,7 @@ async fn next_absolute_without_protocol() -> Result<(), Error> { let res = test::call_service(&app, base.to_request()).await; assert!(res.status().is_redirection()); assert_eq!(res.headers().get("Location").unwrap(), "/"); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -199,7 +191,6 @@ async fn next_absolute_without_protocol() -> Result<(), Error> { #[stubr::mock(port = 4321)] async fn next_absolute_with_path_exploit() -> Result<(), Error> { let pool = reset()?; - wait_for_stubr().await?; let app = test_app_with_login(&pool).await.unwrap(); let app = test::init_service(app).await; @@ -239,8 +230,7 @@ async fn next_absolute_with_path_exploit() -> Result<(), Error> { res.headers().get("Location").unwrap(), "http://localhost:8000//foo.com/bar" ); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } @@ -294,7 +284,6 @@ async fn no_prompt() -> Result<(), Error> { res.headers().get("Location").unwrap(), "http://localhost:8000/foo" ); - - drop(stubr); + drop_stubr(stubr).await; Ok(()) } diff --git a/tests/helpers/app.rs b/tests/helpers/app.rs index c8bcf829..de47f29b 100644 --- a/tests/helpers/app.rs +++ b/tests/helpers/app.rs @@ -1,3 +1,5 @@ +use std::time::Duration; + use actix_http::body::BoxBody; use actix_http::Request; use actix_identity::IdentityMiddleware; @@ -25,6 +27,7 @@ use rumba::fxa::LoginManager; use rumba::settings::SETTINGS; use slog::{slog_o, Drain}; use stubr::{Config, Stubr}; +use tokio::time::sleep; use super::db::reset; use super::http_client::TestHttpClient; @@ -160,3 +163,13 @@ pub async fn init_test( let logged_in_client = TestHttpClient::new(service).await; Ok((logged_in_client, stubr)) } + +// We drop the stubr instances explicitly and add a small +// delay to give it time to shutdown. This is necessary to fix +// flaky tests that fail because leftover stubr instances are still +// holding on the network port in some cases. +const STUBR_POST_DROP_SLEEP: std::time::Duration = Duration::from_millis(10); +pub async fn drop_stubr(stubr: Stubr) { + drop(stubr); + sleep(STUBR_POST_DROP_SLEEP).await; +} diff --git a/tests/helpers/http_client.rs b/tests/helpers/http_client.rs index cd79db5c..43a5359d 100644 --- a/tests/helpers/http_client.rs +++ b/tests/helpers/http_client.rs @@ -6,8 +6,6 @@ use actix_web::{test, Error}; use rumba::settings::SETTINGS; use std::collections::HashMap; -use reqwest::{Client, Method, StatusCode}; - use serde_json::Value; use url::Url; @@ -25,8 +23,6 @@ pub enum PostPayload { impl> TestHttpClient { pub async fn new(service: T) -> Self { - let _stubr_ok = check_stubr_initialized().await; - let login_req = test::TestRequest::get() .uri("/users/fxa/login/authenticate/") .to_request(); @@ -155,13 +151,3 @@ impl> TestHttpC base } } - -pub async fn check_stubr_initialized() -> Result<(), ()> { - //Hardcoded for now. We will 'always' spin stubr at localhost:4321. - let res = Client::new() - .request(Method::GET, "http://localhost:4321/healthz") - .send() - .await; - assert_eq!(res.unwrap().status(), StatusCode::OK); - Ok(()) -} diff --git a/tests/helpers/mod.rs b/tests/helpers/mod.rs index f83d4106..6ec15c00 100644 --- a/tests/helpers/mod.rs +++ b/tests/helpers/mod.rs @@ -1,13 +1,7 @@ -use std::time::Duration; - use actix_http::body::{BoxBody, EitherBody, MessageBody}; -use actix_rt::{ - net::TcpStream, - time::{sleep, timeout}, -}; + use actix_web::dev::ServiceResponse; use actix_web::test; -use anyhow::{anyhow, Error}; use serde_json::Value; pub mod api_assertions; @@ -20,16 +14,3 @@ pub type RumbaTestResponse = ServiceResponse>; pub async fn read_json(res: ServiceResponse) -> Value { serde_json::from_slice(test::read_body(res).await.as_ref()).unwrap() } - -pub async fn wait_for_stubr() -> Result<(), Error> { - timeout(Duration::from_millis(10_000), async { - while let Err(_e) = TcpStream::connect(("127.0.0.1", 4321)).await { - sleep(Duration::from_millis(100)).await; - } - Ok::<(), Error>(()) - }) - .await - .map_err(|_| anyhow!("strubr not ready after 10,000ms"))??; - - Ok(()) -}