Skip to content

Commit

Permalink
Merge branch 'test_upgrade_app-fails-on-windows-des-1565'
Browse files Browse the repository at this point in the history
  • Loading branch information
@MarkusPettersson98
MarkusPettersson98 committed Jan 7, 2025
2 parents 0951379 + 5f0cca3 commit 7eb54f3
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 58 deletions.
25 changes: 12 additions & 13 deletions test/test-manager/src/tests/install.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,14 @@ pub async fn test_upgrade_app(
_mullvad_client: MullvadClientArgument,
) -> anyhow::Result<()> {
// Install the older version of the app and verify that it is running.
install_app(
&rpc,
TEST_CONFIG
.app_package_to_upgrade_from_filename
.as_ref()
.unwrap(),
&ctx.rpc_provider,
)
.await
.context("Failed to install previous app version")?;
let old_version = TEST_CONFIG
.app_package_to_upgrade_from_filename
.as_ref()
.context("Could not find previous app version")?;
log::debug!("Installing app version {old_version}");
install_app(&rpc, old_version, &ctx.rpc_provider)
.await
.context("Failed to install previous app version")?;

// Verify that daemon is running
if rpc.mullvad_daemon_get_status().await? != ServiceStatus::Running {
Expand Down Expand Up @@ -104,9 +102,10 @@ pub async fn test_upgrade_app(
tokio::time::sleep(Duration::from_secs(3)).await;

// verify that daemon is running
if rpc.mullvad_daemon_get_status().await? != ServiceStatus::Running {
bail!(Error::DaemonNotRunning);
}
ensure!(
rpc.mullvad_daemon_get_status().await? == ServiceStatus::Running,
Error::DaemonNotRunning
);

// Verify that the correct version was installed
let running_daemon_version = rpc.mullvad_daemon_version().await?;
Expand Down
8 changes: 7 additions & 1 deletion test/test-rpc/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,14 @@ pub enum Error {
Ping(String),
#[error("Failed to get or set registry value: {0}")]
Registry(String),
#[error("Failed to start the service: {0}")]
ServiceStart(String),
#[error("Failed to stop the service: {0}")]
ServiceStop(String),
#[error("Failed to change the service: {0}")]
Service(String),
ServiceChange(String),
#[error("Failed to find the service: {0}")]
ServiceNotFound(String),
#[error("Could not read from or write to the file system: {0}")]
FileSystem(String),
#[error("Could not serialize or deserialize file: {0}")]
Expand Down
11 changes: 9 additions & 2 deletions test/test-runner/src/app.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,18 @@ use test_rpc::{AppTrace, Error};

/// Get the installed app version string
pub async fn version() -> Result<String, Error> {
let version = tokio::process::Command::new("mullvad")
// The `mullvad` binary is seemingly not in PATH on Windows after upgrading the app..
// So, as a workaround we use the absolute path instead.
const MULLVAD_CLI_BIN: &str = if cfg!(target_os = "windows") {
r"C:\Program Files\Mullvad VPN\resources\mullvad.exe"
} else {
"mullvad"
};
let version = tokio::process::Command::new(MULLVAD_CLI_BIN)
.arg("--version")
.output()
.await
.map_err(|e| Error::Service(e.to_string()))?;
.map_err(|e| Error::ServiceNotFound(e.to_string()))?;
let version = String::from_utf8(version.stdout).map_err(|err| Error::Other(err.to_string()))?;
// HACK: The output from `mullvad --version` includes the `mullvad-cli` binary name followed by
// the version string. Simply remove the leading noise and get at the version string.
Expand Down
91 changes: 49 additions & 42 deletions test/test-runner/src/sys.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ ExecStart=/usr/bin/mullvad-daemon --disable-stdout-timestamps {verbosity}"#
if let Some(parent) = override_path.parent() {
tokio::fs::create_dir_all(parent)
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceChange(e.to_string()))?;
}

let mut file = tokio::fs::OpenOptions::new()
Expand All @@ -189,17 +189,17 @@ ExecStart=/usr/bin/mullvad-daemon --disable-stdout-timestamps {verbosity}"#
.write(true)
.open(override_path)
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceChange(e.to_string()))?;

file.write_all(systemd_service_file_content.as_bytes())
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceChange(e.to_string()))?;

tokio::process::Command::new("systemctl")
.args(["daemon-reload"])
.status()
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceStart(e.to_string()))?;

restart_app().await?;
Ok(())
Expand All @@ -214,7 +214,7 @@ pub async fn restart_app() -> Result<(), test_rpc::Error> {
.args(["restart", "mullvad-daemon"])
.status()
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceStart(e.to_string()))?;
wait_for_service_state(ServiceState::Running).await?;
Ok(())
}
Expand All @@ -228,7 +228,7 @@ pub async fn stop_app() -> Result<(), test_rpc::Error> {
.args(["stop", "mullvad-daemon"])
.status()
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceStop(e.to_string()))?;
wait_for_service_state(ServiceState::Inactive).await?;

