diff --git a/test/test-manager/src/tests/account.rs b/test/test-manager/src/tests/account.rs index 92c95346b286..78eb42adc4f8 100644 --- a/test/test-manager/src/tests/account.rs +++ b/test/test-manager/src/tests/account.rs @@ -324,7 +324,7 @@ pub async fn test_automatic_wireguard_rotation( .pubkey; // Stop daemon - rpc.set_mullvad_daemon_service_state(false) + rpc.stop_mullvad_daemon() .await .expect("Could not stop system service"); @@ -334,7 +334,7 @@ pub async fn test_automatic_wireguard_rotation( .expect("Could not change device.json to have an old created timestamp"); // Start daemon - rpc.set_mullvad_daemon_service_state(true) + rpc.start_mullvad_daemon() .await .expect("Could not start system service"); diff --git a/test/test-manager/src/tests/tunnel_state.rs b/test/test-manager/src/tests/tunnel_state.rs index 90a5086863da..9ae817c7ee9d 100644 --- a/test/test-manager/src/tests/tunnel_state.rs +++ b/test/test-manager/src/tests/tunnel_state.rs @@ -1,6 +1,6 @@ use super::helpers::{ - self, connect_and_wait, disconnect_and_wait, get_tunnel_state, send_guest_probes, - set_relay_settings, unreachable_wireguard_tunnel, wait_for_tunnel_state, + self, connect_and_wait, get_tunnel_state, send_guest_probes, set_relay_settings, + unreachable_wireguard_tunnel, wait_for_tunnel_state, }; use super::{ui, Error, TestContext}; use crate::assert_tunnel_state; @@ -342,18 +342,18 @@ pub async fn test_connected_state( pub async fn test_connecting_state_when_corrupted_state_cache( _: TestContext, rpc: ServiceClient, - mut mullvad_client: ManagementServiceClient, + mullvad_client: ManagementServiceClient, ) -> Result<(), Error> { - // Enter the disconnected state. Normally this would be preserved when - // restarting the app, i.e. the user would still be disconnected after a - // successfull restart. However, as we will intentionally corrupt the state - // target cache the user should end up in the connecting/connected state, - // *not in the disconnected state, upon restart. - disconnect_and_wait(&mut mullvad_client).await?; + // The test should start in a disconnected state. Normally this would be + // preserved when restarting the app, i.e. the user would still be + // disconnected after a successfull restart. However, as we will + // intentionally corrupt the state target cache the user should end up in + // the connecting/connected state, *not in the disconnected state*, upon + // restart. // Stopping the app should write to the state target cache. log::info!("Stopping the app"); - rpc.stop_app().await?; + rpc.stop_mullvad_daemon().await?; // Intentionally corrupt the state cache. Note that we can not simply remove // the cache, as this will put the app in the default target state which is @@ -373,31 +373,16 @@ pub async fn test_connecting_state_when_corrupted_state_cache( // Start the app & make sure that we start in the 'connecting state'. The // side-effect of this is that no network traffic is allowed to leak. log::info!("Starting the app back up again"); - rpc.start_app().await?; - - let new_state = wait_for_tunnel_state(mullvad_client.clone(), |state| { - matches!( - state, - TunnelState::Connecting { .. } | TunnelState::Connected { .. } | TunnelState::Error(..) - ) - }) - .await - .map_err(|err| { - log::error!("App did not start in an expected state."); - err - })?; - - assert!( - matches!( - new_state, - TunnelState::Connecting { .. } | TunnelState::Connected { .. } - ), - "App is not in either `Connecting` or `Connected` state after starting with corrupt state cache! There is a possibility of leaks during app startup" - ); - - log::info!( - "App started successfully! It successfully recovered from a corrupt tunnel state cache." - ); + rpc.start_mullvad_daemon().await?; + wait_for_tunnel_state(mullvad_client.clone(), |state| !state.is_disconnected()) + .await + .map_err(|err| { + log::error!("App did not start in an expected state. \ + App is not in either `Connecting` or `Connected` state after starting with corrupt state cache! \ + There is a possibility of leaks during app startup "); + err + })?; + log::info!("App successfully recovered from a corrupt tunnel state cache."); Ok(()) } diff --git a/test/test-rpc/src/client.rs b/test/test-rpc/src/client.rs index 6234db0262ae..29f86b944370 100644 --- a/test/test-rpc/src/client.rs +++ b/test/test-rpc/src/client.rs @@ -221,8 +221,19 @@ impl ServiceClient { .await? } - pub async fn restart_app(&self) -> Result<(), Error> { - let _ = self.client.restart_app(tarpc::context::current()).await?; + /// Restarts the app. + /// + /// Shuts down a running app, making it disconnect from any current tunnel + /// connection before starting the app again. + /// + /// # Note + /// This function will return *after* the app is running again, thus + /// blocking execution until then. + pub async fn restart_mullvad_daemon(&self) -> Result<(), Error> { + let _ = self + .client + .restart_mullvad_daemon(tarpc::context::current()) + .await?; Ok(()) } @@ -234,18 +245,24 @@ impl ServiceClient { /// # Note /// This function will return *after* the app has been stopped, thus /// blocking execution until then. - pub async fn stop_app(&self) -> Result<(), Error> { - let _ = self.client.stop_app(tarpc::context::current()).await?; + pub async fn stop_mullvad_daemon(&self) -> Result<(), Error> { + let _ = self + .client + .stop_mullvad_daemon(tarpc::context::current()) + .await?; Ok(()) } /// Start the app. /// /// # Note - /// This function will return *after* the app has been start, thus + /// This function will return *after* the app has been started, thus /// blocking execution until then. - pub async fn start_app(&self) -> Result<(), Error> { - let _ = self.client.start_app(tarpc::context::current()).await?; + pub async fn start_mullvad_daemon(&self) -> Result<(), Error> { + let _ = self + .client + .start_mullvad_daemon(tarpc::context::current()) + .await?; Ok(()) } @@ -313,23 +330,6 @@ impl ServiceClient { Ok(()) } - pub async fn set_mullvad_daemon_service_state(&self, on: bool) -> Result<(), Error> { - self.client - .set_mullvad_daemon_service_state(tarpc::context::current(), on) - .await??; - - self.mullvad_daemon_wait_for_state(|state| { - if on { - state == ServiceStatus::Running - } else { - state == ServiceStatus::NotRunning - } - }) - .await?; - - Ok(()) - } - pub async fn make_device_json_old(&self) -> Result<(), Error> { self.client .make_device_json_old(tarpc::context::current()) diff --git a/test/test-rpc/src/lib.rs b/test/test-rpc/src/lib.rs index 2e5007a8088f..73ab467ab8d4 100644 --- a/test/test-rpc/src/lib.rs +++ b/test/test-rpc/src/lib.rs @@ -152,13 +152,13 @@ mod service { async fn resolve_hostname(hostname: String) -> Result, Error>; /// Restart the Mullvad VPN application. - async fn restart_app() -> Result<(), Error>; + async fn restart_mullvad_daemon() -> Result<(), Error>; /// Stop the Mullvad VPN application. - async fn stop_app() -> Result<(), Error>; + async fn stop_mullvad_daemon() -> Result<(), Error>; /// Start the Mullvad VPN application. - async fn start_app() -> Result<(), Error>; + async fn start_mullvad_daemon() -> Result<(), Error>; /// Sets the log level of the daemon service, the verbosity level represents the number of /// `-v`s passed on the command line. This will restart the daemon system service. @@ -177,8 +177,6 @@ mod service { async fn reboot() -> Result<(), Error>; - async fn set_mullvad_daemon_service_state(on: bool) -> Result<(), Error>; - async fn make_device_json_old() -> Result<(), Error>; } } diff --git a/test/test-runner/src/main.rs b/test/test-runner/src/main.rs index 259ccdbab69e..e460162fa425 100644 --- a/test/test-runner/src/main.rs +++ b/test/test-runner/src/main.rs @@ -226,17 +226,17 @@ impl Service for TestServer { logging::get_mullvad_app_logs().await } - async fn restart_app(self, _: context::Context) -> Result<(), test_rpc::Error> { + async fn restart_mullvad_daemon(self, _: context::Context) -> Result<(), test_rpc::Error> { sys::restart_app().await } /// Stop the Mullvad VPN application. - async fn stop_app(self, _: context::Context) -> Result<(), test_rpc::Error> { + async fn stop_mullvad_daemon(self, _: context::Context) -> Result<(), test_rpc::Error> { sys::stop_app().await } /// Start the Mullvad VPN application. - async fn start_app(self, _: context::Context) -> Result<(), test_rpc::Error> { + async fn start_mullvad_daemon(self, _: context::Context) -> Result<(), test_rpc::Error> { sys::start_app().await } @@ -292,13 +292,13 @@ impl Service for TestServer { sys::reboot() } - async fn set_mullvad_daemon_service_state( - self, - _: context::Context, - on: bool, - ) -> Result<(), test_rpc::Error> { - sys::set_mullvad_daemon_service_state(on).await - } + // async fn set_mullvad_daemon_service_state( + // self, + // _: context::Context, + // on: bool, + // ) -> Result<(), test_rpc::Error> { + // sys::set_mullvad_daemon_service_state(on).await + // } async fn make_device_json_old(self, _: context::Context) -> Result<(), test_rpc::Error> { app::make_device_json_old().await diff --git a/test/test-runner/src/sys.rs b/test/test-runner/src/sys.rs index cc892054253e..343beeeea6dc 100644 --- a/test/test-runner/src/sys.rs +++ b/test/test-runner/src/sys.rs @@ -216,7 +216,14 @@ pub async fn restart_app() -> Result<(), test_rpc::Error> { /// This function waits for the app to successfully shut down. #[cfg(target_os = "linux")] pub async fn stop_app() -> Result<(), test_rpc::Error> { - set_mullvad_daemon_service_state(false).await + tokio::process::Command::new("systemctl") + .args(["stop", "mullvad-daemon"]) + .status() + .await + .map_err(|e| test_rpc::Error::Service(e.to_string()))?; + wait_for_service_state(ServiceState::Inactive).await?; + + Ok(()) } /// Start the Mullvad VPN application. @@ -224,7 +231,13 @@ pub async fn stop_app() -> Result<(), test_rpc::Error> { /// This function waits for the app to successfully start again. #[cfg(target_os = "linux")] pub async fn start_app() -> Result<(), test_rpc::Error> { - set_mullvad_daemon_service_state(true).await + tokio::process::Command::new("systemctl") + .args(["start", "mullvad-daemon"]) + .status() + .await + .map_err(|e| test_rpc::Error::Service(e.to_string()))?; + wait_for_service_state(ServiceState::Running).await?; + Ok(()) } /// Restart the Mullvad VPN application. @@ -506,26 +519,6 @@ pub async fn set_daemon_environment(env: HashMap) -> Result<(), Ok(()) } -#[cfg(target_os = "linux")] -pub async fn set_mullvad_daemon_service_state(on: bool) -> Result<(), test_rpc::Error> { - if on { - tokio::process::Command::new("systemctl") - .args(["start", "mullvad-daemon"]) - .status() - .await - .map_err(|e| test_rpc::Error::Service(e.to_string()))?; - wait_for_service_state(ServiceState::Running).await?; - } else { - tokio::process::Command::new("systemctl") - .args(["stop", "mullvad-daemon"]) - .status() - .await - .map_err(|e| test_rpc::Error::Service(e.to_string()))?; - wait_for_service_state(ServiceState::Inactive).await?; - } - Ok(()) -} - #[cfg(target_os = "windows")] pub async fn set_mullvad_daemon_service_state(on: bool) -> Result<(), test_rpc::Error> { if on {