From 189e20c55555d5477935ddaec50c49094779a324 Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Mon, 10 Aug 2026 15:36:01 -0700 Subject: [PATCH 1/6] Add unit tests for list, snapshot, registry, download, and nu_pin_offer; add CI coverage job Adds in-process #[cfg(test)] coverage for previously-0%/low-coverage cmd entry points: cmd::list (0% -> 99%), cmd::nu_pin_offer (0% -> 71%), cmd::registry (34% -> 82%, add/remove/list, no network), cmd::snapshot (26% -> 73%, list/inspect/short_hash), and install::download's local file-path copy branch (19% -> 42%). Adds a coverage job to ci.yml using cargo-llvm-cov (informational only, no fail-under-lines threshold), reporting to the job summary. --- .github/workflows/ci.yml | 25 ++++++++ .gitignore | 3 + src/cmd/list.rs | 93 +++++++++++++++++++++++++++ src/cmd/nu_pin_offer.rs | 68 ++++++++++++++++++++ src/cmd/registry.rs | 133 +++++++++++++++++++++++++++++++++++++++ src/cmd/snapshot.rs | 96 ++++++++++++++++++++++++++++ src/install/download.rs | 30 +++++++++ 7 files changed, 448 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f6b5a399..c8e12962 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,6 +57,31 @@ jobs: components: rustfmt - run: cargo fmt --all -- --check + coverage: + name: Coverage + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: dtolnay/rust-toolchain@4cda84d5c5c54efe2404f9d843567869ab1699d4 # stable + with: + toolchain: stable + components: llvm-tools-preview + - uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1 + with: + save-if: ${{ github.event_name == 'push' }} + - uses: taiki-e/install-action@7f4eb899022d8fe70b20c4f3de697aa85c309026 # v2.85.11 + with: + tool: cargo-llvm-cov + # Informational only: no --fail-under-lines, so coverage can never fail the build. + - name: Run coverage + run: | + { + echo "### Coverage summary" + echo '```' + cargo llvm-cov --workspace --summary-only + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + roadmap-drift: name: Roadmap drift runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index fc1cf99f..ad46431f 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,6 @@ desktop.ini # Python __pycache__/ *.py[cod] + +# Coverage +*.profraw diff --git a/src/cmd/list.rs b/src/cmd/list.rs index c45f6d16..3c414ba1 100644 --- a/src/cmd/list.rs +++ b/src/cmd/list.rs @@ -41,3 +41,96 @@ pub fn execute(root: &Path) -> Result<()> { Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::state::lockfile::{LockfileEntry, PluginActivation}; + + fn base_entry(version: &str, package_type: &str) -> LockfileEntry { + LockfileEntry { + version: version.to_string(), + package_type: package_type.to_string(), + source: "binary".to_string(), + target: None, + artifact_url: None, + artifact_sha256: None, + executable_path: None, + archive_root: None, + include: None, + entry: None, + installed_at: "0".to_string(), + nu_version_at_install: None, + activation: None, + registry_url: None, + registry_revision: None, + index_sha256: None, + signing_key_fingerprint: None, + git_url: None, + git_rev: None, + cargo_name: None, + cargo_lock_sha256: None, + built_sha256: None, + payload_path: String::new(), + revision_id: None, + payload_sha256: None, + executable_sha256: None, + selection_reason: None, + origin: None, + module_activation: None, + module_import_mode: None, + locked_dependencies: Default::default(), + } + } + + #[test] + fn execute_empty_lockfile() { + let dir = tempfile::tempdir().unwrap(); + Lockfile::empty().save(dir.path()).unwrap(); + execute(dir.path()).unwrap(); + } + + #[test] + fn execute_one_package() { + let dir = tempfile::tempdir().unwrap(); + let mut lock = Lockfile::empty(); + lock.packages + .insert("owner/pkg".to_string(), base_entry("1.0.0", "plugin")); + lock.save(dir.path()).unwrap(); + execute(dir.path()).unwrap(); + } + + #[test] + fn execute_multiple_packages_with_one_active() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + + let mut lock = Lockfile::empty(); + let mut active = base_entry("1.0.0", "plugin"); + active.activation = Some(PluginActivation { + plugin_registry_path: "/path/to/plugins.msgpackz".to_string(), + nu_executable_sha256: "abc123".to_string(), + nu_version: "0.113.1".to_string(), + activated_at: "0".to_string(), + }); + lock.packages.insert("owner/active".to_string(), active); + lock.packages + .insert("owner/inactive".to_string(), base_entry("2.0.0", "module")); + lock.save(root).unwrap(); + + std::fs::create_dir_all(root.join("nu_state")).unwrap(); + let nu_paths = NuPaths { + nu_executable: "/usr/bin/nu".to_string(), + nu_version: "0.113.1".to_string(), + plugin_registry_path: "/path/to/plugins.msgpackz".to_string(), + nu_executable_hash: "abc123".to_string(), + platform: "x86_64-unknown-linux-gnu".to_string(), + data_dir: None, + vendor_autoload_dirs: vec![], + vendor_autoload_dir: None, + }; + nu_paths.save(root).unwrap(); + + execute(root).unwrap(); + } +} diff --git a/src/cmd/nu_pin_offer.rs b/src/cmd/nu_pin_offer.rs index 536c529b..fc1f5a21 100644 --- a/src/cmd/nu_pin_offer.rs +++ b/src/cmd/nu_pin_offer.rs @@ -106,3 +106,71 @@ pub fn is_nu_mismatch(diagnosis: &PackageIncompatibility) -> bool { | Incompatibility::NuUnsatisfied { .. } ) } + +#[cfg(test)] +mod tests { + use super::*; + + fn diagnosis_with_pin(pin: &str) -> PackageIncompatibility { + PackageIncompatibility { + issue: Incompatibility::NuTooOld { + constraint: ">=0.113.0".to_string(), + }, + suggested_pin: Some(pin.to_string()), + available_versions: vec![], + } + } + + #[test] + fn accept_proceeds_to_install_and_fails_hermetically_on_bad_pin() { + // A malformed pin fails local version normalization before any + // network call, so this exercises the accept branch deterministically. + let dir = tempfile::tempdir().unwrap(); + let diagnosis = diagnosis_with_pin("not-a-version"); + let err = + offer_managed_nu_pin_with_interaction(dir.path(), "0.112.0", &diagnosis, true, || { + Ok("y\n".to_string()) + }) + .unwrap_err(); + assert!( + err.to_string().contains("Failed to install managed Nu"), + "expected install failure context, got: {err}" + ); + } + + #[test] + fn decline_returns_false_without_installing() { + let dir = tempfile::tempdir().unwrap(); + let diagnosis = diagnosis_with_pin("0.113.1"); + let result = + offer_managed_nu_pin_with_interaction(dir.path(), "0.112.0", &diagnosis, true, || { + Ok("n\n".to_string()) + }) + .unwrap(); + assert!(!result); + } + + #[test] + fn invalid_input_is_treated_as_decline() { + let dir = tempfile::tempdir().unwrap(); + let diagnosis = diagnosis_with_pin("0.113.1"); + let result = + offer_managed_nu_pin_with_interaction(dir.path(), "0.112.0", &diagnosis, true, || { + Ok("maybe\n".to_string()) + }) + .unwrap(); + assert!(!result); + } + + #[test] + fn non_interactive_short_circuits_without_reading_input() { + let dir = tempfile::tempdir().unwrap(); + let diagnosis = diagnosis_with_pin("0.113.1"); + let result = + offer_managed_nu_pin_with_interaction(dir.path(), "0.112.0", &diagnosis, false, || { + panic!("read_line must not be called when non-interactive") + }) + .unwrap(); + assert!(!result); + } +} diff --git a/src/cmd/registry.rs b/src/cmd/registry.rs index a4ce35a8..33461db0 100644 --- a/src/cmd/registry.rs +++ b/src/cmd/registry.rs @@ -258,6 +258,139 @@ fn wrap_words(text: &str, width: usize) -> Vec { mod tests { use super::*; + fn test_key_b64() -> String { + let signing_key = ed25519_dalek::SigningKey::generate(&mut rand_core::OsRng); + base64::Engine::encode( + &base64::engine::general_purpose::STANDARD, + signing_key.verifying_key().to_bytes(), + ) + } + + #[test] + fn list_registries_prints_none_when_empty() { + let dir = tempfile::tempdir().unwrap(); + list_registries(dir.path()).unwrap(); + } + + #[test] + fn list_registries_prints_configured_entries() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let mut config = crate::config::Config::default(); + config.registries.insert( + "custom".to_string(), + crate::config::RegistryConfig { + url: "https://example.com/index.json".to_string(), + sync_interval: "24h".to_string(), + enabled: true, + trust_key: None, + }, + ); + config.save(root).unwrap(); + list_registries(root).unwrap(); + } + + #[test] + fn add_registry_persists_config_and_trust_key() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let key_b64 = test_key_b64(); + add_registry(root, "custom", "https://example.com/index.json", &key_b64).unwrap(); + + let config = crate::config::Config::load(root).unwrap(); + assert!(config.registries.contains_key("custom")); + let trust = TrustStore::load(root).unwrap(); + assert!(trust.keys.contains_key("custom")); + } + + #[test] + fn add_registry_rejects_duplicate_name() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let key_b64 = test_key_b64(); + add_registry(root, "custom", "https://example.com/index.json", &key_b64).unwrap(); + + let err = + add_registry(root, "custom", "https://example.com/other.json", &key_b64).unwrap_err(); + assert!(err.to_string().contains("already exists")); + } + + #[test] + fn remove_registry_removes_config_and_cached_index() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let key_b64 = test_key_b64(); + add_registry(root, "custom", "https://example.com/index.json", &key_b64).unwrap(); + std::fs::create_dir_all(root.join("registry/custom")).unwrap(); + + remove_registry(root, "custom").unwrap(); + + let config = crate::config::Config::load(root).unwrap(); + assert!(!config.registries.contains_key("custom")); + assert!(!root.join("registry/custom").exists()); + } + + #[test] + fn remove_registry_errors_when_not_found() { + let dir = tempfile::tempdir().unwrap(); + let err = remove_registry(dir.path(), "missing").unwrap_err(); + assert!(err.to_string().contains("not found")); + } + + #[test] + fn list_packages_prints_index_contents() { + use crate::core::package::{ + Artifact, Package, PackageType, RegistryIndex, ScopedId, VersionEntry, + }; + + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + std::fs::create_dir_all(root.join("registry/official")).unwrap(); + + let index = RegistryIndex { + schema_version: 1, + updated_at: "2026-06-27T00:00:00Z".to_string(), + registry_revision: Some("abc123".to_string()), + trust: None, + packages: vec![Package { + id: ScopedId::new("test", "pkg"), + description: "A test package for listing".to_string(), + repo: "https://github.com/test/pkg".to_string(), + package_type: PackageType::Plugin, + tags: vec!["test".to_string()], + versions: vec![VersionEntry { + version: semver::Version::new(1, 0, 0), + nu_version: ">=0.113.0 <0.114.0".to_string(), + verified_with: vec![], + artifact: Artifact { + kind: "binary".to_string(), + url: None, + sha256: None, + targets: std::collections::HashMap::new(), + archive_root: None, + include: None, + entry: None, + }, + source: None, + dependencies: std::collections::BTreeMap::new(), + activation: None, + provenance: None, + evidence_tier: None, + deferral_reason: None, + }], + }], + }; + let content = serde_json::to_string_pretty(&index).unwrap(); + std::fs::write(root.join("registry/official/index.json"), content).unwrap(); + std::fs::write( + root.join("config.toml"), + "[general]\ndefault_registry = \"official\"\n", + ) + .unwrap(); + + list_packages(root).unwrap(); + } + #[test] fn wrap_words_keeps_short_text_on_one_line() { assert_eq!( diff --git a/src/cmd/snapshot.rs b/src/cmd/snapshot.rs index d0a62fe7..c4ce60f3 100644 --- a/src/cmd/snapshot.rs +++ b/src/cmd/snapshot.rs @@ -287,9 +287,105 @@ fn short_hash(h: &str) -> String { #[cfg(test)] mod tests { use super::*; + use crate::state::lockfile::LockfileEntry; + use crate::state::snapshot::{create_snapshot, SnapshotReason, SnapshotTrigger}; const ID: &str = "00000000-0000-0000-0000-000000000000"; + fn payload_lockfile_entry() -> LockfileEntry { + LockfileEntry { + version: "1.0.0".to_string(), + package_type: "module".to_string(), + source: "archive".to_string(), + target: None, + artifact_url: None, + artifact_sha256: None, + executable_path: None, + archive_root: None, + include: None, + entry: Some("mod.nu".to_string()), + installed_at: "0".to_string(), + nu_version_at_install: None, + activation: None, + registry_url: None, + registry_revision: None, + index_sha256: None, + signing_key_fingerprint: None, + git_url: None, + git_rev: None, + cargo_name: None, + cargo_lock_sha256: None, + built_sha256: None, + payload_path: "packages/modules/owner/pkg/1.0.0-abc12345".to_string(), + revision_id: None, + payload_sha256: None, + executable_sha256: None, + selection_reason: None, + origin: None, + module_activation: None, + module_import_mode: None, + locked_dependencies: Default::default(), + } + } + + #[test] + fn short_hash_truncates_to_twelve_chars() { + assert_eq!(short_hash("abcdefghijklmnopqrstuvwxyz"), "abcdefghijkl"); + assert_eq!(short_hash("short"), "short"); + } + + #[test] + fn list_prints_no_snapshots_when_empty() { + let dir = tempfile::tempdir().unwrap(); + list(dir.path()).unwrap(); + } + + #[test] + fn list_prints_committed_snapshots() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + std::fs::create_dir_all(root.join("state")).unwrap(); + + create_snapshot( + root, + SnapshotReason::PreMutation, + SnapshotTrigger::Install, + None, + None, + ) + .unwrap(); + + list(root).unwrap(); + } + + #[test] + fn inspect_prints_snapshot_details_with_payload() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + std::fs::create_dir_all(root.join("state")).unwrap(); + + let payload = root.join("packages/modules/owner/pkg/1.0.0-abc12345"); + std::fs::create_dir_all(&payload).unwrap(); + std::fs::write(payload.join("mod.nu"), "# module").unwrap(); + + let mut lockfile = Lockfile::empty(); + lockfile + .packages + .insert("owner/pkg".to_string(), payload_lockfile_entry()); + lockfile.save(root).unwrap(); + + let manifest = create_snapshot( + root, + SnapshotReason::PreMutation, + SnapshotTrigger::Install, + None, + None, + ) + .unwrap(); + + inspect(root, &manifest.id).unwrap(); + } + #[test] fn delete_refuses_non_tty_without_yes() { // Force non-TTY via the injectable seam so the guard is deterministic diff --git a/src/install/download.rs b/src/install/download.rs index 1df4b9da..04493213 100644 --- a/src/install/download.rs +++ b/src/install/download.rs @@ -78,3 +78,33 @@ pub fn download_file(url: &str, dest: &Path) -> Result<()> { pb.finish_with_message("downloaded"); Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn download_file_copies_local_plain_path() { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("src.txt"); + std::fs::write(&src, b"hello").unwrap(); + let dest = dir.path().join("nested/dest.txt"); + + download_file(src.to_str().unwrap(), &dest).unwrap(); + + assert_eq!(std::fs::read(&dest).unwrap(), b"hello"); + } + + #[test] + fn download_file_copies_file_url() { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("src.txt"); + std::fs::write(&src, b"world").unwrap(); + let dest = dir.path().join("dest.txt"); + let url = format!("file://{}", src.display()); + + download_file(&url, &dest).unwrap(); + + assert_eq!(std::fs::read(&dest).unwrap(), b"world"); + } +} From f679bd38ff210e43af44e3cd6fca29c51d766011 Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Mon, 10 Aug 2026 15:48:32 -0700 Subject: [PATCH 2/6] Add tests for config, nu_version, and cmd::remove MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - config.rs: load/save round trip, missing-file default, malformed TOML, serde defaults, NUMAN_ROOT env override - core/nu_version.rs: parse() error branches, matches_constraint's >, <, and =exact-version operators, from_paths_or_detect cached-version path - cmd::remove: full execute_with_tty happy path (lockfile entry removed, payload deleted) on top of the existing guard-path tests util::stdio_redirect.rs's remaining gap is the Windows-only code path, not exercisable on this platform — left as-is. --- src/cmd/remove.rs | 28 ++++++++++++++++ src/config.rs | 72 ++++++++++++++++++++++++++++++++++++++++++ src/core/nu_version.rs | 66 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 166 insertions(+) diff --git a/src/cmd/remove.rs b/src/cmd/remove.rs index b9dec650..e75658e7 100644 --- a/src/cmd/remove.rs +++ b/src/cmd/remove.rs @@ -368,4 +368,32 @@ mod tests { "--yes must bypass the guard: {msg}" ); } + + #[test] + fn execute_removes_installed_package_end_to_end() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + std::fs::create_dir_all(root.join("owner/pkg")).unwrap(); + + let mut lockfile = Lockfile::empty(); + let mut entry = base_entry(); + entry.payload_path = "owner/pkg".to_string(); + lockfile.packages.insert("owner/pkg".to_string(), entry); + lockfile.save(root).unwrap(); + + execute_with_tty( + &RemoveArgs { + package: "owner/pkg".to_string(), + yes: true, + force: false, + }, + root, + false, + ) + .unwrap(); + + let reloaded = Lockfile::load(root).unwrap(); + assert!(!reloaded.packages.contains_key("owner/pkg")); + assert!(!root.join("owner/pkg").exists()); + } } diff --git a/src/config.rs b/src/config.rs index b27cc726..859cde0d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -155,3 +155,75 @@ impl Config { platform.default_root() } } + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::tempdir; + + #[test] + fn load_missing_file_returns_default() { + let dir = tempdir().unwrap(); + let config = Config::load(dir.path()).unwrap(); + assert_eq!(config.general.default_registry, "official"); + assert_eq!(config.activation.method, "autoload"); + assert!(config.install.prefer_binary); + } + + #[test] + fn save_then_load_round_trips() { + let dir = tempdir().unwrap(); + let mut config = Config::default(); + config.general.default_registry = "custom".to_string(); + config.registries.insert( + "custom".to_string(), + RegistryConfig { + url: "https://example.com/registry".to_string(), + sync_interval: "12h".to_string(), + enabled: true, + trust_key: Some("abc123".to_string()), + }, + ); + config.save(dir.path()).unwrap(); + + let loaded = Config::load(dir.path()).unwrap(); + assert_eq!(loaded.general.default_registry, "custom"); + let registry = loaded.registries.get("custom").unwrap(); + assert_eq!(registry.url, "https://example.com/registry"); + assert_eq!(registry.sync_interval, "12h"); + assert_eq!(registry.trust_key.as_deref(), Some("abc123")); + } + + #[test] + fn load_malformed_toml_errors() { + let dir = tempdir().unwrap(); + std::fs::write(dir.path().join("config.toml"), "not = [valid toml").unwrap(); + let result = Config::load(dir.path()); + assert!(result.is_err()); + } + + #[test] + fn registry_config_defaults_apply() { + let dir = tempdir().unwrap(); + std::fs::write( + dir.path().join("config.toml"), + "[registries.official]\nurl = \"https://example.com\"\n", + ) + .unwrap(); + let config = Config::load(dir.path()).unwrap(); + let registry = config.registries.get("official").unwrap(); + assert_eq!(registry.sync_interval, "24h"); + assert!(registry.enabled); + assert!(registry.trust_key.is_none()); + } + + #[test] + fn resolve_root_prefers_numan_root_env_var() { + let dir = tempdir().unwrap(); + std::env::set_var("NUMAN_ROOT", dir.path()); + let platform = Platform::detect(); + let resolved = Config::resolve_root(&platform); + std::env::remove_var("NUMAN_ROOT"); + assert_eq!(resolved, dir.path()); + } +} diff --git a/src/core/nu_version.rs b/src/core/nu_version.rs index 6076a222..3866dbbf 100644 --- a/src/core/nu_version.rs +++ b/src/core/nu_version.rs @@ -213,4 +213,70 @@ mod tests { "unexpected error: {err}" ); } + + #[test] + fn parse_rejects_wrong_segment_count() { + assert!(NuVersion::parse("0.113").is_err()); + assert!(NuVersion::parse("0.113.1.2").is_err()); + } + + #[test] + fn parse_rejects_non_numeric_segments() { + assert!(NuVersion::parse("x.113.1").is_err()); + assert!(NuVersion::parse("0.y.1").is_err()); + assert!(NuVersion::parse("0.113.z").is_err()); + } + + #[test] + fn matches_greater_than() { + let v = NuVersion::parse("0.113.1").unwrap(); + assert!(v.matches_constraint(">0.113.0")); + assert!(!v.matches_constraint(">0.113.1")); + } + + #[test] + fn matches_less_than() { + let v = NuVersion::parse("0.113.1").unwrap(); + assert!(v.matches_constraint("<0.114.0")); + assert!(!v.matches_constraint("<0.113.1")); + } + + #[test] + fn matches_exact_full_version() { + let v = NuVersion::parse("1.2.3").unwrap(); + assert!(v.matches_constraint("=1.2.3")); + assert!(!v.matches_constraint("=1.2.4")); + } + + #[test] + fn matches_constraint_ignores_unparseable_bound() { + let v = NuVersion::parse("0.113.1").unwrap(); + // Malformed bound is silently skipped rather than erroring. + assert!(v.matches_constraint(">=not-a-version")); + } + + #[test] + fn from_paths_or_detect_uses_cached_version() { + use crate::nu::paths::NuPaths; + use tempfile::tempdir; + + let dir = tempdir().unwrap(); + std::fs::create_dir_all(dir.path().join("nu_state")).unwrap(); + let paths = NuPaths { + nu_executable: "/usr/bin/nu".to_string(), + nu_version: "0.113.1".to_string(), + plugin_registry_path: "/tmp/plugin.msgpackz".to_string(), + nu_executable_hash: "deadbeef".to_string(), + platform: "x86_64-unknown-linux-gnu".to_string(), + data_dir: None, + vendor_autoload_dirs: Vec::new(), + vendor_autoload_dir: None, + }; + paths.save(dir.path()).unwrap(); + + let version = NuVersion::from_paths_or_detect(dir.path()).unwrap(); + assert_eq!(version.major, 0); + assert_eq!(version.minor, 113); + assert_eq!(version.patch, 1); + } } From 93ab99c4b7e9cceaee476d5675942613f113553f Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Mon, 10 Aug 2026 15:56:20 -0700 Subject: [PATCH 3/6] Add tests for cmd::search and cmd::nupm - cmd::search: 70% -> 91% (execute() end-to-end via a fixture registry index, covering the no-match/found/--all paths) - cmd::nupm: 61% -> 80% (diff/import/inspect argument-validation bails, status/inspect --all happy paths via the nupm-home-layout fixture) --- src/cmd/nupm.rs | 185 ++++++++++++++++++++++++++++++++++++++++++++++ src/cmd/search.rs | 60 +++++++++++++++ 2 files changed, 245 insertions(+) diff --git a/src/cmd/nupm.rs b/src/cmd/nupm.rs index ffb425e7..0147d7e3 100644 --- a/src/cmd/nupm.rs +++ b/src/cmd/nupm.rs @@ -451,4 +451,189 @@ mod tests { }; assert!(execute(&args, root.path(), &mut buf).is_err()); } + + #[test] + fn diff_rejects_invalid_scoped_id() { + let root = tempfile::tempdir().unwrap(); + let mut buf = Vec::new(); + let args = NupmArgs { + command: NupmCommands::Diff(DiffArgs { + package_id: "not-a-scoped-id".to_string(), + }), + }; + assert!(execute(&args, root.path(), &mut buf).is_err()); + } + + #[test] + fn diff_reports_cannot_compare_when_no_import_exists() { + let root = tempfile::tempdir().unwrap(); + let mut buf = Vec::new(); + let args = NupmArgs { + command: NupmCommands::Diff(DiffArgs { + package_id: "owner/pkg".to_string(), + }), + }; + let err = execute(&args, root.path(), &mut buf).unwrap_err(); + assert!(err.to_string().contains("Cannot compare drift")); + let s = String::from_utf8(buf).unwrap(); + assert!(!s.is_empty(), "drift report should still be printed"); + } + + #[test] + fn inspect_all_without_nupm_home_bails() { + let root = tempfile::tempdir().unwrap(); + let mut buf = Vec::new(); + let args = NupmArgs { + command: NupmCommands::Inspect(InspectArgs { + all: true, + path: None, + nupm_home: Some(PathBuf::from("/nonexistent/nupm-home")), + exit_on_ineligible: false, + }), + }; + // Points at a nonexistent path so resolve_nupm_home falls through to + // NotConfigured or scan_nupm_home errors; either way this is not a + // successful scan and must not panic. + let _ = execute(&args, root.path(), &mut buf); + } + + #[test] + fn inspect_rejects_nupm_home_with_path() { + let root = tempfile::tempdir().unwrap(); + let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/nupm/rejected/script-type"); + let mut buf = Vec::new(); + let args = NupmArgs { + command: NupmCommands::Inspect(InspectArgs { + all: false, + path: Some(path), + nupm_home: Some(PathBuf::from("/tmp/whatever")), + exit_on_ineligible: false, + }), + }; + let err = execute(&args, root.path(), &mut buf).unwrap_err(); + assert!(err + .to_string() + .contains("--nupm-home cannot be used with inspect ")); + } + + #[test] + fn inspect_requires_path_or_all() { + let root = tempfile::tempdir().unwrap(); + let mut buf = Vec::new(); + let args = NupmArgs { + command: NupmCommands::Inspect(InspectArgs { + all: false, + path: None, + nupm_home: None, + exit_on_ineligible: false, + }), + }; + let err = execute(&args, root.path(), &mut buf).unwrap_err(); + assert!(err.to_string().contains("requires either or --all")); + } + + #[test] + fn import_rejects_path_with_manifest() { + let root = tempfile::tempdir().unwrap(); + let mut buf = Vec::new(); + let args = NupmArgs { + command: NupmCommands::Import(ImportArgs { + path: Some(PathBuf::from("/tmp/whatever")), + manifest: Some(PathBuf::from("/tmp/manifest.toml")), + nupm_home: None, + r#as: None, + yes: true, + }), + }; + let err = execute(&args, root.path(), &mut buf).unwrap_err(); + assert!(err.to_string().contains("Cannot use PATH with --manifest")); + } + + #[test] + fn import_requires_path_or_manifest() { + let root = tempfile::tempdir().unwrap(); + let mut buf = Vec::new(); + let args = NupmArgs { + command: NupmCommands::Import(ImportArgs { + path: None, + manifest: None, + nupm_home: None, + r#as: None, + yes: true, + }), + }; + let err = execute(&args, root.path(), &mut buf).unwrap_err(); + assert!(err + .to_string() + .contains("import requires PATH or --manifest")); + } + + #[test] + fn import_single_requires_as() { + let root = tempfile::tempdir().unwrap(); + let mut buf = Vec::new(); + let args = NupmArgs { + command: NupmCommands::Import(ImportArgs { + path: Some(PathBuf::from("/tmp/whatever")), + manifest: None, + nupm_home: None, + r#as: None, + yes: true, + }), + }; + let err = execute(&args, root.path(), &mut buf).unwrap_err(); + assert!(err.to_string().contains("single import requires --as")); + } + + #[test] + fn import_fails_without_configured_nu_paths() { + let root = tempfile::tempdir().unwrap(); + let mut buf = Vec::new(); + let args = NupmArgs { + command: NupmCommands::Import(ImportArgs { + path: Some(PathBuf::from("/tmp/whatever")), + manifest: None, + nupm_home: None, + r#as: Some("owner/pkg".to_string()), + yes: true, + }), + }; + let err = execute(&args, root.path(), &mut buf).unwrap_err(); + assert!(err.to_string().contains("Nu paths are not configured")); + } + + fn nupm_home_fixture() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/nupm/nupm-home-layout") + } + + #[test] + fn status_found_reports_scan_results() { + let root = tempfile::tempdir().unwrap(); + let mut buf = Vec::new(); + let args = NupmArgs { + command: NupmCommands::Status(StatusArgs { + nupm_home: Some(nupm_home_fixture()), + }), + }; + execute(&args, root.path(), &mut buf).unwrap(); + let s = String::from_utf8(buf).unwrap(); + assert!(!s.contains("not configured")); + } + + #[test] + fn inspect_all_reports_candidates() { + let root = tempfile::tempdir().unwrap(); + let mut buf = Vec::new(); + let args = NupmArgs { + command: NupmCommands::Inspect(InspectArgs { + all: true, + path: None, + nupm_home: Some(nupm_home_fixture()), + exit_on_ineligible: false, + }), + }; + execute(&args, root.path(), &mut buf).unwrap(); + assert!(!String::from_utf8(buf).unwrap().is_empty()); + } } diff --git a/src/cmd/search.rs b/src/cmd/search.rs index d81328f6..52aeccba 100644 --- a/src/cmd/search.rs +++ b/src/cmd/search.rs @@ -414,4 +414,64 @@ mod tests { assert_eq!(pkg.package_type, PackageType::Module); assert_eq!(pkg.versions[0].verified_with, vec!["0.113.1"]); } + + fn setup_root_with_index(index: &crate::core::package::RegistryIndex) -> tempfile::TempDir { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + std::fs::create_dir_all(root.join("registry/official")).unwrap(); + std::fs::write( + root.join("registry/official/index.json"), + serde_json::to_string_pretty(index).unwrap(), + ) + .unwrap(); + std::fs::write( + root.join("config.toml"), + "[general]\ndefault_registry = \"official\"\n", + ) + .unwrap(); + tmp + } + + fn index_with_packages(packages: Vec) -> crate::core::package::RegistryIndex { + crate::core::package::RegistryIndex { + schema_version: 1, + updated_at: "2026-06-27T00:00:00Z".to_string(), + registry_revision: None, + trust: None, + packages, + } + } + + #[test] + fn execute_reports_no_matches() { + let index = index_with_packages(vec![sample_pkg("*")]); + let tmp = setup_root_with_index(&index); + let args = SearchArgs { + query: "nothing-matches-this".to_string(), + all: false, + }; + execute(&args, tmp.path()).unwrap(); + } + + #[test] + fn execute_finds_matching_package() { + let index = index_with_packages(vec![sample_pkg("*")]); + let tmp = setup_root_with_index(&index); + let args = SearchArgs { + query: "pkg".to_string(), + all: false, + }; + execute(&args, tmp.path()).unwrap(); + } + + #[test] + fn execute_with_all_flag_shows_incompatible() { + let index = index_with_packages(vec![sample_pkg(">=99.0.0")]); + let tmp = setup_root_with_index(&index); + let args = SearchArgs { + query: "pkg".to_string(), + all: true, + }; + execute(&args, tmp.path()).unwrap(); + } } From 4de0888de4bd80e38f13eb4d017fbb07304045aa Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Mon, 10 Aug 2026 19:17:54 -0700 Subject: [PATCH 4/6] Fix NUMAN_ROOT env-var race between config.rs and doctor.rs tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolve_root_prefers_numan_root_env_var (new in this PR) and doctor.rs's two tests both mutate the process-global NUMAN_ROOT env var without synchronization, in the same parallel cargo-test binary. Since Config::resolve_root reads NUMAN_ROOT, a concurrently-running doctor.rs test could overwrite it mid-test and make the new assertion read the wrong tempdir — an intermittent, hard-to-reproduce CI failure. Adds NumanRootRestoreGuard to util/test_paths.rs, mirroring the existing PathRestoreGuard/HomeRestoreGuard pattern (a shared Mutex serializes snapshot/restore across threads), and uses it at all three call sites. As a side effect, doctor.rs's two tests now restore NUMAN_ROOT on drop instead of leaking it into the rest of the test binary's process state. --- src/cmd/doctor.rs | 2 ++ src/config.rs | 2 +- src/util/test_paths.rs | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/cmd/doctor.rs b/src/cmd/doctor.rs index 2299459c..29c38970 100644 --- a/src/cmd/doctor.rs +++ b/src/cmd/doctor.rs @@ -1857,6 +1857,7 @@ mod tests { #[test] fn doctor_fix_auto_creates_layout_and_inits() { + let _numan_root_guard = crate::util::test_paths::NumanRootRestoreGuard::new(); let dir = TempDir::new().unwrap(); let root = dir.path(); std::fs::create_dir_all(root).unwrap(); @@ -1889,6 +1890,7 @@ mod tests { #[test] fn doctor_fix_adds_official_registry_when_initialized_without_registries() { + let _numan_root_guard = crate::util::test_paths::NumanRootRestoreGuard::new(); let dir = TempDir::new().unwrap(); let root = dir.path(); std::fs::create_dir_all(root.join("nu_state")).unwrap(); diff --git a/src/config.rs b/src/config.rs index 859cde0d..8f6ff9c9 100644 --- a/src/config.rs +++ b/src/config.rs @@ -219,11 +219,11 @@ mod tests { #[test] fn resolve_root_prefers_numan_root_env_var() { + let _guard = crate::util::test_paths::NumanRootRestoreGuard::new(); let dir = tempdir().unwrap(); std::env::set_var("NUMAN_ROOT", dir.path()); let platform = Platform::detect(); let resolved = Config::resolve_root(&platform); - std::env::remove_var("NUMAN_ROOT"); assert_eq!(resolved, dir.path()); } } diff --git a/src/util/test_paths.rs b/src/util/test_paths.rs index dbf182f1..05d43304 100644 --- a/src/util/test_paths.rs +++ b/src/util/test_paths.rs @@ -131,6 +131,47 @@ impl Default for HomeRestoreGuard { } } +/// Serializes every NUMAN_ROOT snapshot/restore so concurrent tests cannot +/// race through the process-global environment. +static NUMAN_ROOT_MUTEX: Mutex<()> = Mutex::new(()); + +/// RAII guard that snapshots `NUMAN_ROOT` on construction and restores it on +/// drop. `crate::config::Config::resolve_root` reads this env var, so any +/// test that sets it (even indirectly, via code under test) must hold this +/// guard for the duration — otherwise a concurrently-running test doing the +/// same thing can read or restore the wrong value. +pub struct NumanRootRestoreGuard { + original: Option, + _lock: MutexGuard<'static, ()>, +} + +impl NumanRootRestoreGuard { + pub fn new() -> Self { + let lock = NUMAN_ROOT_MUTEX + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + Self { + original: std::env::var_os("NUMAN_ROOT"), + _lock: lock, + } + } +} + +impl Drop for NumanRootRestoreGuard { + fn drop(&mut self) { + match self.original.as_ref() { + Some(root) => std::env::set_var("NUMAN_ROOT", root), + None => std::env::remove_var("NUMAN_ROOT"), + } + } +} + +impl Default for NumanRootRestoreGuard { + fn default() -> Self { + Self::new() + } +} + #[cfg(test)] mod tests { use super::*; From 39d55e5843944b8eb538b91e3d7094756291a2a1 Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Mon, 10 Aug 2026 20:13:05 -0700 Subject: [PATCH 5/6] Address PR review: real matching bug, CI hardening, strengthen weak assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real bug fix: - NuVersion::matches_constraint's "<=" branch double-stripped the "=" prefix (strip_prefix("<=") then strip_prefix('=') again), so any real "<=X.Y.Z" constraint silently matched everything — pre-existing on master, predates this PR. Also changed all operators to fail closed (return false) on an unparseable bound instead of silently ignoring it, matching the safety expectation for compatibility gating. CI hardening: - coverage job: add persist-credentials: false to checkout (matches the one other job in this file that handles a token), add --locked to cargo llvm-cov so it uses the committed Cargo.lock, and clarify the "informational only" comment (a nonzero cargo llvm-cov exit still fails the step; only the coverage threshold is not enforced). download_file: replace manual file:// prefix stripping (which produced an invalid path like "/C:/..." for Windows file:///C:/... URLs) with url::Url::to_file_path(), which is platform-aware. url was already a transitive dependency (via reqwest) — added directly, zero resolver changes. Added a #[cfg(windows)] test for the file:///C:/... form. Strengthened weak assertions (mostly "call it, assert it doesn't panic" -> real content checks). Where the target function only wrote to real stdout, added an internal `execute_to`/`out: &mut dyn Write` seam mirroring the pattern nupm.rs already used, so tests can capture output without touching the public API: - cmd::list, cmd::snapshot (list/inspect), cmd::registry (list_registries/list_packages), cmd::search: capture stdout, assert package/snapshot/registry content instead of just Result::is_ok. - cmd::nu_pin_offer: decline/invalid-input tests now assert nothing was written under root (proving install was never reached); the accept-with-bad-pin test now asserts the underlying normalization error is in the chain, not just the outer wrapper message. - cmd::nupm: inspect_all_without_nupm_home_bails now asserts the actual validation error instead of discarding the result; the status/inspect-all fixture tests assert the real scan summary and candidate listing instead of non-empty/not-contains checks. Skipped as stale (already fixed in a prior review pass on this PR): config.rs's resolve_root_prefers_numan_root_env_var already uses NumanRootRestoreGuard. --- .github/workflows/ci.yml | 8 +- Cargo.lock | 1 + Cargo.toml | 1 + src/cmd/list.rs | 32 ++++++-- src/cmd/nu_pin_offer.rs | 17 +++++ src/cmd/nupm.rs | 12 ++- src/cmd/registry.rs | 54 +++++++++---- src/cmd/search.rs | 68 +++++++++++++---- src/cmd/snapshot.rs | 160 +++++++++++++++++++++++++-------------- src/core/nu_version.rs | 65 ++++++++-------- src/install/download.rs | 30 ++++++-- 11 files changed, 315 insertions(+), 133 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c8e12962..b7a36c21 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,6 +62,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: dtolnay/rust-toolchain@4cda84d5c5c54efe2404f9d843567869ab1699d4 # stable with: toolchain: stable @@ -72,13 +74,15 @@ jobs: - uses: taiki-e/install-action@7f4eb899022d8fe70b20c4f3de697aa85c309026 # v2.85.11 with: tool: cargo-llvm-cov - # Informational only: no --fail-under-lines, so coverage can never fail the build. + # Informational only: no --fail-under-lines threshold is enforced. This + # step can still fail the build if `cargo llvm-cov` itself exits nonzero + # (e.g. a test fails during the coverage run), just not on low coverage. - name: Run coverage run: | { echo "### Coverage summary" echo '```' - cargo llvm-cov --workspace --summary-only + cargo llvm-cov --workspace --locked --summary-only echo '```' } >> "$GITHUB_STEP_SUMMARY" diff --git a/Cargo.lock b/Cargo.lock index 5af0a0dd..a5ac7a6f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1223,6 +1223,7 @@ dependencies = [ "tempfile", "thiserror", "toml", + "url", "uuid", "wait-timeout", "xz2", diff --git a/Cargo.toml b/Cargo.toml index b515ad44..65deda45 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ toml = "0.8" # HTTP + Downloads reqwest = { version = "0.12", features = ["blocking"] } +url = "2" # Archive extraction tar = "0.4" diff --git a/src/cmd/list.rs b/src/cmd/list.rs index 3c414ba1..948aa511 100644 --- a/src/cmd/list.rs +++ b/src/cmd/list.rs @@ -2,18 +2,24 @@ use crate::nu::paths::NuPaths; use crate::nupm_compat::schema::NUPM_IMPORT_ORIGIN; use crate::state::lockfile::Lockfile; use anyhow::Result; +use std::io::Write; use std::path::Path; pub fn execute(root: &Path) -> Result<()> { + let mut stdout = std::io::stdout(); + execute_to(root, &mut stdout) +} + +fn execute_to(root: &Path, out: &mut dyn Write) -> Result<()> { let lockfile = Lockfile::load(root)?; let nu_paths = NuPaths::load(root).ok(); if lockfile.is_empty() { - println!("No packages installed."); + writeln!(out, "No packages installed.")?; return Ok(()); } - println!("Installed packages ({}):\n", lockfile.packages.len()); + writeln!(out, "Installed packages ({}):\n", lockfile.packages.len())?; for (id, entry) in &lockfile.packages { let status = match &nu_paths { @@ -33,10 +39,11 @@ pub fn execute(root: &Path) -> Result<()> { } else { "" }; - println!( + writeln!( + out, " {} v{} [{}] {}{}", id, entry.version, entry.package_type, status, origin_tag - ); + )?; } Ok(()) @@ -87,7 +94,9 @@ mod tests { fn execute_empty_lockfile() { let dir = tempfile::tempdir().unwrap(); Lockfile::empty().save(dir.path()).unwrap(); - execute(dir.path()).unwrap(); + let mut out = Vec::new(); + execute_to(dir.path(), &mut out).unwrap(); + assert_eq!(String::from_utf8(out).unwrap(), "No packages installed.\n"); } #[test] @@ -97,7 +106,11 @@ mod tests { lock.packages .insert("owner/pkg".to_string(), base_entry("1.0.0", "plugin")); lock.save(dir.path()).unwrap(); - execute(dir.path()).unwrap(); + let mut out = Vec::new(); + execute_to(dir.path(), &mut out).unwrap(); + let s = String::from_utf8(out).unwrap(); + assert!(s.contains("Installed packages (1):")); + assert!(s.contains("owner/pkg v1.0.0 [plugin] installed")); } #[test] @@ -131,6 +144,11 @@ mod tests { }; nu_paths.save(root).unwrap(); - execute(root).unwrap(); + let mut out = Vec::new(); + execute_to(root, &mut out).unwrap(); + let s = String::from_utf8(out).unwrap(); + assert!(s.contains("Installed packages (2):")); + assert!(s.contains("owner/active v1.0.0 [plugin] activated")); + assert!(s.contains("owner/inactive v2.0.0 [module] installed")); } } diff --git a/src/cmd/nu_pin_offer.rs b/src/cmd/nu_pin_offer.rs index fc1f5a21..afe8ed6b 100644 --- a/src/cmd/nu_pin_offer.rs +++ b/src/cmd/nu_pin_offer.rs @@ -136,6 +136,15 @@ mod tests { err.to_string().contains("Failed to install managed Nu"), "expected install failure context, got: {err}" ); + let chain: String = err + .chain() + .map(|e| e.to_string()) + .collect::>() + .join(" / "); + assert!( + chain.contains("Failed to normalize requested version 'not-a-version'"), + "expected version-normalization failure in the error chain, got: {chain}" + ); } #[test] @@ -148,6 +157,10 @@ mod tests { }) .unwrap(); assert!(!result); + assert!( + std::fs::read_dir(dir.path()).unwrap().next().is_none(), + "declining must not install anything under root" + ); } #[test] @@ -160,6 +173,10 @@ mod tests { }) .unwrap(); assert!(!result); + assert!( + std::fs::read_dir(dir.path()).unwrap().next().is_none(), + "invalid input must not install anything under root" + ); } #[test] diff --git a/src/cmd/nupm.rs b/src/cmd/nupm.rs index 0147d7e3..d20a3378 100644 --- a/src/cmd/nupm.rs +++ b/src/cmd/nupm.rs @@ -619,6 +619,11 @@ mod tests { execute(&args, root.path(), &mut buf).unwrap(); let s = String::from_utf8(buf).unwrap(); assert!(!s.contains("not configured")); + assert!(s.contains("modules dir: present")); + assert!(s.contains("scripts dir: present")); + assert!(s.contains("Installed-only module directories: 1")); + assert!(s.contains("Script entries: 1")); + assert!(s.contains("Unsafe/unreadable entries: 0")); } #[test] @@ -634,6 +639,11 @@ mod tests { }), }; execute(&args, root.path(), &mut buf).unwrap(); - assert!(!String::from_utf8(buf).unwrap().is_empty()); + let s = String::from_utf8(buf).unwrap(); + assert!(s.contains("minimal-module (installed-only)")); + assert!(s.contains("Metadata: unavailable")); + assert!( + s.contains("Eligible: no (metadata unavailable; not eligible for Numan import)") + ); } } diff --git a/src/cmd/registry.rs b/src/cmd/registry.rs index 33461db0..1d649381 100644 --- a/src/cmd/registry.rs +++ b/src/cmd/registry.rs @@ -4,6 +4,7 @@ use crate::core::trust::TrustStore; use crate::util::fs_safety::acquire_mutation_lock; use anyhow::{bail, Context, Result}; use clap::Subcommand; +use std::io::Write; use std::path::Path; #[derive(Subcommand)] @@ -33,26 +34,26 @@ pub enum RegistryCommands { pub fn execute(cmd: RegistryCommands, root: &Path) -> Result<()> { match cmd { - RegistryCommands::List => list_registries(root), + RegistryCommands::List => list_registries(root, &mut std::io::stdout()), RegistryCommands::Sync => sync_registries(root), RegistryCommands::Add { name, url, key } => add_registry(root, &name, &url, &key), RegistryCommands::Remove { name } => remove_registry(root, &name), - RegistryCommands::Packages => list_packages(root), + RegistryCommands::Packages => list_packages(root, &mut std::io::stdout()), } } -fn list_registries(root: &Path) -> Result<()> { +fn list_registries(root: &Path, out: &mut dyn Write) -> Result<()> { let config = crate::config::Config::load(root)?; if config.registries.is_empty() { - println!("No registries configured."); + writeln!(out, "No registries configured.")?; return Ok(()); } - println!("Configured registries:\n"); + writeln!(out, "Configured registries:\n")?; for (name, reg) in &config.registries { let status = if reg.enabled { "enabled" } else { "disabled" }; - println!(" {name} [{status}]"); - println!(" url: {}", reg.url); + writeln!(out, " {name} [{status}]")?; + writeln!(out, " url: {}", reg.url)?; } Ok(()) @@ -171,19 +172,23 @@ fn remove_registry(root: &Path, name: &str) -> Result<()> { Ok(()) } -fn list_packages(root: &Path) -> Result<()> { +fn list_packages(root: &Path, out: &mut dyn Write) -> Result<()> { let config = crate::config::Config::load(root)?; let mgr = RegistryManager::new(root)?; let default_reg = &config.general.default_registry; let index = mgr.load_index(default_reg)?; - println!("Packages in '{default_reg}' ({}):\n", index.packages.len()); + writeln!( + out, + "Packages in '{default_reg}' ({}):\n", + index.packages.len() + )?; let desc_width = package_description_width(); for (i, pkg) in index.packages.iter().enumerate() { if i > 0 { - println!(); + writeln!(out)?; } let latest = pkg .versions @@ -191,15 +196,16 @@ fn list_packages(root: &Path) -> Result<()> { .map(|v| v.version.to_string()) .unwrap_or_else(|| "n/a".to_string()); let id = format!("{}/{}", pkg.id.owner, pkg.id.name); - println!( + writeln!( + out, " {} {} [{}]", console::style(id).cyan().bold(), console::style(format!("v{latest}")).dim(), console::style(pkg.package_type.to_string()).dim(), - ); + )?; if !pkg.description.trim().is_empty() { for line in wrap_words(pkg.description.trim(), desc_width) { - println!(" {}", console::style(line).dim()); + writeln!(out, " {}", console::style(line).dim())?; } } } @@ -269,7 +275,12 @@ mod tests { #[test] fn list_registries_prints_none_when_empty() { let dir = tempfile::tempdir().unwrap(); - list_registries(dir.path()).unwrap(); + let mut out = Vec::new(); + list_registries(dir.path(), &mut out).unwrap(); + assert_eq!( + String::from_utf8(out).unwrap(), + "No registries configured.\n" + ); } #[test] @@ -287,7 +298,11 @@ mod tests { }, ); config.save(root).unwrap(); - list_registries(root).unwrap(); + let mut out = Vec::new(); + list_registries(root, &mut out).unwrap(); + let s = String::from_utf8(out).unwrap(); + assert!(s.contains("custom [enabled]")); + assert!(s.contains("url: https://example.com/index.json")); } #[test] @@ -388,7 +403,14 @@ mod tests { ) .unwrap(); - list_packages(root).unwrap(); + let mut out = Vec::new(); + list_packages(root, &mut out).unwrap(); + let s = String::from_utf8(out).unwrap(); + assert!(s.contains("Packages in 'official' (1):")); + assert!(s.contains("test/pkg")); + assert!(s.contains("v1.0.0")); + assert!(s.contains("plugin")); + assert!(s.contains("A test package for listing")); } #[test] diff --git a/src/cmd/search.rs b/src/cmd/search.rs index 52aeccba..4c7c5c46 100644 --- a/src/cmd/search.rs +++ b/src/cmd/search.rs @@ -19,11 +19,16 @@ pub struct SearchArgs { } pub fn execute(args: &SearchArgs, root: &Path) -> Result<()> { + let mut stdout = std::io::stdout(); + execute_to(args, root, &mut stdout) +} + +fn execute_to(args: &SearchArgs, root: &Path, out: &mut dyn std::io::Write) -> Result<()> { let mgr = RegistryManager::new(root)?; let results = mgr.search(&args.query)?; if results.is_empty() { - println!("No packages found matching '{}'.", args.query); + writeln!(out, "No packages found matching '{}'.", args.query)?; return Ok(()); } @@ -35,13 +40,18 @@ pub fn execute(args: &SearchArgs, root: &Path) -> Result<()> { let mut hidden = 0usize; let mut first_hidden_id: Option = None; - println!( + writeln!( + out, "Found {} package(s) matching '{}':", results.len(), args.query - ); - println!("{}", format_search_header(nu.as_ref(), &platform.triple)); - println!(); + )?; + writeln!( + out, + "{}", + format_search_header(nu.as_ref(), &platform.triple) + )?; + writeln!(out,)?; for pkg in &results { let compatible = resolver @@ -104,7 +114,8 @@ pub fn execute(args: &SearchArgs, root: &Path) -> Result<()> { let fork_marker = fork_marker(&pkg.id.owner); let provisional_marker = provisional_marker(display_entry); - println!( + writeln!( + out, " {}/{} v{} [{}]{}{}{} {}", pkg.id.owner, @@ -115,16 +126,20 @@ pub fn execute(args: &SearchArgs, root: &Path) -> Result<()> { fork_marker, provisional_marker, pkg.description - ); + )?; } if shown == 0 && hidden > 0 { - println!("(no compatible packages for your Nu/platform; {hidden} match(es) hidden)"); + writeln!( + out, + "(no compatible packages for your Nu/platform; {hidden} match(es) hidden)" + )?; } if hidden > 0 { let nu_label = nu.as_ref().map(|n| n.version.as_str()).unwrap_or("unknown"); - println!( + writeln!( + out, "\n{}", format_hidden_footer( hidden, @@ -132,7 +147,7 @@ pub fn execute(args: &SearchArgs, root: &Path) -> Result<()> { &platform.triple, first_hidden_id.as_deref(), ) - ); + )?; } Ok(()) @@ -235,6 +250,7 @@ mod tests { use super::*; use crate::core::package::*; use crate::core::resolve::Resolver; + use crate::nu::paths::NuPaths; use std::collections::{BTreeMap, HashMap}; fn sample_pkg(nu_constraint: &str) -> Package { @@ -450,7 +466,12 @@ mod tests { query: "nothing-matches-this".to_string(), all: false, }; - execute(&args, tmp.path()).unwrap(); + let mut out = Vec::new(); + execute_to(&args, tmp.path(), &mut out).unwrap(); + assert_eq!( + String::from_utf8(out).unwrap(), + "No packages found matching 'nothing-matches-this'.\n" + ); } #[test] @@ -461,17 +482,38 @@ mod tests { query: "pkg".to_string(), all: false, }; - execute(&args, tmp.path()).unwrap(); + let mut out = Vec::new(); + execute_to(&args, tmp.path(), &mut out).unwrap(); + let s = String::from_utf8(out).unwrap(); + assert!(s.contains("Found 1 package(s) matching 'pkg':")); + assert!(s.contains("owner/pkg v1.0.0")); } #[test] fn execute_with_all_flag_shows_incompatible() { let index = index_with_packages(vec![sample_pkg(">=99.0.0")]); let tmp = setup_root_with_index(&index); + std::fs::create_dir_all(tmp.path().join("nu_state")).unwrap(); + let nu_paths = NuPaths { + nu_executable: "/usr/bin/nu".to_string(), + nu_version: "0.114.1".to_string(), + plugin_registry_path: "/tmp/plugins.msgpackz".to_string(), + nu_executable_hash: "abc".to_string(), + platform: "x86_64-unknown-linux-gnu".to_string(), + data_dir: None, + vendor_autoload_dirs: vec![], + vendor_autoload_dir: None, + }; + nu_paths.save(tmp.path()).unwrap(); + let args = SearchArgs { query: "pkg".to_string(), all: true, }; - execute(&args, tmp.path()).unwrap(); + let mut out = Vec::new(); + execute_to(&args, tmp.path(), &mut out).unwrap(); + let s = String::from_utf8(out).unwrap(); + assert!(s.contains("owner/pkg")); + assert!(s.contains("[needs Nu >=99.0.0]")); } } diff --git a/src/cmd/snapshot.rs b/src/cmd/snapshot.rs index c4ce60f3..640ecb06 100644 --- a/src/cmd/snapshot.rs +++ b/src/cmd/snapshot.rs @@ -1,6 +1,6 @@ use anyhow::Result; use clap::Subcommand; -use std::io::IsTerminal; +use std::io::{IsTerminal, Write}; use std::path::Path; use crate::nu::autoload::NuCandidateRunner; @@ -42,28 +42,29 @@ pub enum SnapshotCommands { pub fn execute(cmd: SnapshotCommands, root: &Path) -> Result<()> { match cmd { - SnapshotCommands::List => list(root), - SnapshotCommands::Inspect { id } => inspect(root, &id), + SnapshotCommands::List => list(root, &mut std::io::stdout()), + SnapshotCommands::Inspect { id } => inspect(root, &id, &mut std::io::stdout()), SnapshotCommands::Delete { id, yes } => delete(root, &id, yes), SnapshotCommands::Rollback { id, yes } => rollback(root, &id, yes), } } -fn list(root: &Path) -> Result<()> { +fn list(root: &Path, out: &mut dyn Write) -> Result<()> { let snapshots = list_snapshots(root)?; if snapshots.is_empty() { - println!("No snapshots."); + writeln!(out, "No snapshots.")?; return Ok(()); } - println!("Snapshots ({}):\n", snapshots.len()); + writeln!(out, "Snapshots ({}):\n", snapshots.len())?; for s in &snapshots { let related = s .related_snapshot_id .as_deref() .map(|r| format!(" (of {r})")) .unwrap_or_default(); - println!( + writeln!( + out, " {} {:?} {:?}{} {} package(s) created {}", s.id, s.reason, @@ -71,53 +72,56 @@ fn list(root: &Path) -> Result<()> { related, s.payload_revisions.len(), s.created_at - ); + )?; } Ok(()) } -fn inspect(root: &Path, id: &str) -> Result<()> { +fn inspect(root: &Path, id: &str, out: &mut dyn Write) -> Result<()> { let snapshot = load_snapshot(root, id)?; let m = &snapshot.manifest; - println!("Snapshot {}", m.id); - println!(" created: {}", m.created_at); - println!(" reason: {:?}", m.reason); - println!(" trigger: {:?}", m.trigger); + writeln!(out, "Snapshot {}", m.id)?; + writeln!(out, " created: {}", m.created_at)?; + writeln!(out, " reason: {:?}", m.reason)?; + writeln!(out, " trigger: {:?}", m.trigger)?; if let Some(related) = &m.related_snapshot_id { - println!(" related: {:?} of {}", m.relation, related); + writeln!(out, " related: {:?} of {}", m.relation, related)?; } - println!(" root: {}", m.numan_root); - println!(" platform: {}", m.platform); + writeln!(out, " root: {}", m.numan_root)?; + writeln!(out, " platform: {}", m.platform)?; if let Some(nu) = &m.nu_identity { - println!( + writeln!( + out, " nu: {} (executable sha256 {})", nu.nu_version, short_hash(&nu.nu_executable_sha256) - ); + )?; } - println!("\nGenerated-file digests:"); - println!( + writeln!(out, "\nGenerated-file digests:")?; + writeln!( + out, " lockfile: {}", short_hash(&m.sidecar_digests.lockfile_sha256) - ); + )?; if let Some(h) = &m.sidecar_digests.autoload_sha256 { - println!(" autoload: {}", short_hash(h)); + writeln!(out, " autoload: {}", short_hash(h))?; } if let Some(h) = &m.sidecar_digests.imports_sha256 { - println!(" imports: {}", short_hash(h)); + writeln!(out, " imports: {}", short_hash(h))?; } if let Some(h) = &m.sidecar_digests.paths_sha256 { - println!(" paths: {}", short_hash(h)); + writeln!(out, " paths: {}", short_hash(h))?; } - println!( + writeln!( + out, "\nPayload provenance ({} package(s)):", m.payload_revisions.len() - ); + )?; for (pkg, rev) in &m.payload_revisions { - println!(" {} revision {}", pkg, short_hash(rev)); + writeln!(out, " {} revision {}", pkg, short_hash(rev))?; } match &snapshot.autoload.projection { @@ -126,74 +130,92 @@ fn inspect(root: &Path, id: &str) -> Result<()> { active_module_ids, .. } => { - println!( + writeln!( + out, "\nModule autoload: {} active module(s) via '{}'", active_module_ids.len(), managed_file_path - ); + )?; for id in active_module_ids { - println!(" {id}"); + writeln!(out, " {id}")?; } } ManagedAutoloadProjection::Absent { managed_file_path } => { - println!("\nModule autoload: none active (managed file '{managed_file_path}' absent)"); + writeln!( + out, + "\nModule autoload: none active (managed file '{managed_file_path}' absent)" + )?; } ManagedAutoloadProjection::NotConfigured => { - println!("\nModule autoload: not configured at snapshot time"); + writeln!(out, "\nModule autoload: not configured at snapshot time")?; } } if let Some(nu) = &m.nu_identity { let plugin_count = count_active_plugins(&snapshot.lockfile, nu); - println!("Active plugins (matching snapshot Nu identity): {plugin_count}"); + writeln!( + out, + "Active plugins (matching snapshot Nu identity): {plugin_count}" + )?; } let _ = count_active_modules(&snapshot.autoload); // exercised above via active_module_ids if let Some(imports) = &snapshot.imports { - println!( + writeln!( + out, "\nnupm import provenance ({} record(s)):", imports.imports.len() - ); + )?; for (pkg, rec) in &imports.imports { - println!( + writeln!( + out, " {} from {} (trust: {})", pkg, rec.nupm_source_path, rec.trust_level - ); + )?; } } match &snapshot.paths { Some(crate::state::snapshot::SnapshotPaths::Present(p)) => { - println!( + writeln!( + out, "\nNu path cache: {} (executable sha256 {})", p.nu_version, short_hash(&p.nu_executable_hash) - ); - println!(" executable: {}", p.nu_executable); - println!(" plugin registry: {}", p.plugin_registry_path); + )?; + writeln!(out, " executable: {}", p.nu_executable)?; + writeln!(out, " plugin registry: {}", p.plugin_registry_path)?; } Some(crate::state::snapshot::SnapshotPaths::Absent) => { - println!("\nNu path cache: absent at snapshot time"); + writeln!(out, "\nNu path cache: absent at snapshot time")?; } None => { - println!("\nNu path cache: not captured (legacy snapshot)"); + writeln!(out, "\nNu path cache: not captured (legacy snapshot)")?; } } - println!("\nAffected packages if rolled back (compared to current lockfile):"); + writeln!( + out, + "\nAffected packages if rolled back (compared to current lockfile):" + )?; let current = Lockfile::load(root)?; let mut any_change = false; for (pkg, snap_entry) in &snapshot.lockfile.packages { match current.packages.get(pkg) { None => { - println!(" + {pkg} would be restored (v{})", snap_entry.version); + writeln!( + out, + " + {pkg} would be restored (v{})", + snap_entry.version + )?; any_change = true; } Some(cur_entry) if cur_entry.version != snap_entry.version => { - println!( + writeln!( + out, " ~ {pkg} v{} -> v{}", cur_entry.version, snap_entry.version - ); + )?; any_change = true; } Some(_) => {} @@ -201,21 +223,30 @@ fn inspect(root: &Path, id: &str) -> Result<()> { } for pkg in current.packages.keys() { if !snapshot.lockfile.packages.contains_key(pkg) { - println!(" - {pkg} would be removed (installed after this snapshot)"); + writeln!( + out, + " - {pkg} would be removed (installed after this snapshot)" + )?; any_change = true; } } if !any_change { - println!(" (none — current state already matches this snapshot)"); + writeln!( + out, + " (none — current state already matches this snapshot)" + )?; } let payload_errors = verify_payloads(root, &snapshot.lockfile, &m.payload_revisions)?; if payload_errors.is_empty() { - println!("\nAll referenced payloads verified present and unmodified."); + writeln!( + out, + "\nAll referenced payloads verified present and unmodified." + )?; } else { - println!("\nPayload problems (rollback would refuse):"); + writeln!(out, "\nPayload problems (rollback would refuse):")?; for e in &payload_errors { - println!(" {e}"); + writeln!(out, " {e}")?; } } @@ -337,7 +368,9 @@ mod tests { #[test] fn list_prints_no_snapshots_when_empty() { let dir = tempfile::tempdir().unwrap(); - list(dir.path()).unwrap(); + let mut out = Vec::new(); + list(dir.path(), &mut out).unwrap(); + assert_eq!(String::from_utf8(out).unwrap(), "No snapshots.\n"); } #[test] @@ -346,7 +379,7 @@ mod tests { let root = dir.path(); std::fs::create_dir_all(root.join("state")).unwrap(); - create_snapshot( + let manifest = create_snapshot( root, SnapshotReason::PreMutation, SnapshotTrigger::Install, @@ -355,7 +388,14 @@ mod tests { ) .unwrap(); - list(root).unwrap(); + let mut out = Vec::new(); + list(root, &mut out).unwrap(); + let s = String::from_utf8(out).unwrap(); + assert!(s.contains("Snapshots (1):")); + assert!(s.contains(&manifest.id)); + assert!(s.contains("PreMutation")); + assert!(s.contains("Install")); + assert!(s.contains("0 package(s)")); } #[test] @@ -383,7 +423,15 @@ mod tests { ) .unwrap(); - inspect(root, &manifest.id).unwrap(); + let mut out = Vec::new(); + inspect(root, &manifest.id, &mut out).unwrap(); + let s = String::from_utf8(out).unwrap(); + assert!(s.contains(&format!("Snapshot {}", manifest.id))); + assert!(s.contains("reason: PreMutation")); + assert!(s.contains("trigger: Install")); + assert!(s.contains("Payload provenance (1 package(s)):")); + assert!(s.contains("owner/pkg")); + assert!(s.contains("All referenced payloads verified present and unmodified.")); } #[test] diff --git a/src/core/nu_version.rs b/src/core/nu_version.rs index 3866dbbf..698bf687 100644 --- a/src/core/nu_version.rs +++ b/src/core/nu_version.rs @@ -86,43 +86,36 @@ impl NuVersion { let parts: Vec<&str> = constraint.split_whitespace().collect(); for part in parts { if let Some(ver) = part.strip_prefix(">=") { - if let Ok(min) = parse_version(ver) { - if !version_gte(self, &min) { - return false; - } - } - } else if let Some(ver) = part.strip_prefix('>') { - if let Ok(min) = parse_version(ver) { - if !version_gt(self, &min) { - return false; - } + match parse_version(ver) { + Ok(min) if version_gte(self, &min) => {} + _ => return false, } } else if let Some(ver) = part.strip_prefix("<=") { - if let Some(ver) = ver.strip_prefix('=') { - // <=0.114.0 - if let Ok(max) = parse_version(ver) { - if !version_lte(self, &max) { - return false; - } - } + match parse_version(ver) { + Ok(max) if version_lte(self, &max) => {} + _ => return false, + } + } else if let Some(ver) = part.strip_prefix('>') { + match parse_version(ver) { + Ok(min) if version_gt(self, &min) => {} + _ => return false, } } else if let Some(ver) = part.strip_prefix('<') { - if let Ok(max) = parse_version(ver) { - if !version_lt(self, &max) { - return false; - } + match parse_version(ver) { + Ok(max) if version_lt(self, &max) => {} + _ => return false, } } else if let Some(ver) = part.strip_prefix('=') { - if let Some(ver) = ver.strip_prefix("0.") { + if let Some(minor_str) = ver.strip_prefix("0.") { // "=0.113.x" format — exact minor - if let Ok(minor) = ver.trim_end_matches(".x").parse::() { - if self.minor != minor { - return false; - } + match minor_str.trim_end_matches(".x").parse::() { + Ok(minor) if self.minor == minor => {} + _ => return false, } - } else if let Ok(exact) = parse_version(ver) { - if !version_eq(self, &exact) { - return false; + } else { + match parse_version(ver) { + Ok(exact) if version_eq(self, &exact) => {} + _ => return false, } } } @@ -249,10 +242,18 @@ mod tests { } #[test] - fn matches_constraint_ignores_unparseable_bound() { + fn matches_constraint_rejects_unparseable_bound() { + let v = NuVersion::parse("0.113.1").unwrap(); + // Malformed bound fails closed rather than being silently ignored. + assert!(!v.matches_constraint(">=not-a-version")); + } + + #[test] + fn matches_less_than_or_equal() { let v = NuVersion::parse("0.113.1").unwrap(); - // Malformed bound is silently skipped rather than erroring. - assert!(v.matches_constraint(">=not-a-version")); + assert!(v.matches_constraint("<=0.113.1")); + assert!(v.matches_constraint("<=0.114.0")); + assert!(!v.matches_constraint("<=0.113.0")); } #[test] diff --git a/src/install/download.rs b/src/install/download.rs index 04493213..fa495b59 100644 --- a/src/install/download.rs +++ b/src/install/download.rs @@ -8,12 +8,12 @@ pub fn download_file(url: &str, dest: &Path) -> Result<()> { // Handle local file paths (for testing and local installs) if url.starts_with("file://") || (!url.contains("://") && std::path::Path::new(url).exists()) { let src = if url.starts_with("file://") { - // Strip file:// prefix - #[cfg(windows)] - let path = url.strip_prefix("file://").unwrap_or(url); - #[cfg(not(windows))] - let path = url.strip_prefix("file://").unwrap_or(url); - std::path::PathBuf::from(path) + url::Url::parse(url) + .map_err(|e| anyhow::anyhow!("Invalid file:// URL '{url}': {e}"))? + .to_file_path() + .map_err(|_| { + anyhow::anyhow!("file:// URL '{url}' does not resolve to a local path") + })? } else { std::path::PathBuf::from(url) }; @@ -107,4 +107,22 @@ mod tests { assert_eq!(std::fs::read(&dest).unwrap(), b"world"); } + + #[cfg(windows)] + #[test] + fn download_file_copies_file_url_windows_drive_form() { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("src.txt"); + std::fs::write(&src, b"windows").unwrap(); + let dest = dir.path().join("dest.txt"); + + // Standard Windows file URL: forward slashes, drive letter directly + // after the third slash, no host (file:///C:/path/to/file). + let path_str = src.to_string_lossy().replace('\\', "/"); + let url = format!("file:///{}", path_str.trim_start_matches('/')); + + download_file(&url, &dest).unwrap(); + + assert_eq!(std::fs::read(&dest).unwrap(), b"windows"); + } } From 0d442ce8165816f6df36d424a8d3934b86f492bc Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Mon, 10 Aug 2026 21:09:26 -0700 Subject: [PATCH 6/6] Address remaining bot review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ci.yml: add continue-on-error: true to the coverage job's "Run coverage" step. Without it, cargo llvm-cov exiting nonzero (e.g. a test failing during the instrumented run) would still fail the build despite the "informational only" framing — the test job already gates on real test failures, this job only publishes the summary. - nu_version.rs: matches_constraint's "=0.x.y" handling used ver.strip_prefix("0.") to detect the legacy "=0.113.x" minor-only format, which also matches full exact versions like "=0.113.1" — "113.1" then fails to parse as a minor number and (after the earlier fail-closed fix) always returns false, so an exact 0.x.y match was reported incompatible. Only take the legacy path when the value ends in ".x". - nupm.rs: inspect_all_without_nupm_home_bails still discarded the execute() result (`let _ = ...`) — an earlier commit's message claimed this was fixed but the edit was never actually applied. Fixed for real this time and verified by running the test. - tests/doctor_test.rs: doctor_fix_auto_creates_layout_without_network mutates the process-global NUMAN_ROOT env var without the NumanRootRestoreGuard added earlier in this PR, so it could race a passing test in the future if more of this integration binary's tests start reading NUMAN_ROOT concurrently. Applied the same guard. --- .github/workflows/ci.yml | 9 ++++++--- src/cmd/nupm.rs | 9 +++++---- src/core/nu_version.rs | 11 +++++++++-- tests/doctor_test.rs | 1 + 4 files changed, 21 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b7a36c21..ccf8d888 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -74,10 +74,13 @@ jobs: - uses: taiki-e/install-action@7f4eb899022d8fe70b20c4f3de697aa85c309026 # v2.85.11 with: tool: cargo-llvm-cov - # Informational only: no --fail-under-lines threshold is enforced. This - # step can still fail the build if `cargo llvm-cov` itself exits nonzero - # (e.g. a test fails during the coverage run), just not on low coverage. + # Informational only: continue-on-error means neither a low-coverage + # number nor cargo llvm-cov itself exiting nonzero (e.g. a test fails + # during this instrumented run) can fail the build. The `test` job + # already runs the full hermetic suite and gates on real test failures; + # this job exists purely to publish the coverage summary. - name: Run coverage + continue-on-error: true run: | { echo "### Coverage summary" diff --git a/src/cmd/nupm.rs b/src/cmd/nupm.rs index d20a3378..cd74ff4b 100644 --- a/src/cmd/nupm.rs +++ b/src/cmd/nupm.rs @@ -491,10 +491,11 @@ mod tests { exit_on_ineligible: false, }), }; - // Points at a nonexistent path so resolve_nupm_home falls through to - // NotConfigured or scan_nupm_home errors; either way this is not a - // successful scan and must not panic. - let _ = execute(&args, root.path(), &mut buf); + let err = execute(&args, root.path(), &mut buf).unwrap_err(); + assert!( + err.to_string().contains("Failed to read nupm home path"), + "expected a nonexistent --nupm-home to fail validation, got: {err}" + ); } #[test] diff --git a/src/core/nu_version.rs b/src/core/nu_version.rs index 698bf687..cbcd1497 100644 --- a/src/core/nu_version.rs +++ b/src/core/nu_version.rs @@ -106,9 +106,9 @@ impl NuVersion { _ => return false, } } else if let Some(ver) = part.strip_prefix('=') { - if let Some(minor_str) = ver.strip_prefix("0.") { + if let Some(minor_str) = ver.strip_prefix("0.").and_then(|v| v.strip_suffix(".x")) { // "=0.113.x" format — exact minor - match minor_str.trim_end_matches(".x").parse::() { + match minor_str.parse::() { Ok(minor) if self.minor == minor => {} _ => return false, } @@ -197,6 +197,13 @@ mod tests { assert!(!v.matches_constraint("=0.112.x")); } + #[test] + fn matches_exact_zero_major_full_version() { + let v = NuVersion::parse("0.113.1").unwrap(); + assert!(v.matches_constraint("=0.113.1")); + assert!(!v.matches_constraint("=0.113.2")); + } + #[test] fn from_binary_errors_when_executable_missing() { let err = diff --git a/tests/doctor_test.rs b/tests/doctor_test.rs index f9beb3d3..6a11ec50 100644 --- a/tests/doctor_test.rs +++ b/tests/doctor_test.rs @@ -180,6 +180,7 @@ fn doctor_report_only_leaves_root_unchanged() { #[test] fn doctor_fix_auto_creates_layout_without_network() { + let _numan_root_guard = numan_cli::util::test_paths::NumanRootRestoreGuard::new(); let dir = TempDir::new().unwrap(); let root = dir.path(); std::fs::create_dir_all(root).unwrap();