Refactor integration tests

.cargo/config.toml

@@ -1,4 +1,4 @@
 [alias]
 build-install = "run --package xtask-build-install --"
 package-build = "run --package xtask-package-build --"
-xtest = "run --package xtask-test-runner --"
+xtest = "test --features integration-tests"

CHANGELOG.md

@@ -9,6 +9,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 ### Added
 - The metadata repositories can now contain source files.
 
+### Changes
+- The test environment now uses a fresh environment for each test and can be used in parallel.
+
 ### Fixed
 - Fix `LocalMetadataExistence` verifier fix not updating the last refresh and update state in the register.
 - Fix OS version detection not extracting the Linux kernel version correctly.

docs/tests.md

@@ -12,15 +12,13 @@ cargo test
 ```
 
 ## Integration tests
-Cargo test doesn't have support for a general setup or clean up before and after the integration tests. That's why Packit uses an xtask `xtask-test-runner` which wraps around the `cargo test` command. 
-The arguments to `xtest` are passed along to `cargo test`, so test flags can still be used.
-Run the following command to run the integration tests:
+Packits integration tests make use of `integration-tests` feature flag, to manage the integration test environment. The following shortcut can be used:
 ```
 cargo xtest
 ```
 
-For the setup this wrapper first checks for the `TestConfig.toml`, which is located next to the `Config.toml`. The `TestConfig.toml` is used if it exists, otherwise a default test config is created. The default test config uses `<prefix>/test` as the prefix directory, where `<prefix>` is the 'normal' prefix directory. Lastly the test prefix is created, containing an empty `Register.toml`.
+Each integration test creates a new test environment. The (temporary) test directory is passed to packit with the `--config-path` flag. When the test is done the test directory will get out of scope and be dropped, which will trigger the cleanup function.
 
->Note that the setup fails if the test prefix already exists, because this could mean that the 'normal' prefix was used instead.
+To use a custom config the `TestConfig.toml` can be used. If this file is present in the prefix directory it will be used for the `url` and `provider` type. Other fields are ignored.
 
-After the setup is done, the test can be ran. A cleanup function removes everything which didn't exist before the test execution. So if the `TestConfig.toml` already existed it will not be deleted.
+The environment supports parallel testing.

src/cli/commands.rs

@@ -15,6 +15,9 @@ mod unlink;
 mod update;
 mod util;
 
+#[cfg(feature = "integration-tests")]
+use std::sync::OnceLock;
+
 use std::process::exit;
 
 use clap::{Parser, Subcommand, builder::Styles};
@@ -33,6 +36,9 @@ use crate::{
     utils::packit_version::{packit_version, packit_version_name},
 };
 
+#[cfg(feature = "integration-tests")]
+pub static TEST_CONFIG_PATH: OnceLock<String> = OnceLock::new();
+
 /// Represents the CLI interface with clap.
 #[derive(Parser, Debug)]
 #[command(name = "Packit", version, about)]
@@ -41,6 +47,10 @@ use crate::{
 pub struct Cli {
     #[command(subcommand)]
     command: Commands,
+
+    #[cfg(feature = "integration-tests")]
+    #[arg(long)]
+    config_path: String,
 }
 
 #[derive(Subcommand, Debug)]
@@ -109,7 +119,7 @@ impl Cli {
         }
 
         // Parse command
-        match Cli::try_parse() {
+        let cli = match Cli::try_parse() {
             Ok(cli) => cli,
             Err(e) => {
                 let styles = Styles::default();
@@ -127,7 +137,13 @@ impl Cli {
 
                 exit(e.exit_code())
             },
-        }
+        };
+
+        // Set the `TEST_CONFIG_PATH` when doing integration tests
+        #[cfg(feature = "integration-tests")]
+        TEST_CONFIG_PATH.set(cli.config_path.clone()).expect("Expected single initialization");
+
+        cli
     }
 
     /// Reads and handles the command.

src/cli/commands/info.rs

@@ -2,7 +2,7 @@
 use crate::{
     cli::display::{standard_print::DisplayJoined, styled::MapStyled},
     installer::types::PackageName,
-    platforms::{DEFAULT_CONFIG_DIR, OsVersion, Target},
+    platforms::{OsVersion, Target, get_default_config_dir},
     register::metadata::LocalMetaHandler,
     repositories::manager::RepositoryManager,
     utils::packit_version::{current_packit_version, packit_version_name},
@@ -142,7 +142,7 @@ impl InfoArgs {
         println!("{}", packit_version_name!().italic().cyan());
 
         let mut pair_aligner = PairAligner::new();
-        pair_aligner.add("Config directory", DEFAULT_CONFIG_DIR);
+        pair_aligner.add("Config directory", get_default_config_dir());
         pair_aligner.add("Prefix directory", config.prefix_directory.display());
         pair_aligner.add("Multiuser mode", config.multiuser);
         pair_aligner.add("Installed packages", register.iterate_all().count());

src/cli/commands/init.rs

@@ -19,7 +19,7 @@ use crate::{
         Symlinker,
         types::{PackageId, PackageName, Version},
     },
-    platforms::{DEFAULT_CONFIG_DIR, DEFAULT_PREFIX, permissions},
+    platforms::{DEFAULT_PREFIX, get_default_config_dir, permissions},
     register::{
         installed_package_version::InstalledPackageVersion,
         metadata::{LocalMetaHandler, LocalMetaPackageHandler, LocalMetadata},
@@ -68,15 +68,16 @@ impl HandleCommand for InitArgs {
         }
 
         // Check if config directory exists
-        let config_dir = Path::new(DEFAULT_CONFIG_DIR);
+        let default_config_dir = get_default_config_dir();
+        let config_dir = Path::new(default_config_dir);
         if !config_dir.exists() {
-            error!(msg: "Packit cannot be initialized: the config directory at '{DEFAULT_CONFIG_DIR}' does not exist yet, please create it first");
+            error!(msg: "Packit cannot be initialized: the config directory at '{default_config_dir}' does not exist yet, please create it first");
             exit(1);
         }
 
         // Check if config directory is writable
         if !permissions::is_writable(config_dir).unwrap_or_exit_msg("Unable to check if config directory is writable", 1) {
-            error!(msg: "Packit cannot be initialized: the config directory at '{DEFAULT_CONFIG_DIR}' is not writable, please set the correct permissions");
+            error!(msg: "Packit cannot be initialized: the config directory at '{default_config_dir}' is not writable, please set the correct permissions");
             exit(1);
         }
 

src/config.rs

@@ -17,7 +17,7 @@ use crate::{
         logging::warning,
         standard_print::{self, DisplayOption},
     },
-    platforms::{DEFAULT_CONFIG_DIR, DEFAULT_PREFIX},
+    platforms::{DEFAULT_PREFIX, get_default_config_dir},
     repositories::metadata::DEFAULT_METADATA_PROVIDER_ID,
     utils::{
         constants::{
@@ -143,7 +143,7 @@ impl Config {
 
     /// Gets the default path of the Packit config file.
     pub fn get_default_path() -> PathBuf {
-        Path::new(DEFAULT_CONFIG_DIR).join(CONFIG_FILENAME)
+        Path::new(get_default_config_dir()).join(CONFIG_FILENAME)
     }
 
     fn default_prefix_directory() -> PathBuf {

src/platforms/defaults.rs

@@ -9,11 +9,24 @@ pub const DEFAULT_PREFIX: &str = "/opt/packit";
 #[cfg(target_os = "windows")]
 pub const DEFAULT_PREFIX: &str = "C:\\Program Files\\packit";
 
-#[cfg(target_os = "linux")]
-pub const DEFAULT_CONFIG_DIR: &str = "/etc/packit";
+#[cfg(all(target_os = "linux", not(feature = "integration-tests")))]
+const DEFAULT_CONFIG_DIR: &str = "/etc/packit";
 
-#[cfg(target_os = "macos")]
-pub const DEFAULT_CONFIG_DIR: &str = "/Library/Application Support/packit";
+#[cfg(all(target_os = "macos", not(feature = "integration-tests")))]
+const DEFAULT_CONFIG_DIR: &str = "/Library/Application Support/packit";
 
-#[cfg(target_os = "windows")]
-pub const DEFAULT_CONFIG_DIR: &str = "C:\\Program Files\\packit";
+#[cfg(all(target_os = "windows", not(feature = "integration-tests")))]
+const DEFAULT_CONFIG_DIR: &str = "C:\\Program Files\\packit";
+
+/// Returns the default config directory. It returns the constant defined above if the `integration-tests` feature is not enabled.
+/// Otherwise the value from `TEST_CONFIG_PATH` (given by the test call) is used.
+pub fn get_default_config_dir() -> &'static str {
+    #[cfg(not(feature = "integration-tests"))]
+    return DEFAULT_CONFIG_DIR;
+
+    #[cfg(feature = "integration-tests")]
+    {
+        use crate::cli::commands::TEST_CONFIG_PATH;
+        TEST_CONFIG_PATH.get().expect("Expected initialized config path")
+    }
+}

src/platforms/mod.rs

@@ -7,8 +7,8 @@ mod target;
 mod target_architecture;
 pub mod tool_detection;
 
-pub use defaults::DEFAULT_CONFIG_DIR;
 pub use defaults::DEFAULT_PREFIX;
+pub use defaults::get_default_config_dir;
 
 pub use os::Os;
 pub use os::OsVersion;

src/utils/constants.rs

@@ -6,11 +6,6 @@ use crate::repositories::metadata::WEB_METADATA_PROVIDER_ID;
 
 pub const REGISTER_FILENAME: &str = "Register.toml";
 
-/// Note that when the `integration-tests` feature flag is enabled `CONFIG_FILENAME` is compiled differently.
-#[cfg(feature = "integration-tests")]
-pub const CONFIG_FILENAME: &str = "TestConfig.toml";
-
-#[cfg(not(feature = "integration-tests"))]
 pub const CONFIG_FILENAME: &str = "Config.toml";
 
 pub const DEFAULT_METADATA_REPOSITORY_NAME: &str = "packit-core";

tests/README.md

@@ -12,4 +12,6 @@ cargo test
 Run the following command to run the integration tests:
 ```
 cargo xtest
