Remove superseded RPC for restarting the Mullvad system service

The function `set_mullvad_daemon_service_state(on: bool) -> Result<(),
test_rpc::Error>`, which would conditionally start or stop the Mullvad
daemon in the test runner, has been superseded by two separate functions which
accomplish the same thing: `start_mullvad_daemon` & `stop_mullvad_daemon`.

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");
 

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(())
 }

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())

test/test-rpc/src/lib.rs

@@ -152,13 +152,13 @@ mod service {
         async fn resolve_hostname(hostname: String) -> Result<Vec<SocketAddr>, 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>;
     }
 }

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

test/test-runner/src/sys.rs

@@ -216,15 +216,28 @@ 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.
 ///
 /// 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<String, String>) -> 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 {