Ok(())
Expand All @@ -243,7 +243,7 @@ pub async fn start_app() -> Result<(), test_rpc::Error> {
.args(["start", "mullvad-daemon"])
.status()
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceStart(e.to_string()))?;
wait_for_service_state(ServiceState::Running).await?;
Ok(())
}
Expand All @@ -267,7 +267,7 @@ pub async fn stop_app() -> Result<(), test_rpc::Error> {
.args(["stop", "mullvadvpn"])
.status()
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceStop(e.to_string()))?;
Ok(())
}

Expand All @@ -280,7 +280,7 @@ pub async fn start_app() -> Result<(), test_rpc::Error> {
.args(["start", "mullvadvpn"])
.status()
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceStart(e.to_string()))?;
Ok(())
}

Expand Down Expand Up @@ -325,7 +325,7 @@ pub async fn set_daemon_log_level(verbosity_level: Verbosity) -> Result<(), test
};

let manager = ServiceManager::local_computer(None::<&str>, ServiceManagerAccess::CONNECT)
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceNotFound(e.to_string()))?;
let service = manager
.open_service(
"mullvadvpn",
Expand All @@ -334,23 +334,23 @@ pub async fn set_daemon_log_level(verbosity_level: Verbosity) -> Result<(), test
| ServiceAccess::START
| ServiceAccess::STOP,
)
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceNotFound(e.to_string()))?;

// Stop the service
// TODO: Extract to separate function.
service
.stop()
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceStop(e.to_string()))?;
tokio::process::Command::new("net")
.args(["stop", "mullvadvpn"])
.status()
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceStop(e.to_string()))?;

// Get the current service configuration
let config = service
.query_config()
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceNotFound(e.to_string()))?;

let executable_path = "C:\\Program Files\\Mullvad VPN\\resources\\mullvad-daemon.exe";
let launch_arguments = vec![
Expand All @@ -375,13 +375,13 @@ pub async fn set_daemon_log_level(verbosity_level: Verbosity) -> Result<(), test
// Apply the updated configuration
service
.change_config(&updated_config)
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceChange(e.to_string()))?;

// Start the service
// TODO: Extract to separate function.
service
.start::<String>(&[])
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceNotFound(e.to_string()))?;

Ok(())
}
Expand Down Expand Up @@ -445,19 +445,19 @@ pub async fn set_daemon_environment(env: HashMap<String, String>) -> Result<(),
.map(|env_var| env_var.to_systemd_string())
{
writeln!(&mut override_content, "{env_var}")
.map_err(|err| test_rpc::Error::Service(err.to_string()))?;
.map_err(|err| test_rpc::Error::ServiceChange(err.to_string()))?;
}

let override_path = std::path::Path::new(SYSTEMD_OVERRIDE_FILE);
if let Some(parent) = override_path.parent() {
tokio::fs::create_dir_all(parent)
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceChange(e.to_string()))?;
}

tokio::fs::write(override_path, override_content)
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceChange(e.to_string()))?;

