Merge branch 'make-launching-test-manager-easier-for-individual-tests-des-1039'

12 files changed, 161 insertions, 159 deletions

diff --git a/test/README.md b/test/README.md
index 07f223dac3..b490166ef5 100644
--- a/test/README.md
+++ b/test/README.md
@@ -115,8 +115,8 @@ cargo run --bin test-manager run-vm debian11
 cargo run --bin test-manager run-tests debian11 \
     --display \
     --account 0123456789 \
-    --current-app <git hash or tag> \
-    --previous-app 2023.2
+    --app-package <git hash or tag> \
+    --app-package-to-upgrade-from 2023.2
 ```
 
 ## macOS
@@ -141,8 +141,8 @@ cargo run --bin test-manager set macos-ventura tart ventura-base macos \
 cargo run --bin test-manager run-tests macos-ventura \
     --display \
     --account 0123456789 \
-    --current-app <git hash or tag> \
-    --previous-app 2023.2
+    --app-package <git hash or tag> \
+    --app-package-to-upgrade-from 2023.2
 ```
 
 ## Note on `ci-runtests.sh`
diff --git a/test/ci-runtests.sh b/test/ci-runtests.sh
index 4b8704643a..e6b7ba7b18 100755
--- a/test/ci-runtests.sh
+++ b/test/ci-runtests.sh
@@ -197,8 +197,9 @@ function run_tests_for_os {
     RUST_LOG=debug cargo run --bin test-manager \
         run-tests \
         --account "${ACCOUNT_TOKEN:?Error: ACCOUNT_TOKEN not set}" \
-        --current-app "${cur_filename}" \
-        --previous-app "${prev_filename}" \
+        --app-package "${cur_filename}" \
+        --app-package-to-upgrade-from "${prev_filename}" \
+        --package-folder "$PACKAGES_DIR" \
         --test-report "$SCRIPT_DIR/.ci-logs/${os}_report" \
         "$os" 2>&1 | sed "s/${ACCOUNT_TOKEN}/\{ACCOUNT_TOKEN\}/g"
 }
diff --git a/test/scripts/ssh-setup.sh b/test/scripts/ssh-setup.sh
index b3d358f5a0..7eb8ab56d3 100644
--- a/test/scripts/ssh-setup.sh
+++ b/test/scripts/ssh-setup.sh
@@ -6,7 +6,7 @@ SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
 cd "$SCRIPT_DIR"
 
 RUNNER_DIR="$1"
-CURRENT_APP="$2"
+APP_PACKAGE="$2"
 PREVIOUS_APP="$3"
 UI_RUNNER="$4"
 
@@ -16,7 +16,7 @@ echo "Copying test-runner to $RUNNER_DIR"
 
 mkdir -p "$RUNNER_DIR"
 
-for file in test-runner connection-checker $CURRENT_APP $PREVIOUS_APP $UI_RUNNER openvpn.ca.crt; do
+for file in test-runner connection-checker $APP_PACKAGE $PREVIOUS_APP $UI_RUNNER openvpn.ca.crt; do
     echo "Moving $file to $RUNNER_DIR"
     cp -f "$SCRIPT_DIR/$file" "$RUNNER_DIR"
 done
diff --git a/test/test-manager/src/container.rs b/test/test-manager/src/container.rs
index 84b80282c2..19cde03ee4 100644
--- a/test/test-manager/src/container.rs
+++ b/test/test-manager/src/container.rs
@@ -28,7 +28,9 @@ pub async fn relaunch_with_rootlesskit(vnc_port: Option<u16>) {
 
     cmd.args(std::env::args());
 
-    let status = cmd.status().await.unwrap();
+    let status = cmd.status().await.unwrap_or_else(|e| {
+        panic!("failed to execute [{:?}]: {}", cmd, e);
+    });
 
     std::process::exit(status.code().unwrap_or(1));
 }
