diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 7cc01469e..7a9f04e64 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -1506,6 +1506,37 @@ provisions `pytest`, `pyyaml`, and `hypothesis` through `uv run --with`, so `uv` is the only prerequisite and no virtual environment needs creating by hand. +### Configuration-precedence regression tests + +The config-precedence ladder and display-policy domain are covered by three +modules under `tests/cli_tests/`: + +- `config_precedence_ladder.rs` pins the closed selector model (`--config` > + `NETSUKE_CONFIG` > automatic discovery) end to end and checks that the + merged scalar fields follow CLI > environment > project > discovered + (user/system) > defaults. It includes an explicit guard that the removed + `NETSUKE_CONFIG_PATH` alias is not a selector, even when it names an + existing file with distinct values. +- `display_policy_domain.rs` exhaustively verifies the consolidated + display-policy resolution (`EmojiPolicy`, `ColourPolicy`, + `ProgressPolicy`, `AccessibilityPolicy`, `json`, `NO_COLOR`, and + `TERM`/output mode) against a handwritten truth model, using one flat + Cartesian-product sweep plus a proptest. It adds coverage only; the + production resolution in `src/theme.rs` and `src/output_prefs.rs` is not + changed. +- `merge_targets_proptests.rs` holds the handwritten proptest strategies (no + `#[derive(Arbitrary)]`) for the `default_targets` append-in-discovery-order + invariant and scalar merge ordering (defaults → file → environment → CLI). + +These tests drive a re-executed worker process through +`tests/cli_tests/merge_probe.rs`. `merge_probe` builds an isolated +environment (`HOME`, `XDG_CONFIG_HOME`, `XDG_CONFIG_DIRS`, and, for the +system-scope variants, a redirectable `XDG_CONFIG_DIRS`) and `merge_in_child` +runs the real ambient adapters in a child process, so the parent harness never +mutates the process environment. The XDG system/user scope scenarios are +Unix-only: Windows discovers configuration through `APPDATA`/`LOCALAPPDATA` +rather than the XDG variables these tests inject. + ### Temporary executable test helpers The low-level executable-stub primitive is owned by diff --git a/docs/roadmap.md b/docs/roadmap.md index d51b8310e..514dabc8d 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -130,10 +130,28 @@ and agents. `NETSUKE_CONFIG`) verified by exhaustive rstest cases and a proptest property test (PR `#327`, closes `#291`). - [ ] Depend on OrthoConfig `5.2.3` for consumer boundary guidance. - - [ ] Preserve Netsuke-specific precedence expectations for manifest path, - display policies, locale, and profile selection. - - [ ] Verify that CLI flags override environment, profile, project, user, - system, and default configuration layers. + OrthoConfig `5.2.3` is an upstream OrthoConfig roadmap identifier, not a + crate version; Netsuke stays pinned at `ortho_config = "0.9.0"` until + that guidance ships. Blocked on upstream; no `Cargo.toml` change. + - [x] Preserve Netsuke-specific precedence expectations for manifest path, + display policies, and locale across the two-selector ladder + (`--config` > `NETSUKE_CONFIG` > automatic discovery), where the + discovered rung is a single exclusive winner among system scope, user + scope, and defaults. When a user-scope configuration wins over the system + scope, system-only fields are not merged through and fall back to their + defaults (regression tests added in issue `#385`). + - [ ] Preserve Netsuke-specific precedence expectations for profile + selection (deferred to 5.3.1, when the `--profile` flag lands). + - [x] Verify that CLI flags override environment, project, user, system, + and default configuration layers for scalar fields (manifest path, + display policies, locale, jobs) in issue `#385`. + - [ ] Verify that CLI flags override the profile configuration layer + (deferred to 5.3.1, when the `--profile` flag lands). + + - Note: OrthoConfig automatic discovery is exclusive: one discovered file + wins, so system-only fields are absent when a user-scope file is + selected (user-over-system and system-only merge-through cases are + covered by the regression tests added in issue #385). ### 3.12. Terminal rendering verification diff --git a/tests/cli_tests/config_discovery_scopes.rs b/tests/cli_tests/config_discovery_scopes.rs index 56a1576ca..ffc7353f7 100644 --- a/tests/cli_tests/config_discovery_scopes.rs +++ b/tests/cli_tests/config_discovery_scopes.rs @@ -2,6 +2,8 @@ //! project-file discovery, user-scope fallback, and project-over-user //! precedence on Unix and Windows. +#[cfg(unix)] +use super::super::merge_probe::environment_with_system_scope; use super::super::merge_probe::{isolated_environment, merge_in_child}; use anyhow::{Context, Result, ensure}; use netsuke::cli::config::{ColourPolicy, EmojiPolicy}; @@ -221,3 +223,174 @@ fn project_config_takes_precedence_over_user_config() -> Result<()> { )?; assert_project_precedence_applied(&merged) } + +/// System-scope config content used by the Unix variants. Windows discovers +/// user and system configuration through `APPDATA`/`LOCALAPPDATA` rather than +/// the XDG variables these tests inject, so the scenarios below are Unix-only. +#[cfg(unix)] +const SYSTEM_CONFIG_CONTENT: &str = r#" +file = "Systemfile" +emoji = "always" +color = "always" +jobs = 9 +locale = "de-DE" +"#; + +#[cfg(unix)] +fn assert_system_config_applied(merged: &netsuke::cli::Cli) -> Result<()> { + ensure!( + merged.file.as_path() == Path::new("Systemfile"), + "system config manifest path should be discovered when no user or project config exists" + ); + ensure!( + merged.emoji == EmojiPolicy::Always, + "system config emoji policy should be discovered when no user or project config exists" + ); + ensure!( + merged.color == ColourPolicy::Always, + "system config color policy should be discovered" + ); + ensure!( + merged.jobs == Some(9), + "system config jobs should be discovered" + ); + ensure!( + merged.locale.as_deref() == Some("de-DE"), + "system config locale should be discovered" + ); + Ok(()) +} + +/// Write discovery-scope config files and merge in an isolated child rooted at +/// `project`. The selector set stays closed at the documented variables; the +/// `system` `TempDir` is discarded after the child exits. Unix-only because the +/// injected environment uses the XDG variables that Windows does not read. +#[cfg(unix)] +fn run_system_scope_scenario( + project: &Path, + home: &Path, + system: &Path, + scopes: &ScopeLayers, +) -> Result { + let system_dir = system.join("netsuke"); + fs::create_dir_all(&system_dir) + .with_context(|| format!("create system config directory {}", system_dir.display()))?; + fs::write(system_dir.join("config.toml"), SYSTEM_CONFIG_CONTENT) + .context("write system config")?; + if let Some(user_content) = scopes.user_config { + let user_dir = home.join(".config").join("netsuke"); + fs::create_dir_all(&user_dir).context("create user config directory")?; + fs::write(user_dir.join("config.toml"), user_content).context("write user config")?; + } + if let Some(project_content) = scopes.project_config { + fs::write(project.join(".netsuke.toml"), project_content) + .context("write project config")?; + } + let environment = environment_with_system_scope(home, system, &[]); + merge_in_child(&["netsuke"], project, &environment) +} + +/// Optional user- and project-scope layers for a system-scope discovery run. +#[cfg(unix)] +#[derive(Default)] +struct ScopeLayers { + user_config: Option<&'static str>, + project_config: Option<&'static str>, +} + +/// System-scope discovery is platform-neutral here: `run_system_scope_scenario` +/// points the child at an isolated `XDG_CONFIG_DIRS` via the injected +/// environment seam. The XDG variables are not read on Windows (which uses +/// `APPDATA`/`LOCALAPPDATA`), so the discovery scenario is Unix-only. +#[cfg(unix)] +#[rstest] +fn system_scope_config_discovered_when_no_user_or_project_config() -> Result<()> { + let temp_project = tempdir().context("create temporary project directory")?; + let temp_home = tempdir().context("create temporary home directory")?; + let temp_system = tempdir().context("create temporary system directory")?; + let merged = run_system_scope_scenario( + temp_project.path(), + temp_home.path(), + temp_system.path(), + &ScopeLayers::default(), + )?; + assert_system_config_applied(&merged) +} + +#[cfg(unix)] +#[rstest] +fn user_scope_config_takes_precedence_over_system_scope() -> Result<()> { + let temp_project = tempdir().context("create temporary project directory")?; + let temp_home = tempdir().context("create temporary home directory")?; + let temp_system = tempdir().context("create temporary system directory")?; + let merged = run_system_scope_scenario( + temp_project.path(), + temp_home.path(), + temp_system.path(), + &ScopeLayers { + user_config: Some("emoji = \"never\"\njobs = 4\n"), + ..ScopeLayers::default() + }, + )?; + // OrthoConfig discovery is exclusive: the user layer wins over the system + // layer for every overlapping field, and the system file is not merged. + ensure!( + merged.file.as_path() == Path::new("Netsukefile"), + "manifest path should fall back to the default when the system layer loses, got {:?}", + merged.file + ); + ensure!( + merged.emoji == EmojiPolicy::Never, + "user config emoji policy should override system config" + ); + ensure!( + merged.jobs == Some(4), + "user config jobs should override system config" + ); + ensure!( + merged.color == ColourPolicy::Auto, + "system color field should not appear when the user layer wins" + ); + Ok(()) +} + +/// Project-scope config coexists with a system-scope file: the project layer +/// outranks the system layer for overlapping fields, and a system-only field +/// still merges through. +#[cfg(unix)] +#[rstest] +fn project_config_overrides_system_and_system_only_field_merges() -> Result<()> { + let temp_project = tempdir().context("create temporary project directory")?; + let temp_home = tempdir().context("create temporary home directory")?; + let temp_system = tempdir().context("create temporary system directory")?; + let merged = run_system_scope_scenario( + temp_project.path(), + temp_home.path(), + temp_system.path(), + &ScopeLayers { + project_config: Some("emoji = \"never\"\njobs = 4\n"), + ..ScopeLayers::default() + }, + )?; + ensure!( + merged.file.as_path() == Path::new("Systemfile"), + "system-only manifest path should merge through when project config does not set it" + ); + ensure!( + merged.emoji == EmojiPolicy::Never, + "project config emoji policy should override system config" + ); + ensure!( + merged.jobs == Some(4), + "project config jobs should override system config" + ); + ensure!( + merged.color == ColourPolicy::Always, + "system-only color field should merge through when project config does not set it" + ); + ensure!( + merged.locale.as_deref() == Some("de-DE"), + "system-only locale should merge through when project config does not set it" + ); + Ok(()) +} diff --git a/tests/cli_tests/config_precedence_ladder.rs b/tests/cli_tests/config_precedence_ladder.rs new file mode 100644 index 000000000..c4d6e90a1 --- /dev/null +++ b/tests/cli_tests/config_precedence_ladder.rs @@ -0,0 +1,392 @@ +//! End-to-end configuration precedence ladder tests. +//! +//! These tests exercise the complete implemented precedence ladder from the +//! lowest layer to the highest for scalar fields: +//! +//! ```text +//! CLI flags (--file, --emoji, --jobs, --locale, --color) +//! Environment (NETSUKE_FILE, NETSUKE_EMOJI, ...) +//! Project scope (.netsuke.toml, appended on top of discovery) +//! Discovered scope (XDG_CONFIG_HOME > XDG_CONFIG_DIRS > $HOME dotfile) +//! Defaults (CliConfig::default) +//! ``` +//! +//! The profile rung (`--profile`) is not implemented yet; it is tracked by +//! roadmap item 5.3.1 and will sit between the project scope and the +//! environment layer. No test asserts unimplemented profile behaviour. +//! +//! The environment selector set is closed at `NETSUKE_CONFIG` (ADR-004). +//! [`NETSUKE_CONFIG_PATH`][2] is never set as a selector here; a guard test +//! confirms it has no effect on selection. +//! +//! [2]: https://github.com/leynos/netsuke/blob/main/docs/adr-004-explicit-config-selection-outside-orthoconfig.md + +use super::merge_probe::{environment_with_system_scope, merge_in_child}; +use anyhow::{Context, Result, ensure}; +use netsuke::cli::{Cli, EmojiPolicy, config::ColourPolicy}; +use rstest::rstest; +use std::ffi::OsString; +use std::path::Path; +use tempfile::tempdir; +use test_support::fs as test_fs; + +/// Build the full ladder environment with a distinct value per layer. +/// +/// Layer-to-value mapping (all for the same field set): +/// - system scope: `XDG_CONFIG_DIRS/netsuke/config.toml` +/// - user scope: `XDG_CONFIG_HOME/netsuke/config.toml` +/// - project scope: `project/.netsuke.toml` +/// - environment: `NETSUKE_*` overrides +/// +/// Returns the environment vector (caller keeps the `TempDir`s alive). +fn ladder_environment( + project: &Path, + home: &Path, + system: &Path, + enabled_layers: &[Layer], +) -> Result> { + // System scope (lowest discovered layer). + if enabled_layers.contains(&Layer::System) { + write_scope_config(system.join("netsuke").join("config.toml"), SYSTEM_CONFIG)?; + } + // User scope. + if enabled_layers.contains(&Layer::User) { + let user_config = home.join(".config").join("netsuke").join("config.toml"); + write_scope_config(user_config, USER_CONFIG)?; + } + // Project scope. + if enabled_layers.contains(&Layer::Project) { + write_scope_config(project.join(".netsuke.toml"), PROJECT_CONFIG)?; + } + + let mut environment = environment_with_system_scope( + home, + system, + &[(OsString::from("NETSUKE_FILE"), OsString::from("Envfile"))], + ); + environment.extend([ + (OsString::from("NETSUKE_EMOJI"), OsString::from("never")), + (OsString::from("NETSUKE_JOBS"), OsString::from("16")), + (OsString::from("NETSUKE_LOCALE"), OsString::from("it-IT")), + (OsString::from("NETSUKE_COLOR"), OsString::from("never")), + ]); + if !enabled_layers.contains(&Layer::Environment) { + // Clear the NETSUKE_* value layer so it does not participate. + environment.retain(|(key, _)| !key.to_string_lossy().starts_with("NETSUKE_")); + } + Ok(environment) +} + +/// Write a per-scope configuration file, creating parent directories first. +fn write_scope_config(path: impl AsRef, contents: &'static str) -> Result<()> { + let target = path.as_ref(); + test_fs::create_dir_all( + target + .parent() + .context("config file has no parent directory")?, + ) + .context("create config directory")?; + test_fs::write(target, contents).with_context(|| format!("write {}", target.display()))?; + Ok(()) +} + +/// System-scope (lowest discovered) configuration values. +const SYSTEM_CONFIG: &str = r#" +file = "Systemfile" +emoji = "always" +jobs = 9 +locale = "de-DE" +color = "always" +"#; + +/// User-scope configuration values. +const USER_CONFIG: &str = r#" +file = "Userfile" +emoji = "never" +jobs = 4 +locale = "fr-FR" +color = "never" +"#; + +/// Project-scope configuration values. +const PROJECT_CONFIG: &str = r#" +file = "Projectfile" +emoji = "never" +jobs = 8 +locale = "es-ES" +color = "never" +"#; + +/// The discovered or injected layers the ladder test may seed. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Layer { + System, + User, + Project, + Environment, +} + +/// Expected merged values for one ladder rung. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct LadderExpectation { + file: &'static str, + emoji: EmojiPolicy, + jobs: Option, + locale: Option<&'static str>, + color: ColourPolicy, +} + +impl LadderExpectation { + const fn defaults() -> Self { + Self { + file: "Netsukefile", + emoji: EmojiPolicy::Auto, + jobs: None, + locale: None, + color: ColourPolicy::Auto, + } + } +} + +fn assert_ladder(merged: &Cli, expected: LadderExpectation) -> Result<()> { + ensure!( + merged.file.as_path() == Path::new(expected.file), + "manifest path should be {:?}, got {:?}", + expected.file, + merged.file + ); + ensure!( + merged.emoji == expected.emoji, + "emoji policy should be {:?}, got {:?}", + expected.emoji, + merged.emoji + ); + ensure!( + merged.jobs == expected.jobs, + "jobs should be {:?}, got {:?}", + expected.jobs, + merged.jobs + ); + ensure!( + merged.locale.as_deref() == expected.locale, + "locale should be {:?}, got {:?}", + expected.locale, + merged.locale + ); + ensure!( + merged.color == expected.color, + "color policy should be {:?}, got {:?}", + expected.color, + merged.color + ); + Ok(()) +} + +/// Merge a ladder scenario with the given CLI/extra env layer enabled and +/// return the merged CLI. +fn run_ladder_scenario( + enabled_layers: &[Layer], + cli_args: &[&str], + extra_env: &[(OsString, OsString)], +) -> Result { + let project = tempdir().context("create project directory")?; + let home = tempdir().context("create home directory")?; + let system = tempdir().context("create system directory")?; + let mut environment = + ladder_environment(project.path(), home.path(), system.path(), enabled_layers)?; + environment.extend_from_slice(extra_env); + merge_in_child(cli_args, project.path(), &environment) +} + +/// Cases where the environment layer alone supplies the value for each rung. +#[rstest] +#[case::defaults_only(&[], &["netsuke"], &[], LadderExpectation::defaults())] +#[case::system_only( + &[Layer::System], + &["netsuke"], + &[], + LadderExpectation { + file: "Systemfile", + emoji: EmojiPolicy::Always, + jobs: Some(9), + locale: Some("de-DE"), + color: ColourPolicy::Always, + } +)] +#[case::user_overrides_system( + &[Layer::System, Layer::User], + &["netsuke"], + &[], + LadderExpectation { + file: "Userfile", + emoji: EmojiPolicy::Never, + jobs: Some(4), + locale: Some("fr-FR"), + color: ColourPolicy::Never, + } +)] +#[case::project_overrides_user_and_system( + &[Layer::System, Layer::User, Layer::Project], + &["netsuke"], + &[], + LadderExpectation { + file: "Projectfile", + emoji: EmojiPolicy::Never, + jobs: Some(8), + locale: Some("es-ES"), + color: ColourPolicy::Never, + } +)] +#[case::environment_overrides_project( + &[Layer::System, Layer::User, Layer::Project, Layer::Environment], + &["netsuke"], + &[], + LadderExpectation { + file: "Envfile", + emoji: EmojiPolicy::Never, + jobs: Some(16), + locale: Some("it-IT"), + color: ColourPolicy::Never, + } +)] +#[case::cli_overrides_everything( + &[Layer::System, Layer::User, Layer::Project, Layer::Environment], + &[ + "netsuke", + "--file", + "CliFile", + "--emoji", + "always", + "--jobs", + "1", + "--locale", + "ja-JP", + "--color", + "always", + ], + &[], + LadderExpectation { + file: "CliFile", + emoji: EmojiPolicy::Always, + jobs: Some(1), + locale: Some("ja-JP"), + color: ColourPolicy::Always, + } +)] +fn config_ladder_seeds_every_layer_and_winner_follows_precedence( + #[case] enabled_layers: &[Layer], + #[case] cli_args: &[&str], + #[case] extra_env: &[(OsString, OsString)], + #[case] expected: LadderExpectation, +) -> Result<()> { + let merged = run_ladder_scenario(enabled_layers, cli_args, extra_env)?; + assert_ladder(&merged, expected) +} + +/// Merge a scenario with a staged (enabled) set of layers and assert the +/// winner follows the ladder for every rung that participates. +#[rstest] +#[case::only_system(&[Layer::System], "Systemfile")] +#[case::only_user(&[Layer::User], "Userfile")] +#[case::only_project(&[Layer::Project], "Projectfile")] +#[case::only_environment(&[Layer::Environment], "Envfile")] +fn each_rung_wins_over_all_lower_rungs_when_alone( + #[case] enabled_layers: &[Layer], + #[case] expected_file: &str, +) -> Result<()> { + let merged = run_ladder_scenario(enabled_layers, &["netsuke"], &[])?; + ensure!( + merged.file.as_path() == Path::new(expected_file), + "the only enabled rung should win over all lower rungs, got {:?}", + merged.file + ); + Ok(()) +} + +/// The removed `NETSUKE_CONFIG_PATH` alias is not a selector: setting it alone +/// must not select a configuration file, and automatic discovery still runs. +#[rstest] +fn netsuke_config_path_alias_is_not_a_selector_and_discovery_continues() -> Result<()> { + let project = tempdir().context("create project directory")?; + let home = tempdir().context("create home directory")?; + let system = tempdir().context("create system directory")?; + + // Seed a system-scope file and point the legacy alias at a real, existing + // file carrying values distinct from the system scope. A legacy alias that + // were ever (incorrectly) treated as a selector would read this file and + // override the discovered system scope. + let mut environment = + ladder_environment(project.path(), home.path(), system.path(), &[Layer::System])?; + let legacy = tempdir().context("create legacy config directory")?; + test_fs::write( + legacy.path().join("legacy-config.toml"), + r#" +file = "Legacyfile" +emoji = "never" +jobs = 1 +locale = "en-US" +color = "never" +"#, + ) + .context("write legacy config")?; + environment.push(( + OsString::from("NETSUKE_CONFIG_PATH"), + legacy.path().join("legacy-config.toml").into_os_string(), + )); + let merged = merge_in_child(&["netsuke"], project.path(), &environment)?; + + // Discovery still finds the system file; the legacy alias is ignored. + ensure!( + merged.file.as_path() == Path::new("Systemfile"), + "legacy NETSUKE_CONFIG_PATH must not select a config file, got {:?}", + merged.file + ); + ensure!( + merged.jobs == Some(9), + "automatic discovery should continue when the legacy alias is set" + ); + Ok(()) +} + +/// The profile rung is deferred to roadmap item 5.3.1 and does not exist yet. +/// This marker documents where it will sit and must never assert unimplemented +/// behaviour; it only pins the currently-implemented ladder around the gap. +#[rstest] +fn profile_rung_is_deferred_and_does_not_affect_current_ladder() -> Result<()> { + // Today the implemented rungs are CLI > env > project > discovered > + // defaults. When `--profile` lands (5.3.1) it sits between project and + // environment; update this test then. For now the highest stable rung is + // the CLI, so a full layered run still resolves to the CLI values. + let project = tempdir().context("create project directory")?; + let home = tempdir().context("create home directory")?; + let system = tempdir().context("create system directory")?; + let environment = ladder_environment( + project.path(), + home.path(), + system.path(), + &[ + Layer::System, + Layer::User, + Layer::Project, + Layer::Environment, + ], + )?; + let merged = merge_in_child( + &["netsuke", "--jobs", "2", "--emoji", "always"], + project.path(), + &environment, + )?; + ensure!( + merged.jobs == Some(2), + "CLI jobs should win over every config and env layer while profile is deferred" + ); + ensure!( + merged.emoji == EmojiPolicy::Always, + "CLI emoji should win over every config and env layer while profile is deferred" + ); + ensure!( + merged.file.as_path() == Path::new("Envfile"), + "environment manifest path should still merge through when the CLI does not set it" + ); + Ok(()) +} diff --git a/tests/cli_tests/display_policy_domain.rs b/tests/cli_tests/display_policy_domain.rs new file mode 100644 index 000000000..6ca91969b --- /dev/null +++ b/tests/cli_tests/display_policy_domain.rs @@ -0,0 +1,397 @@ +//! Exhaustive display-policy resolution coverage across the consolidated +//! enum domain. +//! +//! Verifies the consolidated display-policy resolution in `src/theme.rs` and +//! `src/output_prefs.rs`, honouring the established precedence (explicit theme +//! preference, then emoji policy, then `NO_COLOR`, then output mode) over the +//! full `EmojiPolicy`, `ColourPolicy`, `ProgressPolicy`, `AccessibilityPolicy`, +//! `json` and `NO_COLOR` domain. +//! Coverage: a deterministic Cartesian-product sweep against a handwritten +//! truth model, plus a proptest (with `TERM`/output-mode and JSON states). +use anyhow::{Result, ensure}; +use itertools::iproduct; +use netsuke::cli::Cli; +use netsuke::cli::config::{AccessibilityPolicy, ColourPolicy, EmojiPolicy, ProgressPolicy}; +use netsuke::output_mode::{OutputMode, resolve_with}; +use netsuke::output_prefs; +use netsuke::theme::{ThemeContext, ThemePreference, resolve_theme}; +use proptest::prelude::*; +use rstest::rstest; + +/// An injected environment lookup whose `NO_COLOR`/`TERM` presence is fixed. +fn fake_env(no_color: bool, term_dumb: bool) -> impl Fn(&str) -> Option { + move |key| match key { + "NO_COLOR" if no_color => Some(String::from("1")), + "TERM" if term_dumb => Some(String::from("dumb")), + _ => None, + } +} + +/// One point in the display-policy domain plus the ambient environment +/// signals that influence the consolidated output decisions. +#[derive(Debug, Clone, Copy)] +struct DomainCase { + emoji: EmojiPolicy, + color: ColourPolicy, + progress: ProgressPolicy, + accessibility: AccessibilityPolicy, + no_color: bool, + term_dumb: bool, + json: bool, +} + +/// An emoji-policy strategy sampling directly from the enum domain. +fn emoji_strategy() -> impl Strategy { + prop::sample::select(&[EmojiPolicy::Auto, EmojiPolicy::Always, EmojiPolicy::Never]) +} + +/// A colour-policy strategy sampling directly from the enum domain. +fn color_strategy() -> impl Strategy { + prop::sample::select(&[ + ColourPolicy::Auto, + ColourPolicy::Always, + ColourPolicy::Never, + ]) +} + +/// A progress-policy strategy sampling directly from the enum domain. +fn progress_strategy() -> impl Strategy { + prop::sample::select(&[ + ProgressPolicy::Auto, + ProgressPolicy::Always, + ProgressPolicy::Never, + ]) +} + +/// An accessibility-policy strategy sampling directly from the enum domain. +fn accessibility_strategy() -> impl Strategy { + prop::sample::select(&[ + AccessibilityPolicy::Auto, + AccessibilityPolicy::On, + AccessibilityPolicy::Off, + ]) +} + +/// One `DomainCase` per combination of the finite policy domain (648 cases): +/// the flat Cartesian product via ``iproduct!``. +fn all_domain_cases() -> Vec { + let emojis = [EmojiPolicy::Auto, EmojiPolicy::Always, EmojiPolicy::Never]; + let colours = [ + ColourPolicy::Auto, + ColourPolicy::Always, + ColourPolicy::Never, + ]; + let progresses = [ + ProgressPolicy::Auto, + ProgressPolicy::Always, + ProgressPolicy::Never, + ]; + let accessibilities = [ + AccessibilityPolicy::Auto, + AccessibilityPolicy::On, + AccessibilityPolicy::Off, + ]; + let flags = [false, true]; + iproduct!( + emojis, + colours, + progresses, + accessibilities, + flags, + flags, + flags + ) + .map( + |(emoji, color, progress, accessibility, no_color, term_dumb, json)| DomainCase { + emoji, + color, + progress, + accessibility, + no_color, + term_dumb, + json, + }, + ) + .collect() +} + +/// Expected theme preference derived from the emoji policy. +const fn expected_theme_preference(emoji: EmojiPolicy) -> Option { + match emoji { + EmojiPolicy::Auto => None, + EmojiPolicy::Always => Some(ThemePreference::Unicode), + EmojiPolicy::Never => Some(ThemePreference::Ascii), + } +} + +/// Expected accessible-output override derived from the accessibility policy. +const fn expected_accessibility_override(accessibility: AccessibilityPolicy) -> Option { + match accessibility { + AccessibilityPolicy::Auto => None, + AccessibilityPolicy::On => Some(true), + AccessibilityPolicy::Off => Some(false), + } +} + +/// Expected progress decision derived from the progress policy. +const fn expected_progress_enabled(progress: ProgressPolicy) -> bool { + !matches!(progress, ProgressPolicy::Never) +} + +/// Whether `NO_COLOR` is active: never for `Always`, always for `Never`, +/// otherwise deferred to the ambient flag captured in the case. +const fn expected_no_color_active(case: DomainCase) -> bool { + match case.color { + ColourPolicy::Always => false, + ColourPolicy::Never => true, + ColourPolicy::Auto => case.no_color, + } +} + +/// Expected output mode: an explicit accessibility override wins; otherwise +/// `NO_COLOR` (when active) or `TERM=dumb` forces Accessible, else Standard. +const fn expected_output_mode(case: DomainCase) -> OutputMode { + if let Some(forced) = expected_accessibility_override(case.accessibility) { + return if forced { + OutputMode::Accessible + } else { + OutputMode::Standard + }; + } + if expected_no_color_active(case) || case.term_dumb { + OutputMode::Accessible + } else { + OutputMode::Standard + } +} + +/// Expected emoji allowance decided by `theme::should_use_unicode`: an explicit +/// theme preference wins; otherwise `NO_COLOR`-active forces ASCII, and +/// otherwise Accessible mode uses ASCII while Standard uses Unicode. +const fn expected_emoji_allowed(case: DomainCase, mode: OutputMode) -> bool { + match case.emoji { + EmojiPolicy::Always => true, + EmojiPolicy::Never => false, + EmojiPolicy::Auto => !expected_no_color_active(case) && !mode.is_accessible(), + } +} + +/// The consolidated expected decisions for one display-policy tuple. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct ExpectedDisplay { + theme_preference: Option, + accessibility_override: Option, + progress_enabled: bool, + output_mode: OutputMode, + emoji_allowed: bool, +} + +const fn expected_display(case: DomainCase) -> ExpectedDisplay { + let theme_preference = expected_theme_preference(case.emoji); + let accessibility_override = expected_accessibility_override(case.accessibility); + let progress_enabled = expected_progress_enabled(case.progress); + let output_mode = expected_output_mode(case); + let emoji_allowed = expected_emoji_allowed(case, output_mode); + ExpectedDisplay { + theme_preference, + accessibility_override, + progress_enabled, + output_mode, + emoji_allowed, + } +} + +/// Build a `Cli` carrying the policy tuple and compare the theme, accessibility, +/// and progress projections against the truth model. +fn assert_consolidated(case: DomainCase) -> Result<()> { + let expected = expected_display(case); + let cli = Cli { + emoji: case.emoji, + color: case.color, + progress: case.progress, + accessibility: case.accessibility, + json: case.json, + ..Cli::default() + }; + ensure!( + cli.json == case.json, + "json policy mismatch for json={}: got {}, expected {}", + case.json, + cli.json, + case.json + ); + ensure!( + cli.theme_preference() == expected.theme_preference, + "theme preference mismatch for emoji={:?}: got {:?}, expected {:?}", + case.emoji, + cli.theme_preference(), + expected.theme_preference + ); + ensure!( + cli.accessibility_override() == expected.accessibility_override, + "accessibility override mismatch for accessibility={:?}: got {:?}, expected {:?}", + case.accessibility, + cli.accessibility_override(), + expected.accessibility_override + ); + ensure!( + cli.progress_enabled() == expected.progress_enabled, + "progress mismatch for progress={:?}: got {}, expected {}", + case.progress, + cli.progress_enabled(), + expected.progress_enabled + ); + assert_env_resolution(case, &cli, expected) +} + +/// Compare the environment-sensitive output-mode, theme-emoji, and `OutputPrefs` +/// decisions against the truth model. Split out of `assert_consolidated` to +/// keep each helper under the file's line ceiling. +fn assert_env_resolution(case: DomainCase, cli: &Cli, expected: ExpectedDisplay) -> Result<()> { + let output_mode = resolve_with( + cli.accessibility_override(), + Some(cli.color), + fake_env(case.no_color, case.term_dumb), + ); + ensure!( + output_mode == expected.output_mode, + "output mode mismatch for color={:?} accessibility={:?} no_color={} term_dumb={}: got {output_mode:?}, expected {:?}", + case.color, + case.accessibility, + case.no_color, + case.term_dumb, + expected.output_mode + ); + + let resolved = resolve_theme( + cli.theme_preference(), + ThemeContext::new(None, Some(cli.color), output_mode), + fake_env(case.no_color, false), + ); + ensure!( + resolved.tokens.emoji_allowed == expected.emoji_allowed, + "emoji allowance mismatch for emoji={:?} color={:?} mode={output_mode:?} no_color={}: got {}, expected {}", + case.emoji, + case.color, + case.no_color, + resolved.tokens.emoji_allowed, + expected.emoji_allowed + ); + + // OutputPrefs, the theme-backed facade, mirrors the theme emoji decision. + let prefs = output_prefs::resolve_from_theme_with( + cli.theme_preference(), + ThemeContext::new(None, Some(cli.color), output_mode), + fake_env(case.no_color, false), + ); + ensure!( + prefs.emoji_allowed() == expected.emoji_allowed, + "OutputPrefs emoji mismatch for emoji={:?} color={:?}: got {}, expected {}", + case.emoji, + case.color, + prefs.emoji_allowed(), + expected.emoji_allowed + ); + Ok(()) +} + +proptest! { + /// The production display-policy pipeline agrees with the handwritten + /// truth model over the generated full domain and arbitrary environment + /// signals. + #[test] + fn consolidated_display_policies_resolve_correctly( + emoji in emoji_strategy(), + color in color_strategy(), + progress in progress_strategy(), + accessibility in accessibility_strategy(), + no_color in any::(), + term_dumb in any::(), + json in any::(), + ) { + let case = DomainCase { + emoji, + color, + progress, + accessibility, + no_color, + term_dumb, + json, + }; + // `assert_consolidated` uses `ensure!` and returns `Result`; divert the + // failure to a prop-level assertion so shrinking reports the tuple. + let result = assert_consolidated(case); + prop_assert!(result.is_ok(), "consolidated policy failure: {result:?}"); + } +} + +/// Exhaustive single-field projection coverage (rstest). +#[rstest] +#[case(EmojiPolicy::Auto, None)] +#[case(EmojiPolicy::Always, Some(ThemePreference::Unicode))] +#[case(EmojiPolicy::Never, Some(ThemePreference::Ascii))] +fn emoji_policy_maps_to_theme_preference( + #[case] emoji: EmojiPolicy, + #[case] expected: Option, +) -> Result<()> { + let cli = Cli { + emoji, + ..Cli::default() + }; + ensure!( + cli.theme_preference() == expected, + "emoji {emoji:?} should map to theme {expected:?}, got {:?}", + cli.theme_preference() + ); + Ok(()) +} + +#[rstest] +#[case(AccessibilityPolicy::Auto, None)] +#[case(AccessibilityPolicy::On, Some(true))] +#[case(AccessibilityPolicy::Off, Some(false))] +fn accessibility_policy_maps_to_override( + #[case] accessibility: AccessibilityPolicy, + #[case] expected: Option, +) -> Result<()> { + let cli = Cli { + accessibility, + ..Cli::default() + }; + ensure!( + cli.accessibility_override() == expected, + "accessibility {accessibility:?} should map to override {expected:?}, got {:?}", + cli.accessibility_override() + ); + Ok(()) +} + +#[rstest] +#[case(ProgressPolicy::Auto, true)] +#[case(ProgressPolicy::Always, true)] +#[case(ProgressPolicy::Never, false)] +fn progress_policy_enables_progress( + #[case] progress: ProgressPolicy, + #[case] expected: bool, +) -> Result<()> { + let cli = Cli { + progress, + ..Cli::default() + }; + ensure!( + cli.progress_enabled() == expected, + "progress {progress:?} should enable {expected}, got {}", + cli.progress_enabled() + ); + Ok(()) +} + +/// Exhaustive sweep of the 3 x 3 x 3 x 3 policy domain across the `NO_COLOR`, +/// `json`, and `TERM=dumb` toggles, comparing the pipeline to the truth model. +#[rstest] +fn exhaustive_domain_sweep_matches_truth_model() -> Result<()> { + for case in all_domain_cases() { + assert_consolidated(case)?; + } + Ok(()) +} diff --git a/tests/cli_tests/merge_probe.rs b/tests/cli_tests/merge_probe.rs index 4ac4c28c8..98b40c714 100644 --- a/tests/cli_tests/merge_probe.rs +++ b/tests/cli_tests/merge_probe.rs @@ -72,6 +72,55 @@ pub(super) fn isolated_environment( Ok((xdg_config_dirs, environment)) } +/// Build the environment for a configuration probe whose system scope is +/// redirected to `system_dirs`. +/// +/// Unlike [`isolated_environment`], this variant does not return a sandbox +/// `TempDir` for `XDG_CONFIG_DIRS`: it points the system-configuration +/// directory at the caller-supplied `system_dirs` path so a config file +/// written there is discovered as the system-scope layer. The caller retains +/// ownership of `system_dirs` (typically a `TempDir`) and must keep it alive +/// until the child process has finished, matching the lifetime contract of +/// the sandbox directory returned by [`isolated_environment`]. +/// +/// The environment selector set stays closed at the documented variables; +/// no selector other than `NETSUKE_CONFIG` participates in discovery. +/// +/// # Examples +/// +/// ```rust,ignore +/// let home = tempfile::tempdir()?; +/// let system = tempfile::tempdir()?; +/// std::fs::create_dir_all(system.path().join("netsuke"))?; +/// std::fs::write(system.path().join("netsuke/config.toml"), "emoji = \"always\"\n")?; +/// let environment = environment_with_system_scope(home.path(), system.path(), &[]); +/// let merged = merge_in_child(&["netsuke"], home.path(), &environment)?; +/// assert_eq!(merged.emoji, netsuke::cli::EmojiPolicy::Always); +/// # Ok::<(), anyhow::Error>(()) +/// ``` +/// The injected XDG environment is only meaningful on Unix; Windows discovers +/// user and system configuration through `APPDATA`/`LOCALAPPDATA` instead. +#[cfg(unix)] +pub(super) fn environment_with_system_scope( + home: &Path, + system_dirs: &Path, + overrides: &[(OsString, OsString)], +) -> Vec<(OsString, OsString)> { + let mut environment = vec![ + (OsString::from("HOME"), home.as_os_str().to_owned()), + ( + OsString::from("XDG_CONFIG_HOME"), + home.join(".config").into_os_string(), + ), + ( + OsString::from("XDG_CONFIG_DIRS"), + system_dirs.as_os_str().to_owned(), + ), + ]; + environment.extend_from_slice(overrides); + environment +} + /// Merge configuration in an isolated process with the supplied environment. pub(super) fn merge_in_child( args: &[&str], diff --git a/tests/cli_tests/merge_targets_proptests.rs b/tests/cli_tests/merge_targets_proptests.rs new file mode 100644 index 000000000..ed9230269 --- /dev/null +++ b/tests/cli_tests/merge_targets_proptests.rs @@ -0,0 +1,208 @@ +//! Property tests for list-append and scalar merge ordering of the build-target +//! and display-policy fields. +//! +//! The example merge tests pin one fixed composition per story. These +//! properties hold across generated inputs for the fields the ladder tests do +//! not enumerate exhaustively: +//! +//! - `default_targets` appends in discovery order (file → environment → CLI), +//! mirroring the ladder tests for scalar fields. +//! - Scalar merge ordering (defaults → file → environment → CLI) holds for +//! generated locale and policy-enum values. +//! +//! The explicit-CLI `build ` replacement asymmetry lives in the +//! command layer of `netsuke::cli::merge` and is asserted end to end in +//! [`super::merge`] where a parsed command is available; a pure +//! `MergeComposer` composition cannot observe it. +//! +//! No `#[derive(Arbitrary)]` is used anywhere; every strategy is handwritten +//! and stays free of any second environment selector. + +use netsuke::cli::CliConfig; +use ortho_config::{MergeComposer, sanitize_value}; +use proptest::prelude::*; +use serde_json::{Value, json}; + +/// Values a generated merge layer may carry for the fields under test. +/// +/// A field left `None` is omitted from the layer, matching a layer the +/// discovery pass simply did not produce. +struct LayerValues { + default_targets: Option>, + jobs: Option, + locale: Option, + emoji: Option<&'static str>, + color: Option<&'static str>, +} + +impl LayerValues { + /// An empty layer carrying no generated values. + const fn empty() -> Self { + Self { + default_targets: None, + jobs: None, + locale: None, + emoji: None, + color: None, + } + } +} + +/// A generated target name: lower-case letters with an optional numeric +/// suffix. No second environment selector participates anywhere in these +/// strategies. +fn target_strategy() -> impl Strategy { + "[a-z]{1,12}(-[a-z0-9]{1,4})?" +} + +/// A locale tag in the `xx-YY` shape Netsuke accepts. +fn locale_strategy() -> impl Strategy { + prop::sample::select(&["en-US", "es-ES", "de-DE", "fr-FR", "it-IT", "ja-JP"]) + .prop_map(str::to_owned) +} + +/// A policy-enum string accepted by the display-policy parsers. +fn policy_strategy() -> impl Strategy { + prop::sample::select(&["auto", "always", "never"]) +} + +/// Build the JSON layer for `values`. +fn build_target_layer(values: &LayerValues) -> Value { + let mut layer = serde_json::Map::new(); + if let Some(targets) = &values.default_targets { + layer.insert("default_targets".to_owned(), json!(targets)); + } + if let Some(jobs) = values.jobs { + layer.insert("jobs".to_owned(), json!(jobs)); + } + if let Some(locale) = &values.locale { + layer.insert("locale".to_owned(), json!(locale)); + } + if let Some(emoji) = values.emoji { + layer.insert("emoji".to_owned(), json!(emoji)); + } + if let Some(color) = values.color { + layer.insert("color".to_owned(), json!(color)); + } + Value::Object(layer) +} + +/// Merge generated layers through `MergeComposer`. +/// +/// # Errors +/// +/// Propagates an `OrthoError` when the generated layers do not merge cleanly, +/// which a well-formed strategy should never trigger. +fn merge_generated( + file_layer: Value, + env_layer: Value, + cli_layer: Value, + defaults: Value, +) -> anyhow::Result { + let mut composer = MergeComposer::new(); + composer.push_defaults(defaults); + composer.push_file(file_layer, None); + composer.push_environment(env_layer); + composer.push_cli(cli_layer); + Ok(CliConfig::merge_from_layers(composer.layers())?) +} + +proptest! { + /// The `default_targets` alias appends through the whole ladder: the merged + /// vector is the concatenation of the file, environment, and CLI layers in + /// that order, regardless of how long or empty each generated layer is. + #[test] + fn default_targets_append_in_layer_order( + file_targets in prop::collection::vec(target_strategy(), 0..5), + env_targets in prop::collection::vec(target_strategy(), 0..5), + cli_targets in prop::collection::vec(target_strategy(), 0..5), + ) { + let defaults = sanitize_value(&CliConfig::default()).expect("sanitizable defaults"); + let file_layer = build_target_layer(&LayerValues { + default_targets: Some(file_targets.clone()), + ..LayerValues::empty() + }); + let env_layer = build_target_layer(&LayerValues { + default_targets: Some(env_targets.clone()), + ..LayerValues::empty() + }); + let cli_layer = build_target_layer(&LayerValues { + default_targets: Some(cli_targets.clone()), + ..LayerValues::empty() + }); + let merged = merge_generated(file_layer, env_layer, cli_layer, defaults) + .expect("generated layers must merge cleanly"); + + // The append strategy concatenates default_targets across file, env, + // and CLI layers in discovery order (defaults contributes nothing). + let expected_appended = file_targets + .iter() + .chain(env_targets.iter()) + .chain(cli_targets.iter()) + .cloned() + .collect::>(); + prop_assert_eq!(&merged.default_targets, &expected_appended); + // The compatibility alias never leaks into the explicit build targets: + // with no explicit build command, the merged cmds.build.targets stays + // empty. + prop_assert!(merged.cmds.build.targets.is_empty()); + } + + /// Scalar merge ordering holds for generated locale and policy-enum values: + /// the highest populated layer wins, and each field is resolved + /// independently. + #[test] + fn scalar_merge_ordering_holds_for_locale_and_policies( + file_locale in prop::option::of(locale_strategy()), + env_locale in prop::option::of(locale_strategy()), + cli_locale in prop::option::of(locale_strategy()), + file_emoji in prop::option::of(policy_strategy()), + env_emoji in prop::option::of(policy_strategy()), + cli_emoji in prop::option::of(policy_strategy()), + file_color in prop::option::of(policy_strategy()), + env_color in prop::option::of(policy_strategy()), + cli_color in prop::option::of(policy_strategy()), + file_jobs in prop::option::of(1u64..=64), + env_jobs in prop::option::of(1u64..=64), + cli_jobs in prop::option::of(1u64..=64), + ) { + let defaults = sanitize_value(&CliConfig::default()).expect("sanitizable defaults"); + let file_layer = build_target_layer(&LayerValues { + default_targets: None, + jobs: file_jobs, + locale: file_locale.clone(), + emoji: file_emoji, + color: file_color, + }); + let env_layer = build_target_layer(&LayerValues { + default_targets: None, + jobs: env_jobs, + locale: env_locale.clone(), + emoji: env_emoji, + color: env_color, + }); + let cli_layer = build_target_layer(&LayerValues { + default_targets: None, + jobs: cli_jobs, + locale: cli_locale.clone(), + emoji: cli_emoji, + color: cli_color, + }); + let merged = merge_generated(file_layer, env_layer, cli_layer, defaults) + .expect("generated layers must merge cleanly"); + + let expected_locale = cli_locale.or(env_locale).or(file_locale); + let expected_emoji = cli_emoji.or(env_emoji).or(file_emoji).unwrap_or("auto"); + let expected_color = cli_color.or(env_color).or(file_color).unwrap_or("auto"); + let expected_jobs = cli_jobs.or(env_jobs).or(file_jobs); + let emoji_text = merged.emoji.to_string(); + let color_text = merged.color.to_string(); + prop_assert_eq!(merged.locale.as_deref(), expected_locale.as_deref()); + prop_assert_eq!(emoji_text.as_str(), expected_emoji); + prop_assert_eq!(color_text.as_str(), expected_color); + prop_assert_eq!( + merged.jobs, + expected_jobs.map(|jobs| usize::try_from(jobs).expect("generated jobs fit usize")) + ); + } +} diff --git a/tests/cli_tests/mod.rs b/tests/cli_tests/mod.rs index 4c0910d76..d07d0e198 100644 --- a/tests/cli_tests/mod.rs +++ b/tests/cli_tests/mod.rs @@ -3,12 +3,16 @@ //! This module exercises the command-line interface defined in `netsuke::cli`. mod config_discovery; +#[cfg(unix)] +mod config_precedence_ladder; mod config_selection; +mod display_policy_domain; mod helpers; mod locale; mod merge; mod merge_diag; mod merge_precedence_proptests; mod merge_probe; +mod merge_targets_proptests; mod parsing; mod policy;