From 7bb8e17e88367058820a2490812a5bdd0c807252 Mon Sep 17 00:00:00 2001 From: Jonas Greitemann Date: Wed, 16 Apr 2025 17:26:03 +0200 Subject: [PATCH] tests: factor out utility function `is_external_tool_installed` A pattern has emerged where a integration tests check for the availability of an external tool (`git`, `taplo`, `gpg`, ...) and skip the test (by simply passing it) when it is not available. To check this, the program is run with the `--version` flag. Some tests require that the program be available at least when running in CI, by calling `ensure_running_outside_ci` conditionally on the outcome. The decision is up to each test, though, the utility merely returns a `bool`. --- cli/tests/test_config_schema.rs | 8 ++------ lib/tests/test_git_backend.rs | 4 ++-- lib/tests/test_gpg.rs | 6 +++--- lib/testutils/src/lib.rs | 17 +++++++++++------ 4 files changed, 18 insertions(+), 17 deletions(-) diff --git a/cli/tests/test_config_schema.rs b/cli/tests/test_config_schema.rs index 490d493f2..8c1ce900e 100644 --- a/cli/tests/test_config_schema.rs +++ b/cli/tests/test_config_schema.rs @@ -4,14 +4,10 @@ use std::process::Output; use std::process::Stdio; use testutils::ensure_running_outside_ci; +use testutils::is_external_tool_installed; fn taplo_check_config(file: &Path) -> datatest_stable::Result> { - if Command::new("taplo") - .arg("--version") - .stdout(Stdio::null()) - .status() - .is_err() - { + if !is_external_tool_installed("taplo") { ensure_running_outside_ci("`taplo` must be in the PATH"); eprintln!("Skipping test because taplo is not installed on the system"); return Ok(None); diff --git a/lib/tests/test_git_backend.rs b/lib/tests/test_git_backend.rs index aa9adb59f..bd8dfa7b0 100644 --- a/lib/tests/test_git_backend.rs +++ b/lib/tests/test_git_backend.rs @@ -15,7 +15,6 @@ use std::collections::HashMap; use std::collections::HashSet; use std::path::Path; -use std::process::Command; use std::sync::Arc; use std::time::Duration; use std::time::SystemTime; @@ -39,6 +38,7 @@ use testutils::commit_with_tree; use testutils::create_random_commit; use testutils::create_single_tree; use testutils::create_tree; +use testutils::is_external_tool_installed; use testutils::repo_path; use testutils::repo_path_buf; use testutils::CommitGraphBuilder; @@ -96,7 +96,7 @@ fn make_commit( #[test] fn test_gc() { // TODO: Better way to disable the test if git command couldn't be executed - if Command::new("git").arg("--version").status().is_err() { + if !is_external_tool_installed("git") { eprintln!("Skipping because git command might fail to run"); return; } diff --git a/lib/tests/test_gpg.rs b/lib/tests/test_gpg.rs index 7930c646b..a234a584d 100644 --- a/lib/tests/test_gpg.rs +++ b/lib/tests/test_gpg.rs @@ -3,7 +3,6 @@ use std::fs::Permissions; use std::io::Write as _; #[cfg(unix)] use std::os::unix::prelude::PermissionsExt as _; -use std::process::Command; use std::process::Stdio; use assert_matches::assert_matches; @@ -14,6 +13,7 @@ use jj_lib::signing::SigStatus; use jj_lib::signing::SignError; use jj_lib::signing::SigningBackend as _; use testutils::ensure_running_outside_ci; +use testutils::is_external_tool_installed; static GPG_PRIVATE_KEY: &str = r#"-----BEGIN PGP PRIVATE KEY BLOCK----- @@ -165,7 +165,7 @@ impl GpgsmEnvironment { macro_rules! gpg_guard { () => { - if Command::new("gpg").arg("--version").status().is_err() { + if !is_external_tool_installed("gpg") { ensure_running_outside_ci("`gpg` must be in the PATH"); eprintln!("Skipping test because gpg is not installed on the system"); return; @@ -175,7 +175,7 @@ macro_rules! gpg_guard { macro_rules! gpgsm_guard { () => { - if Command::new("gpgsm").arg("--version").status().is_err() { + if !is_external_tool_installed("gpgsm") { ensure_running_outside_ci("`gpgsm` must be in the PATH"); eprintln!("Skipping test because gpgsm is not installed on the system"); return; diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index 78e71a4f0..38042f97b 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -15,6 +15,7 @@ use std::collections::HashMap; use std::collections::HashSet; use std::env; +use std::ffi::OsStr; use std::fs; use std::fs::OpenOptions; use std::io::Read as _; @@ -142,6 +143,15 @@ pub fn ensure_running_outside_ci(reason: &str) { assert!(!running_in_ci, "Running in CI, {reason}."); } +/// Tests if an external tool is installed and in the PATH +pub fn is_external_tool_installed(program_name: impl AsRef) -> bool { + Command::new(program_name) + .arg("--version") + .stdout(Stdio::null()) + .status() + .is_ok() +} + #[derive(Debug)] pub struct TestEnvironment { temp_dir: TempDir, @@ -627,12 +637,7 @@ pub fn assert_abandoned_with_parent( pub fn assert_no_forgotten_test_files(test_dir: &Path) { // We require `taplo` for this check; if it's not installed, that's ok unless // we're running in CI. - if Command::new("taplo") - .arg("--version") - .stdout(Stdio::null()) - .status() - .is_err() - { + if !is_external_tool_installed("taplo") { ensure_running_outside_ci("`taplo` must be in the PATH"); eprintln!( "Skipping check for forgotten test files because taplo is not installed on the system"