diff --git a/test/test-manager/src/logging.rs b/test/test-manager/src/logging.rs
index e85920b1cd..3d1aa13156 100644
--- a/test/test-manager/src/logging.rs
+++ b/test/test-manager/src/logging.rs
@@ -30,7 +30,7 @@ impl Logger {
             logger.filter_module("tower", log::LevelFilter::Info);
             logger.filter_module("hyper", log::LevelFilter::Info);
             logger.filter_module("rustls", log::LevelFilter::Info);
-            logger.filter_level(log::LevelFilter::Debug);
+            logger.filter_level(log::LevelFilter::Info);
             logger.parse_env(env_logger::DEFAULT_FILTER_ENV);
 
             let env_logger = logger.build();
diff --git a/test/test-manager/src/main.rs b/test/test-manager/src/main.rs
index 37d7227967..8dd56e6a03 100644
--- a/test/test-manager/src/main.rs
+++ b/test/test-manager/src/main.rs
@@ -79,22 +79,29 @@ enum Commands {
         #[arg(long, short)]
         account: String,
 
-        /// App package to test.
+        /// App package to test. Can be a path to the package, just the package file name, git hash
+        /// or tag. If the direct path is not given, the package is assumed to be in the directory
+        /// specified by the `--package-folder` argument.
         ///
         /// # Note
         ///
         /// The gRPC interface must be compatible with the version specified for
         /// `mullvad-management-interface` in Cargo.toml.
-        #[arg(long, short)]
-        current_app: String,
+        #[arg(long)]
+        app_package: String,
 
-        /// App package to upgrade from.
+        /// App package to upgrade from when running `test_install_previous_app`, can be left empty
+        /// if this test is not ran. Parsed the same way as `--app-package`.
         ///
         /// # Note
         ///
         /// The CLI interface must be compatible with the upgrade test.
-        #[arg(long, short)]
-        previous_app: String,
+        #[arg(long)]
+        app_package_to_upgrade_from: Option<String>,
+
+        /// Folder to search for packages. Defaults to current directory.
+        #[arg(long, value_name = "DIR")]
+        package_folder: Option<PathBuf>,
 
         /// Only run tests matching substrings
         test_filters: Vec<String>,
@@ -217,8 +224,9 @@ async fn main() -> Result<()> {
             display,
             vnc,
             account,
-            current_app,
-            previous_app,
+            app_package,
+            app_package_to_upgrade_from,
+            package_folder,
             test_filters,
             verbose,
             test_report,
@@ -252,9 +260,13 @@ async fn main() -> Result<()> {
                 None => None,
             };
 
-            let manifest = package::get_app_manifest(vm_config, current_app, previous_app)
-                .await
-                .context("Could not find the specified app packages")?;
+            let manifest = package::get_app_manifest(
+                vm_config,
+                app_package,
+                app_package_to_upgrade_from,
+                package_folder,
+            )
+            .context("Could not find the specified app packages")?;
 
             let mut instance = vm::run(&config, &name)
                 .await
@@ -276,24 +288,18 @@ async fn main() -> Result<()> {
                 tests::config::TestConfig {
                     account_number: account,
                     artifacts_dir,
-                    current_app_filename: manifest
-                        .current_app_path
-                        .file_name()
-                        .unwrap()
-                        .to_string_lossy()
-                        .into_owned(),
-                    previous_app_filename: manifest
-                        .previous_app_path
+                    app_package_filename: manifest
+                        .app_package_path
                         .file_name()
                         .unwrap()
                         .to_string_lossy()
                         .into_owned(),
+                    app_package_to_upgrade_from_filename: manifest
+                        .app_package_to_upgrade_from_path
+                        .map(|path| path.file_name().unwrap().to_string_lossy().into_owned()),
                     ui_e2e_tests_filename: manifest
                         .ui_e2e_tests_path
-                        .file_name()
-                        .unwrap()
-                        .to_string_lossy()
-                        .into_owned(),
+                        .map(|path| path.file_name().unwrap().to_string_lossy().into_owned()),
                     mullvad_host,
                     #[cfg(target_os = "macos")]
                     host_bridge_name: crate::vm::network::macos::find_vm_bridge()?,
diff --git a/test/test-manager/src/package.rs b/test/test-manager/src/package.rs
index 7e69ae867f..f3fd535673 100644
--- a/test/test-manager/src/package.rs
+++ b/test/test-manager/src/package.rs
@@ -3,56 +3,59 @@ use anyhow::{Context, Result};
 use once_cell::sync::Lazy;
 use regex::Regex;
 use std::path::{Path, PathBuf};
-use tokio::fs;
 
 static VERSION_REGEX: Lazy<Regex> =
     Lazy::new(|| Regex::new(r"\d{4}\.\d+(-beta\d+)?(-dev)?-([0-9a-z])+").unwrap());
 
 #[derive(Debug, Clone)]
 pub struct Manifest {
-    pub current_app_path: PathBuf,
-    pub previous_app_path: PathBuf,
-    pub ui_e2e_tests_path: PathBuf,
+    pub app_package_path: PathBuf,
+    pub app_package_to_upgrade_from_path: Option<PathBuf>,
+    pub ui_e2e_tests_path: Option<PathBuf>,
 }
 
 /// Obtain app packages and their filenames
 /// If it's a path, use the path.
 /// If it corresponds to a file in packages/, use that package.
 /// TODO: If it's a git tag or rev, download it.
-pub async fn get_app_manifest(
+pub fn get_app_manifest(
     config: &VmConfig,
-    current_app: String,
-    previous_app: String,
+    app_package: String,
+    app_package_to_upgrade_from: Option<String>,
+    package_folder: Option<PathBuf>,
 ) -> Result<Manifest> {
     let package_type = (config.os_type, config.package_type, config.architecture);
 
-    let current_app_path = find_app(&current_app, false, package_type).await?;
-    log::info!("Current app: {}", current_app_path.display());
+    let app_package_path = find_app(&app_package, false, package_type, package_folder.as_ref())?;
+    log::info!("App package: {}", app_package_path.display());
 
-    let previous_app_path = find_app(&previous_app, false, package_type).await?;
-    log::info!("Previous app: {}", previous_app_path.display());
+    let app_package_to_upgrade_from_path = app_package_to_upgrade_from
+        .map(|app| find_app(&app, false, package_type, package_folder.as_ref()))
+        .transpose()?;
+    log::info!("App package to upgrade from: {app_package_to_upgrade_from_path:?}");
 
     let capture = VERSION_REGEX
-        .captures(current_app_path.to_str().unwrap())
-        .with_context(|| format!("Cannot parse version: {}", current_app_path.display()))?
+        .captures(app_package_path.to_str().unwrap())
+        .with_context(|| format!("Cannot parse version: {}", app_package_path.display()))?
         .get(0)
         .map(|c| c.as_str())
-        .expect("Could not parse version from package name: {current_app}");
+        .expect("Could not parse version from package name: {app_package}");
 
-    let ui_e2e_tests_path = find_app(capture, true, package_type).await?;
-    log::info!("Runner executable: {}", ui_e2e_tests_path.display());
+    let ui_e2e_tests_path = find_app(capture, true, package_type, package_folder.as_ref()).ok();
+    log::info!("GUI e2e test binary: {ui_e2e_tests_path:?}");
 
     Ok(Manifest {
-        current_app_path,
-        previous_app_path,
+        app_package_path,
+        app_package_to_upgrade_from_path,
         ui_e2e_tests_path,
     })
 }
 
-async fn find_app(
+fn find_app(
     app: &str,
     e2e_bin: bool,
     package_type: (OsType, Option<PackageType>, Option<Architecture>),
+    package_folder: Option<&PathBuf>,
 ) -> Result<PathBuf> {
     // If it's a path, use that path
     let app_path = Path::new(app);
@@ -64,79 +67,45 @@ async fn find_app(
     let mut app = app.to_owned();
     app.make_ascii_lowercase();
 
-    let packages_dir = dirs::cache_dir()
-        .context("Could not find cache directory")?
-        .join("mullvad-test")
-        .join("packages");
-    fs::create_dir_all(&packages_dir).await?;
-    let mut dir = fs::read_dir(packages_dir.clone())
-        .await
-        .context("Failed to list packages")?;
+    let current_dir = std::env::current_dir().expect("Unable to get current directory");
+    let packages_dir = package_folder.unwrap_or(&current_dir);
+    std::fs::create_dir_all(packages_dir)?;
+    let dir = std::fs::read_dir(packages_dir.clone()).context("Failed to list packages")?;
 
-    let mut matches = vec![];
-
-    while let Ok(Some(entry)) = dir.next_entry().await {
-        let path = entry.path();
-        if !path.is_file() {
-            continue;
-        }
-
-        // Filter out irrelevant platforms
-        if !e2e_bin {
-            let ext = get_ext(package_type);
-
-            // Skip file if wrong file extension
-            if !path
+    dir
+        .filter_map(|entry| entry.ok())
+        .map(|entry| entry.path())
+        .filter(|entry| entry.is_file())
+        .filter(|path| {
+            e2e_bin ||
+            path
                 .extension()
-                .map(|m_ext| m_ext.eq_ignore_ascii_case(ext))
+                .map(|m_ext| m_ext.eq_ignore_ascii_case(get_ext(package_type)))
                 .unwrap_or(false)
-            {
-                continue;
-            }
-        }
-
-        let mut u8_path = path.as_os_str().to_string_lossy().into_owned();
-        u8_path.make_ascii_lowercase();
-
-        // Skip non-UI-e2e binaries or vice versa
-        if e2e_bin ^ u8_path.contains("app-e2e-tests") {
-            continue;
-        }
-
-        // Filter out irrelevant platforms
-        if e2e_bin && !u8_path.contains(get_os_name(package_type)) {
-            continue;
-        }
-
-        // Skip file if it doesn't match the architecture
-        if let Some(arch) = package_type.2 {
-            // Skip for non-e2e bin on non-Linux, because there's only one package
-            if (e2e_bin || package_type.0 == OsType::Linux)
-                && !arch.get_identifiers().iter().any(|id| u8_path.contains(id))
-            {
-                continue;
-            }
-        }
-
-        if u8_path.contains(&app) {
-            matches.push(path);
-        }
-    }
-
-    // TODO: Search for package in git repository if not found
-
-    // Take the shortest match
-    matches.sort_unstable_by_key(|path| path.as_os_str().len());
-    matches.into_iter().next().context(if e2e_bin {
-        format!(
-            "Could not find UI/e2e test for package: {app}.\n\
-         Expecting a binary named like `app-e2e-tests-{app}_ARCH` to exist in {package_dir}/\n\
-         Example ARCH: `amd64-unknown-linux-gnu`, `x86_64-unknown-linux-gnu`",
-            package_dir = packages_dir.display()
-        )
-    } else {
-        format!("Could not find package for app: {app}")
-    })
+        }) // Filter out irrelevant platforms
+        .map(|path| {
+            let u8_path = path.as_os_str().to_string_lossy().to_ascii_lowercase();
+            (path, u8_path)
+        })
+        .filter(|(_path, u8_path)| !(e2e_bin ^ u8_path.contains("app-e2e-tests"))) // Skip non-UI-e2e binaries or vice versa
+        .filter(|(_path, u8_path)| !e2e_bin || u8_path.contains(get_os_name(package_type))) // Filter out irrelevant platforms
+        .filter(|(_path, u8_path)| {
+            let linux = e2e_bin || package_type.0 == OsType::Linux;
+            let matching_ident = package_type.2.map(|arch| arch.get_identifiers().iter().any(|id| u8_path.contains(id))).unwrap_or(true);
+            // Skip for non-Linux, because there's only one package
+            !linux || matching_ident
+        }) // Skip file if it doesn't match the architecture
+        .find(|(_path, u8_path)| u8_path.contains(&app)) //  Find match
+        .map(|(path, _)| path).context(if e2e_bin {
+            format!(
+                "Could not find UI/e2e test for package: {app}.\n\
+                Expecting a binary named like `app-e2e-tests-{app}_ARCH` to exist in {package_dir}/\n\
+                Example ARCH: `amd64-unknown-linux-gnu`, `x86_64-unknown-linux-gnu`",
+                package_dir = packages_dir.display()
+            )
+        } else {
+            format!("Could not find package for app: {app}")
+        })
 }
 
 fn get_ext(package_type: (OsType, Option<PackageType>, Option<Architecture>)) -> &'static str {
diff --git a/test/test-manager/src/tests/config.rs b/test/test-manager/src/tests/config.rs
index 7ffe737aa7..58ebc4fa01 100644
--- a/test/test-manager/src/tests/config.rs
+++ b/test/test-manager/src/tests/config.rs
@@ -12,9 +12,9 @@ pub struct TestConfig {
     pub account_number: String,
 
     pub artifacts_dir: String,
-    pub current_app_filename: String,
-    pub previous_app_filename: String,
-    pub ui_e2e_tests_filename: String,
+    pub app_package_filename: String,
+    pub app_package_to_upgrade_from_filename: Option<String>,
+    pub ui_e2e_tests_filename: Option<String>,
 
     /// Used to override MULLVAD_API_*, for conncheck,
     /// and for resolving relay IPs.
diff --git a/test/test-manager/src/tests/install.rs b/test/test-manager/src/tests/install.rs
index d7056cce76..0c9e2b82fd 100644
--- a/test/test-manager/src/tests/install.rs
+++ b/test/test-manager/src/tests/install.rs
@@ -22,8 +22,13 @@ pub async fn test_install_previous_app(_: TestContext, rpc: ServiceClient) -> an
 
     // install package
     log::debug!("Installing old app");
-    rpc.install_app(get_package_desc(&TEST_CONFIG.previous_app_filename)?)
-        .await?;
+    rpc.install_app(get_package_desc(
+        TEST_CONFIG
+            .app_package_to_upgrade_from_filename
+            .as_ref()
+            .context("Missing previous app version")?,
+    )?)
+    .await?;
 
     // verify that daemon is running
     if rpc.mullvad_daemon_get_status().await? != ServiceStatus::Running {
@@ -103,7 +108,7 @@ pub async fn test_upgrade_app(ctx: TestContext, rpc: ServiceClient) -> anyhow::R
 
     // install new package
     log::debug!("Installing new app");
-    rpc.install_app(get_package_desc(&TEST_CONFIG.current_app_filename)?)
+    rpc.install_app(get_package_desc(&TEST_CONFIG.app_package_filename)?)
         .await?;
 
     // Give it some time to start
@@ -253,7 +258,7 @@ pub async fn test_install_new_app(_: TestContext, rpc: ServiceClient) -> anyhow:
 
     // install package
     log::debug!("Installing new app");
-    rpc.install_app(get_package_desc(&TEST_CONFIG.current_app_filename)?)
+    rpc.install_app(get_package_desc(&TEST_CONFIG.app_package_filename)?)
         .await?;
 
     // verify that daemon is running
@@ -308,7 +313,7 @@ pub async fn test_installation_idempotency(
     for _ in 0..2 {
         // Install the app
         log::info!("Installing new app");
-        let app_package = get_package_desc(&TEST_CONFIG.current_app_filename)?;
+        let app_package = get_package_desc(&TEST_CONFIG.app_package_filename)?;
         rpc.install_app(app_package).await?;
         log::info!("App was successfully installed!");
 
diff --git a/test/test-manager/src/tests/mod.rs b/test/test-manager/src/tests/mod.rs
index 255efb5941..23a3976ef2 100644
--- a/test/test-manager/src/tests/mod.rs
+++ b/test/test-manager/src/tests/mod.rs
@@ -59,6 +59,9 @@ pub enum Error {
     #[error("The gRPC client ran into an error: {0}")]
     ManagementInterface(#[from] mullvad_management_interface::Error),
 
+    #[error("GUI test binary missing")]
+    MissingGuiTest,
+
     #[cfg(target_os = "macos")]
     #[error("An error occurred: {0}")]
     Other(String),
diff --git a/test/test-manager/src/tests/ui.rs b/test/test-manager/src/tests/ui.rs
index 65093a6758..f4b9af04f3 100644
--- a/test/test-manager/src/tests/ui.rs
+++ b/test/test-manager/src/tests/ui.rs
@@ -37,15 +37,23 @@ pub async fn run_test_env<
         Os::Linux => {
             bin_path = PathBuf::from("/usr/bin/xvfb-run");
 
-            let ui_runner_path =
-                Path::new(&TEST_CONFIG.artifacts_dir).join(&TEST_CONFIG.ui_e2e_tests_filename);
+            let ui_runner_path = Path::new(&TEST_CONFIG.artifacts_dir).join(
+                TEST_CONFIG
+                    .ui_e2e_tests_filename
+                    .as_ref()
+                    .ok_or(Error::MissingGuiTest)?,
+            );
             new_params = std::iter::once(ui_runner_path.to_string_lossy().into_owned())
                 .chain(params.iter().map(|param| param.as_ref().to_owned()))
                 .collect();
         }
         _ => {
-            bin_path =
-                Path::new(&TEST_CONFIG.artifacts_dir).join(&TEST_CONFIG.ui_e2e_tests_filename);
+            bin_path = Path::new(&TEST_CONFIG.artifacts_dir).join(
+                TEST_CONFIG
+                    .ui_e2e_tests_filename
+                    .as_ref()
+                    .ok_or(Error::MissingGuiTest)?,
+            );
             new_params = params
                 .iter()
                 .map(|param| param.as_ref().to_owned())
diff --git a/test/test-manager/src/vm/provision.rs b/test/test-manager/src/vm/provision.rs
index dfdfd30a39..f440cbe9c8 100644
--- a/test/test-manager/src/vm/provision.rs
+++ b/test/test-manager/src/vm/provision.rs
@@ -115,12 +115,22 @@ fn blocking_ssh(
         .context("Failed to send connection-checker to remote")?;
 
     // Transfer app packages
-    ssh_send_file_path(&session, &local_app_manifest.current_app_path, temp_dir)
+    ssh_send_file_path(&session, &local_app_manifest.app_package_path, temp_dir)
         .context("Failed to send current app package to remote")?;
-    ssh_send_file_path(&session, &local_app_manifest.previous_app_path, temp_dir)
-        .context("Failed to send previous app package to remote")?;
-    ssh_send_file_path(&session, &local_app_manifest.ui_e2e_tests_path, temp_dir)
-        .context("Failed to send UI test runner to remote")?;
+    if let Some(app_package_to_upgrade_from_path) =
+        &local_app_manifest.app_package_to_upgrade_from_path
+    {
+        ssh_send_file_path(&session, app_package_to_upgrade_from_path, temp_dir)
+            .context("Failed to send previous app package to remote")?;
+    } else {
+        log::warn!("No previous app to send to remote")
+    }
+    if let Some(ui_e2e_tests_path) = &local_app_manifest.ui_e2e_tests_path {
+        ssh_send_file_path(&session, ui_e2e_tests_path, temp_dir)
+            .context("Failed to send ui_e2e_tests_path to remote")?;
+    } else {
+        log::warn!("No UI e2e test to send to remote")
+    }
 
     // Transfer openvpn cert
     let dest: std::path::PathBuf = temp_dir.join("openvpn.ca.crt");
@@ -147,28 +157,26 @@ fn blocking_ssh(
     .context("failed to send bootstrap script to remote")?;
 
     // Run setup script
+    let app_package_path = local_app_manifest
+        .app_package_path
+        .file_name()
+        .unwrap()
+        .to_string_lossy();
+    let app_package_to_upgrade_from_path = local_app_manifest
+        .app_package_to_upgrade_from_path
+        .map(|path| path.file_name().unwrap().to_string_lossy().into_owned())
+        .unwrap_or_default();
+    let ui_e2e_tests_path = local_app_manifest
+        .ui_e2e_tests_path
+        .map(|path| path.file_name().unwrap().to_string_lossy().into_owned())
+        .unwrap_or_default();
 
-    let args = format!(
-        "{remote_dir} \"{}\" \"{}\" \"{}\"",
-        local_app_manifest
-            .current_app_path
-            .file_name()
-            .unwrap()
-            .to_string_lossy(),
-        local_app_manifest
-            .previous_app_path
-            .file_name()
-            .unwrap()
-            .to_string_lossy(),
-        local_app_manifest
-            .ui_e2e_tests_path
-            .file_name()
-            .unwrap()
-            .to_string_lossy(),
+    let cmd = format!(
+        "sudo {} {remote_dir} \"{app_package_path}\" \"{app_package_to_upgrade_from_path}\" \"{ui_e2e_tests_path}\"",
+        dest.display()
     );
-
-    log::debug!("Running setup script on remote, args: {args}");
-    ssh_exec(&session, &format!("sudo {} {args}", dest.display()))
+    log::debug!("Running setup script on remote, cmd: {cmd}");
+    ssh_exec(&session, &cmd)
         .map(drop)
         .context("Failed to run setup script")
 }