Make test-manager CLI simpler

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`

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"
 }

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

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

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

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

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 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
+    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")?;
+
+    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 {

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.