diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f6b5a39..ccf8d88 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,6 +57,38 @@ jobs: components: rustfmt - run: cargo fmt --all -- --check + coverage: + name: Coverage + 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 + 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: 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" + echo '```' + cargo llvm-cov --workspace --locked --summary-only + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + roadmap-drift: name: Roadmap drift runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index fc1cf99..ad46431 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,6 @@ desktop.ini # Python __pycache__/ *.py[cod] + +# Coverage +*.profraw diff --git a/Cargo.lock b/Cargo.lock index 5af0a0d..a5ac7a6 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 b515ad4..65deda4 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/doctor.rs b/src/cmd/doctor.rs index 2299459..29c3897 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/cmd/list.rs b/src/cmd/list.rs index c45f6d1..948aa51 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,11 +39,116 @@ pub fn execute(root: &Path) -> Result<()> { } else { "" }; - println!( + writeln!( + out, " {} v{} [{}] {}{}", id, entry.version, entry.package_type, status, origin_tag - ); + )?; } 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(); + let mut out = Vec::new(); + execute_to(dir.path(), &mut out).unwrap(); + assert_eq!(String::from_utf8(out).unwrap(), "No packages installed.\n"); + } + + #[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(); + 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] + 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(); + + 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 536c529..afe8ed6 100644 --- a/src/cmd/nu_pin_offer.rs +++ b/src/cmd/nu_pin_offer.rs @@ -106,3 +106,88 @@ 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}" + ); + 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] + 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); + assert!( + std::fs::read_dir(dir.path()).unwrap().next().is_none(), + "declining must not install anything under root" + ); + } + + #[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); + assert!( + std::fs::read_dir(dir.path()).unwrap().next().is_none(), + "invalid input must not install anything under root" + ); + } + + #[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/nupm.rs b/src/cmd/nupm.rs index ffb425e..cd74ff4 100644 --- a/src/cmd/nupm.rs +++ b/src/cmd/nupm.rs @@ -451,4 +451,200 @@ 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, + }), + }; + 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] + 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")); + 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] + 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(); + 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 a4ce35a..1d64938 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())?; } } } @@ -258,6 +264,155 @@ 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(); + let mut out = Vec::new(); + list_registries(dir.path(), &mut out).unwrap(); + assert_eq!( + String::from_utf8(out).unwrap(), + "No registries configured.\n" + ); + } + + #[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(); + 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] + 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(); + + 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] fn wrap_words_keeps_short_text_on_one_line() { assert_eq!( diff --git a/src/cmd/remove.rs b/src/cmd/remove.rs index b9dec65..e75658e 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/cmd/search.rs b/src/cmd/search.rs index d81328f..4c7c5c4 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 { @@ -414,4 +430,90 @@ 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, + }; + 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] + 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, + }; + 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, + }; + 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 d0a62fe..640ecb0 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}")?; } } @@ -287,9 +318,122 @@ 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(); + let mut out = Vec::new(); + list(dir.path(), &mut out).unwrap(); + assert_eq!(String::from_utf8(out).unwrap(), "No snapshots.\n"); + } + + #[test] + fn list_prints_committed_snapshots() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + std::fs::create_dir_all(root.join("state")).unwrap(); + + let manifest = create_snapshot( + root, + SnapshotReason::PreMutation, + SnapshotTrigger::Install, + None, + None, + ) + .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] + 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(); + + 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] fn delete_refuses_non_tty_without_yes() { // Force non-TTY via the injectable seam so the guard is deterministic diff --git a/src/config.rs b/src/config.rs index b27cc72..8f6ff9c 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 _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); + assert_eq!(resolved, dir.path()); + } +} diff --git a/src/core/nu_version.rs b/src/core/nu_version.rs index 6076a22..cbcd149 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.").and_then(|v| v.strip_suffix(".x")) { // "=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.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, } } } @@ -204,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 = @@ -213,4 +213,78 @@ 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_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(); + assert!(v.matches_constraint("<=0.113.1")); + assert!(v.matches_constraint("<=0.114.0")); + assert!(!v.matches_constraint("<=0.113.0")); + } + + #[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); + } } diff --git a/src/install/download.rs b/src/install/download.rs index 1df4b9d..fa495b5 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) }; @@ -78,3 +78,51 @@ 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"); + } + + #[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"); + } +} diff --git a/src/util/test_paths.rs b/src/util/test_paths.rs index dbf182f..05d4330 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::*; diff --git a/tests/doctor_test.rs b/tests/doctor_test.rs index f9beb3d..6a11ec5 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();