From a70fe5466866fb86227e3d6f4141adedd2a5b600 Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Fri, 17 Nov 2023 10:30:25 -0500 Subject: [PATCH] test: use mockito crate to test network-interfacing code (#106) * test: use mockito crate to test network-interfacing code * tweak --- library/Cargo.toml | 1 + library/src/config.rs | 6 +++++ library/src/network.rs | 34 ++--------------------- library/src/updater.rs | 61 +++++++++++++++++++++--------------------- 4 files changed, 39 insertions(+), 63 deletions(-) diff --git a/library/Cargo.toml b/library/Cargo.toml index 7c74b43..a05a0ee 100644 --- a/library/Cargo.toml +++ b/library/Cargo.toml @@ -66,6 +66,7 @@ simple-logging = "2.0.2" [dev-dependencies] mockall = "0.11.4" +mockito = "1.2.0" # Gives #[serial] attribute for locking all of our shorebird_init # tests to a single thread so they don't conflict with each other. serial_test = "2.0.0" diff --git a/library/src/config.rs b/library/src/config.rs index da9fb59..e1bfae9 100644 --- a/library/src/config.rs +++ b/library/src/config.rs @@ -16,7 +16,13 @@ use std::println as debug; // Workaround to use println! for logs. // cbindgen looks for const, ignore these so it doesn't warn about them. /// cbindgen:ignore +#[cfg(test)] +const DEFAULT_BASE_URL: &str = "DEFAULT_BASE_URL should be mocked using mockito::Server"; + +/// cbindgen:ignore +#[cfg(not(test))] const DEFAULT_BASE_URL: &str = "https://api.shorebird.dev"; + /// cbindgen:ignore const DEFAULT_CHANNEL: &str = "stable"; diff --git a/library/src/network.rs b/library/src/network.rs index 40855dd..f06ef34 100644 --- a/library/src/network.rs +++ b/library/src/network.rs @@ -50,26 +50,7 @@ impl core::fmt::Debug for NetworkHooks { } } -#[cfg(test)] -fn patch_check_request_throws( - _url: &str, - _request: PatchCheckRequest, -) -> anyhow::Result { - bail!("please set a patch_check_request_fn"); -} - -#[cfg(test)] -fn download_file_throws(_url: &str) -> anyhow::Result> { - bail!("please set a download_file_fn"); -} - -#[cfg(test)] -pub fn report_event_throws(_url: &str, _request: CreatePatchEventRequest) -> anyhow::Result<()> { - bail!("please set a report_event_fn"); -} - impl Default for NetworkHooks { - #[cfg(not(test))] fn default() -> Self { Self { patch_check_request_fn: patch_check_request_default, @@ -77,18 +58,8 @@ impl Default for NetworkHooks { report_event_fn: report_event_default, } } - - #[cfg(test)] - fn default() -> Self { - Self { - patch_check_request_fn: patch_check_request_throws, - download_file_fn: download_file_throws, - report_event_fn: report_event_throws, - } - } } -#[cfg(not(test))] pub fn patch_check_request_default( url: &str, request: PatchCheckRequest, @@ -99,7 +70,6 @@ pub fn patch_check_request_default( Ok(response) } -#[cfg(not(test))] pub fn download_file_default(url: &str) -> anyhow::Result> { let client = reqwest::blocking::Client::new(); let result = client.get(url).send(); @@ -166,7 +136,7 @@ pub fn testing_set_network_hooks( }); } -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, Serialize)] pub struct Patch { /// The patch number. Starts at 1 for each new release and increases /// monotonically. @@ -218,7 +188,7 @@ pub struct CreatePatchEventRequest { event: PatchEvent, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Deserialize, Serialize)] pub struct PatchCheckResponse { pub patch_available: bool, #[serde(default)] diff --git a/library/src/updater.rs b/library/src/updater.rs index 32f0c0f..83678f7 100644 --- a/library/src/updater.rs +++ b/library/src/updater.rs @@ -493,9 +493,14 @@ mod tests { use crate::config::testing_reset_config; - fn init_for_testing(tmp_dir: &TempDir) { + fn init_for_testing(tmp_dir: &TempDir, base_url: Option<&str>) { testing_reset_config(); let cache_dir = tmp_dir.path().to_str().unwrap().to_string(); + let mut yaml = "app_id: 1234".to_string(); + if let Some(url) = base_url { + yaml += &format!("\nbase_url: {}", url); + } + crate::init( crate::AppConfig { app_storage_dir: cache_dir.clone(), @@ -503,7 +508,7 @@ mod tests { release_version: "1.0.0+1".to_string(), original_libapp_paths: vec!["/dir/lib/arch/libapp.so".to_string()], }, - "app_id: 1234", + &yaml, ) .unwrap(); } @@ -512,7 +517,7 @@ mod tests { #[test] fn ignore_version_after_marked_bad() { let tmp_dir = TempDir::new("example").unwrap(); - init_for_testing(&tmp_dir); + init_for_testing(&tmp_dir, None); use crate::cache::{PatchInfo, UpdaterState}; use crate::config::with_config; @@ -612,7 +617,7 @@ mod tests { #[test] fn report_launch_result_with_no_current_patch() { let tmp_dir = TempDir::new("example").unwrap(); - init_for_testing(&tmp_dir); + init_for_testing(&tmp_dir, None); assert_eq!( crate::report_launch_failure() .unwrap_err() @@ -629,7 +634,7 @@ mod tests { use crate::cache::{PatchInfo, UpdaterState}; use crate::config::with_config; let tmp_dir = TempDir::new("example").unwrap(); - init_for_testing(&tmp_dir); + init_for_testing(&tmp_dir, None); // Install a fake patch. with_config(|config| { @@ -686,7 +691,7 @@ mod tests { use crate::cache::{PatchInfo, UpdaterState}; use crate::config::with_config; let tmp_dir = TempDir::new("example").unwrap(); - init_for_testing(&tmp_dir); + init_for_testing(&tmp_dir, None); // Install a fake patch. with_config(|config| { @@ -749,9 +754,25 @@ mod tests { use crate::cache::UpdaterState; use crate::config::{current_arch, current_platform, with_config}; use crate::events::{EventType, PatchEvent}; - use crate::network::{testing_set_network_hooks, PatchCheckResponse}; + use crate::network::PatchCheckResponse; + + let mut server = mockito::Server::new(); + let check_response = PatchCheckResponse { + patch_available: false, + patch: None, + }; + let check_response_body = serde_json::to_string(&check_response).unwrap(); + let _ = server + .mock("POST", "/api/v1/patches/check") + .with_status(200) + .with_body(check_response_body) + .create(); + let event_mock = server + .mock("POST", "/api/v1/patches/events") + .with_status(201) + .create(); let tmp_dir = TempDir::new("example").unwrap(); - init_for_testing(&tmp_dir); + init_for_testing(&tmp_dir, Some(&server.url())); with_config(|config| { let mut state = @@ -775,31 +796,9 @@ mod tests { }) .unwrap(); - // TODO(eseidel): Count the number of events sent. - // let mut event_call_count = 0; - // set up the network hooks to return a patch. - testing_set_network_hooks( - |_url, _request| { - Ok(PatchCheckResponse { - patch_available: false, - patch: None, - }) - }, - |_url| { - // Never called. - Ok(Vec::new()) - }, - // I can't actually count the number of times this is called - // without making this a closure, or refactoring NetworkHooks - // to be a trait. - |_url, _event| { - // event_call_count += 1; - Ok(()) - }, - ); super::update().unwrap(); // Only 3 events should have been sent. - // assert_eq!(event_call_count, 3); + event_mock.expect(3); with_config(|config| { let state =