prioritize and order tests

also check cache before (re)populating it

Author: 2bndy5

.config/nextest.toml

@@ -7,7 +7,7 @@ default-filter = """\
     not test(#*rate_limit_secondary) \
     - test(#git::test::with_*) \
     - test(#git::test::repo_get_sha) \
-    - test(#*eval_static_dist)
+    - test(#*eval_version)
 """
 
 # This will flag any test that runs longer than 10 seconds. Useful when writing new tests.
@@ -16,12 +16,23 @@ slow-timeout = "10s"
 # show which tests were skipped
 status-level = "skip"
 
+[test-groups]
+# keep cache-setup group serialized to avoid race conditions installing clang tools.
+cache-setup = { max-threads = 1 }
+
+[[profile.default.overrides]]
+# Run these tests with the highest priority.
+filter = 'test(#*eval_version)'
+priority = 100
+# assign it to a test group so that it can be run separately
+test-group = 'cache-setup'
+
 [profile.ci]
 # A profile to run only tests that use clang-tidy and/or clang-format
 # NOTE: This profile is intended to keep CI runtime low. Locally, use default or all profiles
 
 # This is all tests in tests/ folder + unit test for --extra-args.
-default-filter = "kind(test) + test(#*use_extra_args)"
+default-filter = "kind(test) + test(#*use_extra_args) + test(#*eval_version)"
 
 # show log output from each test
 success-output = "final"

clang-installer/src/downloader/native_packages/mod.rs