-```
+```
+
+Find out more about the test environment [here](../docs/tests.md).

tests/integration/cli.rs

@@ -1,28 +1,31 @@
 // SPDX-License-Identifier: GPL-3.0-only
-use crate::packit;
+use crate::{common::environment::Environment, packit};
 
 #[test]
 fn version() {
-    packit!("--version").assert().success();
+    let env = Environment::init();
+    packit!(env, "--version").assert().success();
+    env.clean();
 }
 
 #[test]
 fn info_requirements() {
+    let env = Environment::init();
+
     // Install simple package for test
-    _ = packit!("install", "simple@0.0.1", "--build").ok();
+    _ = packit!(env, "install", "simple@0.0.1", "--build").assert().success();
 
-    packit!("info", "--tree").assert().failure();
-    packit!("info", "--active").assert().failure();
-    packit!("info", "--tree --active").assert().failure();
-    packit!("info", "simple", "--tree").assert().failure();
+    packit!(env, "info", "--tree").assert().failure();
+    packit!(env, "info", "--active").assert().failure();
+    packit!(env, "info", "--tree --active").assert().failure();
+    packit!(env, "info", "simple", "--tree").assert().failure();
 
-    packit!("info").assert().success();
-    packit!("info", "simple").assert().success();
-    packit!("info", "simple@0.0.1").assert().success();
-    packit!("info", "simple@0.0.1", "--tree").assert().success();
-    packit!("info", "simple", "--active").assert().success();
-    packit!("info", "simple", "--tree", "--active").assert().success();
+    packit!(env, "info").assert().success();
+    packit!(env, "info", "simple").assert().success();
+    packit!(env, "info", "simple@0.0.1").assert().success();
+    packit!(env, "info", "simple@0.0.1", "--tree").assert().success();
+    packit!(env, "info", "simple", "--active").assert().success();
+    packit!(env, "info", "simple", "--tree", "--active").assert().success();
 
-    // Cleanup simple package after test
-    _ = packit!("uninstall", "simple").ok();
+    env.clean();
 }