test(test-tool): uninstall all versions and clear cache before installation#6393
test(test-tool): uninstall all versions and clear cache before installation#6393
Conversation
…lation This ensures a clean state when testing tools by: - Uninstalling all existing versions of the tool - Clearing the backend cache directory - Then performing a fresh installation and test This helps ensure tests are reproducible and not affected by previous installations or cached data.
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the mise test-tool command to ensure a clean testing environment by uninstalling all existing versions and clearing cache before testing. This makes tool testing more reproducible by eliminating interference from previous installations or cached data.
- Adds cleanup logic to uninstall all installed versions and clear cache before running tests
- Implements proper progress reporting during the cleanup process
- Adds comprehensive e2e test coverage to verify the new cleanup behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/cli/test_tool.rs | Adds cleanup logic to uninstall all versions and clear cache before testing |
| e2e/cli/test_test_tool_clean | Adds comprehensive e2e test to verify cleanup behavior works correctly |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/cli/test_tool.rs
Outdated
| // List and uninstall all installed versions | ||
| let installed_versions = backend.list_installed_versions(); | ||
| debug!("Backend short name: {}", tool.ba.short); | ||
| debug!("Tool short name: {}", tool.short); |
There was a problem hiding this comment.
[nitpick] These debug statements appear to be leftover from development. Consider removing them as they don't add value in production and the information is already available through other logging.
| debug!("Tool short name: {}", tool.short); |
src/cli/test_tool.rs
Outdated
| let request = crate::toolset::ToolRequest::Version { | ||
| backend: tool.ba.clone(), | ||
| version: version.clone(), | ||
| options: Default::default(), | ||
| source: crate::toolset::ToolSource::Unknown, | ||
| }; | ||
| let tv = crate::toolset::ToolVersion::new(request, version); |
There was a problem hiding this comment.
The version is cloned twice unnecessarily. Consider using version.clone() only once and reusing it, or restructure to avoid the extra clone in line 173.
| let request = crate::toolset::ToolRequest::Version { | |
| backend: tool.ba.clone(), | |
| version: version.clone(), | |
| options: Default::default(), | |
| source: crate::toolset::ToolSource::Unknown, | |
| }; | |
| let tv = crate::toolset::ToolVersion::new(request, version); | |
| let version_cloned = version.clone(); | |
| let request = crate::toolset::ToolRequest::Version { | |
| backend: tool.ba.clone(), | |
| version: version_cloned.clone(), | |
| options: Default::default(), | |
| source: crate::toolset::ToolSource::Unknown, | |
| }; | |
| let tv = crate::toolset::ToolVersion::new(request, version_cloned); |
…ling - Clear downloads directory in addition to cache directory - Call Config::reset() after uninstalling to clear in-memory backend metadata caches - Only reset config if we actually uninstalled any versions
Instead of uninstalling each version, directly remove the entire installs, cache, and downloads directories for the tool. This is simpler and more thorough.
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.9.16 x -- echo |
19.5 ± 0.3 | 18.9 | 21.8 | 1.00 |
mise x -- echo |
19.5 ± 0.4 | 19.1 | 26.5 | 1.00 ± 0.03 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.9.16 env |
18.9 ± 0.5 | 18.3 | 23.6 | 1.00 |
mise env |
19.0 ± 0.3 | 18.4 | 20.3 | 1.01 ± 0.03 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.9.16 hook-env |
18.9 ± 0.3 | 18.2 | 22.6 | 1.01 ± 0.03 |
mise hook-env |
18.7 ± 0.3 | 18.1 | 20.2 | 1.00 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2025.9.16 ls |
16.4 ± 0.2 | 16.0 | 17.4 | 1.00 |
mise ls |
16.5 ± 0.2 | 16.1 | 17.6 | 1.01 ± 0.02 |
xtasks/test/perf
| Command | mise-2025.9.16 | mise | Variance |
|---|---|---|---|
| install (cached) | 169ms | ✅ 103ms | +64% |
| ls (cached) | 63ms | 62ms | +1% |
| bin-paths (cached) | 70ms | 69ms | +1% |
| task-ls (cached) | 467ms | 476ms | -1% |
✅ Performance improvement: install cached is 64%
The test now properly validates that test-tool cleans directories before installation. Updated to use --include-non-defined flag for tiny since it lacks a test definition, and to handle the expected test failure gracefully while still validating the cleaning behavior.
Replaced tiny with shellcheck in the test since shellcheck has a proper test definition in the registry. This eliminates the need for the --include-non-defined flag and handling expected failures, making the test cleaner and more straightforward.
| // Reset the config to clear in-memory backend metadata caches if we cleaned anything | ||
| if cleaned_any { | ||
| *config = Config::reset().await?; | ||
| } |
There was a problem hiding this comment.
Bug: Cleanup Fails, Report Incomplete
The cleanup logic only removes directories for the main tool, not its dependencies, which can lead to inconsistent test states. Also, if any directory removal fails, the progress report's finish() method is skipped, leaving the report unfinished and potentially causing UI issues.
### 📦 Registry - replace amplify-cli github backend with ubi by @eggplants in [#6396](#6396) ### 🚀 Features - **(template)** add read_file() function by @jdx in [#6400](#6400) ### 🐛 Bug Fixes - **(aqua)** support github_artifact_attestations.enabled by @risu729 in [#6372](#6372) - use /c instead of -c on windows in postinstall hook by @risu729 in [#6397](#6397) ### 🧪 Testing - **(test-tool)** uninstall all versions and clear cache before installation by @jdx in [#6393](#6393) ### New Contributors - @eggplants made their first contribution in [#6396](#6396) Co-authored-by: mise-en-dev <release@mise.jdx.dev>
## [2025.9.18](https://github.com/jdx/mise/compare/v2025.9.17..v2025.9.18) - 2025-09-24 ### 📦 Registry - replace amplify-cli github backend with ubi by @eggplants in [#6396](jdx/mise#6396) ### 🚀 Features - **(template)** add read_file() function by @jdx in [#6400](jdx/mise#6400) ### 🐛 Bug Fixes - **(aqua)** support github_artifact_attestations.enabled by @risu729 in [#6372](jdx/mise#6372) - use /c instead of -c on windows in postinstall hook by @risu729 in [#6397](jdx/mise#6397) ### 🧪 Testing - **(test-tool)** uninstall all versions and clear cache before installation by @jdx in [#6393](jdx/mise#6393) ### New Contributors - @eggplants made their first contribution in [#6396](jdx/mise#6396) ## [2025.9.17](https://github.com/jdx/mise/compare/v2025.9.16..v2025.9.17) - 2025-09-24 ### 🚀 Features - **(java)** add support for Liberica NIK releases by @roele in [#6382](jdx/mise#6382) ### 🐛 Bug Fixes - **(toolset)** handle underflow in version_sub function by @koh-sh in [#6389](jdx/mise#6389) ### 📚 Documentation - document MISE_ENV behavior for global/system configs by @jdx in [#6385](jdx/mise#6385) ### New Contributors - @jc00ke made their first contribution in [#6386](jdx/mise#6386) - @koh-sh made their first contribution in [#6389](jdx/mise#6389)
## [2025.9.18](https://github.com/jdx/mise/compare/v2025.9.17..v2025.9.18) - 2025-09-24 ### 📦 Registry - replace amplify-cli github backend with ubi by @eggplants in [#6396](jdx/mise#6396) ### 🚀 Features - **(template)** add read_file() function by @jdx in [#6400](jdx/mise#6400) ### 🐛 Bug Fixes - **(aqua)** support github_artifact_attestations.enabled by @risu729 in [#6372](jdx/mise#6372) - use /c instead of -c on windows in postinstall hook by @risu729 in [#6397](jdx/mise#6397) ### 🧪 Testing - **(test-tool)** uninstall all versions and clear cache before installation by @jdx in [#6393](jdx/mise#6393) ### New Contributors - @eggplants made their first contribution in [#6396](jdx/mise#6396) ## [2025.9.17](https://github.com/jdx/mise/compare/v2025.9.16..v2025.9.17) - 2025-09-24 ### 🚀 Features - **(java)** add support for Liberica NIK releases by @roele in [#6382](jdx/mise#6382) ### 🐛 Bug Fixes - **(toolset)** handle underflow in version_sub function by @koh-sh in [#6389](jdx/mise#6389) ### 📚 Documentation - document MISE_ENV behavior for global/system configs by @jdx in [#6385](jdx/mise#6385) ### New Contributors - @jc00ke made their first contribution in [#6386](jdx/mise#6386) - @koh-sh made their first contribution in [#6389](jdx/mise#6389)
Summary
mise test-toolto uninstall all existing versions and clear cache before testingDetails
This ensures a clean state when testing tools by:
This helps ensure tests are reproducible and not affected by previous installations or cached data.
Test plan
test_test_tool_cleanto verify the cleanup behavior🤖 Generated with Claude Code