if tokio::process::Command::new("systemctl")
.args(["daemon-reload"])
Expand All @@ -467,7 +467,7 @@ pub async fn set_daemon_environment(env: HashMap<String, String>) -> Result<(),
.success()
.not()
{
return Err(test_rpc::Error::Service(
return Err(test_rpc::Error::ServiceChange(
"Daemon service could not be reloaded".to_owned(),
));
};
Expand All @@ -480,7 +480,7 @@ pub async fn set_daemon_environment(env: HashMap<String, String>) -> Result<(),
.success()
.not()
{
return Err(test_rpc::Error::Service(
return Err(test_rpc::Error::ServiceStart(
"Daemon service could not be restarted".to_owned(),
));
};
Expand Down Expand Up @@ -541,8 +541,9 @@ pub fn get_system_path_var() -> Result<String, test_rpc::Error> {
#[cfg(target_os = "macos")]
pub async fn set_daemon_environment(env: HashMap<String, String>) -> Result<(), test_rpc::Error> {
tokio::task::spawn_blocking(|| {
let mut parsed_plist = plist::Value::from_file(PLIST_OVERRIDE_FILE)
.map_err(|error| test_rpc::Error::Service(format!("failed to parse plist: {error}")))?;
let mut parsed_plist = plist::Value::from_file(PLIST_OVERRIDE_FILE).map_err(|error| {
test_rpc::Error::ServiceNotFound(format!("failed to parse plist: {error}"))
})?;

let mut vars = plist::Dictionary::new();
for (k, v) in env {
Expand All @@ -551,25 +552,25 @@ pub async fn set_daemon_environment(env: HashMap<String, String>) -> Result<(),
.arg("setenv")
.args([&k, &v])
.status()
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
.map_err(|e| test_rpc::Error::ServiceChange(e.to_string()))?;
vars.insert(k, plist::Value::String(v));
}

// Add permanent env var
parsed_plist
.as_dictionary_mut()
.ok_or_else(|| test_rpc::Error::Service("plist missing dict".to_owned()))?
.ok_or_else(|| test_rpc::Error::ServiceChange("plist missing dict".to_owned()))?
.insert(
"EnvironmentVariables".to_owned(),
plist::Value::Dictionary(vars),
);

let daemon_plist = std::fs::File::create(PLIST_OVERRIDE_FILE)
.map_err(|e| test_rpc::Error::Service(format!("failed to open plist: {e}")))?;
.map_err(|e| test_rpc::Error::ServiceChange(format!("failed to open plist: {e}")))?;

parsed_plist
.to_writer_xml(daemon_plist)
.map_err(|e| test_rpc::Error::Service(format!("failed to replace plist: {e}")))?;
.map_err(|e| test_rpc::Error::ServiceChange(format!("failed to replace plist: {e}")))?;

Ok::<(), test_rpc::Error>(())
})
Expand All @@ -586,15 +587,20 @@ pub async fn set_daemon_environment(env: HashMap<String, String>) -> Result<(),

#[cfg(target_os = "macos")]
async fn set_launch_daemon_state(on: bool) -> Result<(), test_rpc::Error> {
tokio::process::Command::new("launchctl")
.args([
if on { "load" } else { "unload" },
"-w",
PLIST_OVERRIDE_FILE,
])
.status()
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?;
let mut launchctl = tokio::process::Command::new("launchctl");
if on {
launchctl
.args(["load", "-w", PLIST_OVERRIDE_FILE])
.status()
.await
.map_err(|e| test_rpc::Error::ServiceStart(e.to_string()))?;
} else {
launchctl
.args(["unload", "-w", PLIST_OVERRIDE_FILE])
.status()
.await
.map_err(|e| test_rpc::Error::ServiceStop(e.to_string()))?;
}
Ok(())
}

Expand Down Expand Up @@ -665,8 +671,9 @@ pub async fn get_daemon_environment() -> Result<HashMap<String, String>, test_rp
#[cfg(target_os = "macos")]
pub async fn get_daemon_environment() -> Result<HashMap<String, String>, test_rpc::Error> {
let plist = tokio::task::spawn_blocking(|| {
let parsed_plist = plist::Value::from_file(PLIST_OVERRIDE_FILE)
.map_err(|error| test_rpc::Error::Service(format!("failed to parse plist: {error}")))?;
let parsed_plist = plist::Value::from_file(PLIST_OVERRIDE_FILE).map_err(|error| {
test_rpc::Error::ServiceNotFound(format!("failed to parse plist: {error}"))
})?;

Ok::<plist::Value, test_rpc::Error>(parsed_plist)
})
Expand All @@ -675,14 +682,14 @@ pub async fn get_daemon_environment() -> Result<HashMap<String, String>, test_rp

let plist_tree = plist
.as_dictionary()
.ok_or_else(|| test_rpc::Error::Service("plist missing dict".to_owned()))?;
.ok_or_else(|| test_rpc::Error::ServiceNotFound("plist missing dict".to_owned()))?;
let Some(env_vars) = plist_tree.get("EnvironmentVariables") else {
// `EnvironmentVariables` does not exist in plist file, so there are no env variables to
// parse.
return Ok(HashMap::new());
};
let env_vars = env_vars.as_dictionary().ok_or_else(|| {
test_rpc::Error::Service("`EnvironmentVariables` is not a dict".to_owned())
test_rpc::Error::ServiceNotFound("`EnvironmentVariables` is not a dict".to_owned())
})?;

let env = env_vars
Expand All @@ -707,7 +714,7 @@ async fn wait_for_service_state(awaited_state: ServiceState) -> Result<(), test_
loop {
attempt += 1;
if attempt > RETRY_ATTEMPTS {
return Err(test_rpc::Error::Service(String::from(
return Err(test_rpc::Error::ServiceStart(String::from(
"Awaiting new service state timed out",
)));
}
Expand All @@ -716,7 +723,7 @@ async fn wait_for_service_state(awaited_state: ServiceState) -> Result<(), test_
.args(["status", "mullvad-daemon"])
.output()
.await
.map_err(|e| test_rpc::Error::Service(e.to_string()))?
.map_err(|e| test_rpc::Error::ServiceNotFound(e.to_string()))?
.stdout;
let output = String::from_utf8_lossy(&output);

Expand Down

0 comments on commit 7eb54f3

Please sign in to comment.