From ed2cae122e2f3d852caa5f320ec67eb3424885ca Mon Sep 17 00:00:00 2001 From: Reactor Scram Date: Mon, 5 Feb 2024 12:49:28 -0600 Subject: [PATCH] feat(windows): add smoke test subcommand (#3541) Part of #3534 This PR creates the subcommand, which you can run locally, but it doesn't run it in `ci.yml` yet. That's in #3542 --------- Signed-off-by: Reactor Scram Co-authored-by: Gabi --- rust/windows-client/src-tauri/src/client.rs | 55 +++++++++---- .../src-tauri/src/client/gui.rs | 80 +++++++++++++++++-- 2 files changed, 114 insertions(+), 21 deletions(-) diff --git a/rust/windows-client/src-tauri/src/client.rs b/rust/windows-client/src-tauri/src/client.rs index ed80a66e9..8e2603b90 100644 --- a/rust/windows-client/src-tauri/src/client.rs +++ b/rust/windows-client/src-tauri/src/client.rs @@ -96,6 +96,18 @@ pub(crate) fn run() -> Result<()> { rt.block_on(deep_link::open(&deep_link.url))?; Ok(()) } + Some(Cmd::SmokeTest) => { + let result = gui::run(&cli); + if let Err(error) = &result { + // In smoke-test mode, don't show the dialog, since it might be running + // unattended in CI and the dialog would hang forever + + // Because of , + // errors returned from `gui::run` may not be logged correctly + tracing::error!(?error, "gui::run error"); + } + Ok(result?) + } } } @@ -103,30 +115,32 @@ pub(crate) fn run() -> Result<()> { /// /// Automatically logs or shows error dialogs for important user-actionable errors fn run_gui(cli: Cli) -> Result<()> { - let result = gui::run(cli); + let result = gui::run(&cli); // Make sure errors get logged, at least to stderr if let Err(error) = &result { tracing::error!(?error, "gui::run error"); - let error_msg = match &error { - gui::Error::WebViewNotInstalled => "Firezone cannot start because WebView2 is not installed. Follow the instructions at .".to_string(), - // TODO: Wording - gui::Error::DeepLink(deep_link::Error::CantListen) => "Firezone is already running. If it's not responding, force-stop it.".to_string(), - error => format!("{}", error), - }; - - native_dialog::MessageDialog::new() - .set_title("Firezone Error") - .set_text(&error_msg) - .set_type(native_dialog::MessageType::Error) - .show_alert()?; + show_error_dialog(error)?; } - // `Error` refers to Tauri types, so it shouldn't be used in main. - // Make it into an anyhow. Ok(result?) } +fn show_error_dialog(error: &gui::Error) -> Result<()> { + let error_msg = match error { + gui::Error::WebViewNotInstalled => "Firezone cannot start because WebView2 is not installed. Follow the instructions at .".to_string(), + gui::Error::DeepLink(deep_link::Error::CantListen) => "Firezone is already running. If it's not responding, force-stop it.".to_string(), + error => error.to_string(), + }; + + native_dialog::MessageDialog::new() + .set_title("Firezone Error") + .set_text(&error_msg) + .set_type(native_dialog::MessageType::Error) + .show_alert()?; + Ok(()) +} + /// The debug / test flags like `crash_on_purpose` and `test_update_notification` /// don't propagate when we use `RunAs` to elevate ourselves. So those must be run /// from an admin terminal, or with "Run as administrator" in the right-click menu. @@ -141,6 +155,9 @@ struct Cli { /// If true, purposely crash the program to test the crash handler #[arg(long, hide = true)] crash_on_purpose: bool, + /// If true, `gui::run` returns an error on purpose to test the error logging and dialog + #[arg(long, hide = true)] + error_on_purpose: bool, /// If true, slow down I/O operations to test how the GUI handles slow I/O #[arg(long, hide = true)] inject_faults: bool, @@ -160,6 +177,14 @@ pub enum Cmd { }, Elevated, OpenDeepLink(DeepLink), + /// SmokeTest gets its own subcommand because elevating would start a new process and trash the exit code + /// + /// We could solve that by keeping the un-elevated process around, blocking on the elevated + /// child process, but then we'd always have an extra process hanging around. + /// + /// It's also invalid for release builds, because we build the exe as a GUI app, + /// so Windows won't give us a valid exit code, it'll just detach from the terminal instantly. + SmokeTest, } #[derive(Args)] diff --git a/rust/windows-client/src-tauri/src/client/gui.rs b/rust/windows-client/src-tauri/src/client/gui.rs index c0fede21c..9f1d154d7 100644 --- a/rust/windows-client/src-tauri/src/client/gui.rs +++ b/rust/windows-client/src-tauri/src/client/gui.rs @@ -57,6 +57,8 @@ pub(crate) enum Error { ClickableNotification(String), #[error("Deep-link module error: {0}")] DeepLink(#[from] deep_link::Error), + #[error("Fake error for testing")] + Fake, #[error("Can't show log filter error dialog: {0}")] LogFilterErrorDialog(native_dialog::Error), #[error("Logging module error: {0}")] @@ -74,7 +76,7 @@ pub(crate) enum Error { } /// Runs the Tauri GUI and returns on exit or unrecoverable error -pub(crate) fn run(cli: client::Cli) -> Result<(), Error> { +pub(crate) fn run(cli: &client::Cli) -> Result<(), Error> { let advanced_settings = settings::load_advanced_settings().unwrap_or_default(); // If the log filter is unparsable, show an error and use the default @@ -100,8 +102,8 @@ pub(crate) fn run(cli: client::Cli) -> Result<(), Error> { }; // Start logging - // TODO: After lands, try using an - // Arc to keep the file logger alive even if Tauri bails out + // TODO: Try using an Arc to keep the file logger alive even if Tauri bails out + // That may fix let logging_handles = client::logging::setup(&advanced_settings.log_filter)?; tracing::info!("started log"); tracing::info!("GIT_VERSION = {}", crate::client::GIT_VERSION); @@ -125,10 +127,10 @@ pub(crate) fn run(cli: client::Cli) -> Result<(), Error> { let notify_controller = Arc::new(Notify::new()); if cli.crash_on_purpose { - tokio::spawn(async { + tokio::spawn(async move { let delay = 10; tracing::info!("Will crash on purpose in {delay} seconds to test crash handling."); - tokio::time::sleep(std::time::Duration::from_secs(delay)).await; + tokio::time::sleep(Duration::from_secs(delay)).await; tracing::info!("Crashing on purpose because of `--crash-on-purpose` flag"); // SAFETY: Crashing is unsafe @@ -146,6 +148,16 @@ pub(crate) fn run(cli: client::Cli) -> Result<(), Error> { } }); + if let Some(client::Cmd::SmokeTest) = &cli.command { + let ctlr_tx = ctlr_tx.clone(); + tokio::spawn(async move { + if let Err(error) = smoke_test(ctlr_tx).await { + tracing::error!(?error, "Error during smoke test"); + std::process::exit(1); + } + }); + } + // Make sure we're single-instance // We register our deep links to call the `open-deep-link` subcommand, // so if we're at this point, we know we've been launched manually @@ -252,6 +264,10 @@ pub(crate) fn run(cli: client::Cli) -> Result<(), Error> { } }; + if cli.error_on_purpose { + return Err(Error::Fake); + } + app.run(|_app_handle, event| { if let tauri::RunEvent::ExitRequested { api, .. } = event { // Don't exit if we close our main window @@ -263,6 +279,54 @@ pub(crate) fn run(cli: client::Cli) -> Result<(), Error> { Ok(()) } +/// Runs a smoke test and then asks Controller to exit gracefully +/// +/// You can purposely fail this test by deleting the exported zip file during +/// the 10-second sleep. +async fn smoke_test(ctlr_tx: CtlrTx) -> Result<()> { + let delay = 10; + tracing::info!("Will quit on purpose in {delay} seconds as part of the smoke test."); + let quit_time = tokio::time::Instant::now() + Duration::from_secs(delay); + + // Test log exporting + let path = connlib_shared::windows::app_local_data_dir()? + .join("data") + .join("smoke_test_log_export.zip"); + let stem = "connlib-smoke-test".into(); + match tokio::fs::remove_file(&path).await { + Ok(()) => {} + Err(error) => { + if error.kind() != std::io::ErrorKind::NotFound { + bail!("Error while removing old zip file") + } + } + } + ctlr_tx + .send(ControllerRequest::ExportLogs { + path: path.clone(), + stem, + }) + .await?; + + // Give the app some time to export the zip and reach steady state + tokio::time::sleep_until(quit_time).await; + + // Check results of tests + let zip_len = tokio::fs::metadata(&path).await?.len(); + if zip_len == 0 { + bail!("Exported log zip has 0 bytes"); + } + tokio::fs::remove_file(&path).await?; + tracing::info!(?path, ?zip_len, "Exported log zip looks okay"); + + tracing::info!("Quitting on purpose because of `smoke-test` subcommand"); + ctlr_tx + .send(ControllerRequest::SystemTrayMenu(TrayMenuEvent::Quit)) + .await?; + + Ok::<_, anyhow::Error>(()) +} + async fn check_for_updates(ctlr_tx: CtlrTx, always_show_update_notification: bool) -> Result<()> { let release = client::updates::check() .await @@ -317,7 +381,11 @@ fn handle_system_tray_event(app: &tauri::AppHandle, event: TrayMenuEvent) -> Res pub(crate) enum ControllerRequest { Disconnected, DisconnectedTokenExpired, - ExportLogs { path: PathBuf, stem: PathBuf }, + /// The same as the arguments to `client::logging::export_logs_to` + ExportLogs { + path: PathBuf, + stem: PathBuf, + }, GetAdvancedSettings(oneshot::Sender), SchemeRequest(url::Url), SystemTrayMenu(TrayMenuEvent),