@@ -66,21 +66,21 @@ pub fn get_available_package_managers() -> Vec<impl PackageManager + Display> {
 pub fn try_install_package(
     tool: &ClangTool,
     version_req: &VersionReq,
+    min_version: &Version,
 ) -> Result<Option<ClangVersion>, GetToolError> {
     let os_pkg_managers = get_available_package_managers();
     if os_pkg_managers.is_empty() {
         log::error!("No supported package managers found on the system.");
         return Ok(None);
     } else {
-        let min_version = get_min_ver(version_req).ok_or(GetToolError::VersionMajorRequired)?;
         for mgr in os_pkg_managers {
             if !mgr.is_installed() {
                 log::debug!("Skipping {mgr} package manager as it is not installed.");
                 continue;
             }
             log::info!("Trying to install {tool} v{min_version} using {mgr} package manager.");
             let pkg_name = mgr.get_package_name(tool);
-            if mgr.is_installed_package(&pkg_name, Some(&min_version)) {
+            if mgr.is_installed_package(&pkg_name, Some(min_version)) {
                 let path =
                     tool.get_exe_path(&RequestedVersion::Requirement(version_req.clone()))?;
                 let version = tool.capture_version(&path)?;
@@ -92,7 +92,7 @@ pub fn try_install_package(
                 log::info!(
                     "{mgr} package manager does not have a version of {tool} matching {version_req} installed."
                 );
-                match mgr.install_package(&pkg_name, Some(&min_version)) {
+                match mgr.install_package(&pkg_name, Some(min_version)) {
                     Ok(()) => {
                         log::info!(
                             "Successfully installed {tool} v{min_version} using {mgr} package manager."
@@ -121,22 +121,3 @@ pub fn try_install_package(
     }
     Ok(None)
 }
-
-fn get_min_ver(version_req: &VersionReq) -> Option<Version> {
-    let mut result = None;
-    for cmp in &version_req.comparators {
-        if matches!(cmp.op, semver::Op::Exact | semver::Op::Caret) {
-            let ver = Version {
-                major: cmp.major,
-                minor: cmp.minor.unwrap_or(0),
-                patch: cmp.patch.unwrap_or(0),
-                pre: cmp.pre.clone(),
-                build: Default::default(),
-            };
-            if result.as_ref().is_none_or(|r| ver < *r) {
-                result = Some(ver);
-            }
-        }
-    }
-    result
-}

clang-installer/src/version.rs

@@ -1,7 +1,7 @@
 use std::{path::PathBuf, str::FromStr};
 
 use crate::{
-    ClangTool, PyPiDownloadError, PyPiDownloader,
+    Cacher, ClangTool, PyPiDownloadError, PyPiDownloader,
     downloader::{native_packages::try_install_package, static_dist::StaticDistDownloader},
     tool::{GetClangPathError, GetClangVersionError},
     utils::normalize_path,
@@ -92,7 +92,7 @@ impl RequestedVersion {
                 }))
             }
             RequestedVersion::SystemDefault => {
-                let path = tool.get_exe_path(&RequestedVersion::SystemDefault)?;
+                let path = tool.get_exe_path(&Self::SystemDefault)?;
                 let version = tool.capture_version(&path)?;
                 log::info!(
                     "Found {tool} version {version} at path: {:?}",
@@ -101,9 +101,8 @@ impl RequestedVersion {
                 Ok(Some(ClangVersion { version, path }))
             }
             RequestedVersion::Requirement(version_req) => {
-                if let Ok(path) =
-                    tool.get_exe_path(&RequestedVersion::Requirement(version_req.clone()))
-                {
+                // check default available version first (if any)
+                if let Ok(path) = tool.get_exe_path(&Self::Requirement(version_req.clone())) {
                     let version = tool.capture_version(&path)?;
                     if version_req.matches(&version) {
                         log::info!(
@@ -113,11 +112,33 @@ impl RequestedVersion {
                         return Ok(Some(ClangVersion { version, path }));
                     }
                 }
+
+                // check if cache has a suitable version
+                let bin_ext = if cfg!(windows) { ".exe" } else { "" };
+                let min_ver = get_min_ver(version_req).ok_or(GetToolError::VersionMajorRequired)?;
+                let cached_bin = StaticDistDownloader::get_cache_dir()
+                    .join("bin")
+                    .join(format!("{tool}-{min_ver}{bin_ext}"));
+                if cached_bin.exists() {
+                    let version = tool.capture_version(&cached_bin)?;
+                    if version_req.matches(&version) {
+                        log::info!(
+                            "Found {tool} version {version} in cache at path: {:?}",
+                            cached_bin.to_string_lossy()
+                        );
+                        return Ok(Some(ClangVersion {
+                            version,
+                            path: cached_bin,
+                        }));
+                    }
+                }
+
+                // try to download a suitable version
                 let bin = match PyPiDownloader::download_tool(tool, version_req).await {
                     Ok(bin) => bin,
                     Err(e) => {
                         log::error!("Failed to download {tool} {version_req} from PyPi: {e}");
-                        if let Some(result) = try_install_package(tool, version_req)? {
+                        if let Some(result) = try_install_package(tool, version_req, &min_ver)? {
                             return Ok(Some(result));
                         }
                         log::info!("Falling back to downloading {tool} static binaries.");
@@ -132,9 +153,10 @@ impl RequestedVersion {
                         }
                     }
                 };
+
+                // create a symlink
                 let bin_dir = bin.parent().ok_or(GetToolError::ExecutablePathNoParent)?;
-                let symlink_path =
-                    bin_dir.join(format!("{tool}{}", if cfg!(windows) { ".exe" } else { "" }));
+                let symlink_path = bin_dir.join(format!("{tool}{bin_ext}"));
                 tool.symlink_bin(&bin, &symlink_path, overwrite_symlink)
                     .map_err(GetToolError::SymlinkError)?;
                 let version = tool.capture_version(&bin)?;
@@ -152,6 +174,25 @@ impl RequestedVersion {
     }
 }
 
+pub fn get_min_ver(version_req: &VersionReq) -> Option<Version> {
+    let mut result = None;
+    for cmp in &version_req.comparators {
+        if matches!(cmp.op, semver::Op::Exact | semver::Op::Caret) {
+            let ver = Version {
+                major: cmp.major,
+                minor: cmp.minor.unwrap_or(0),
+                patch: cmp.patch.unwrap_or(0),
+                pre: cmp.pre.clone(),
+                build: Default::default(),
+            };
+            if result.as_ref().is_none_or(|r| ver < *r) {
+                result = Some(ver);
+            }
+        }
+    }
+    result
+}
+
 /// Represents an error that occurred while parsing a requested version.
 #[derive(Debug, thiserror::Error)]
 pub enum RequestedVersionParsingError {
@@ -272,13 +313,10 @@ mod tests {
     /// It is designed to use the system's package manager to install clang-tidy.
     /// If successful, clang-tidy will be installed globally, which may be undesirable.
     #[tokio::test]
-    async fn eval_static_dist() {
-        let tmp_cache_dir = TempDir::new().unwrap();
-        unsafe {
-            std::env::set_var("CPP_LINTER_CACHE", tmp_cache_dir.path());
-        }
+    async fn eval_version() {
+        let clang_version = option_env!("CLANG_VERSION").unwrap_or("12.0.1");
         let tool = ClangTool::ClangTidy;
-        let version_req = VersionReq::parse("=12.0.1").unwrap();
+        let version_req = VersionReq::parse(clang_version).unwrap();
         let clang_path = RequestedVersion::Requirement(version_req.clone())
             .eval_tool(&tool, false)
             .await