diff --git a/src/cli/config_merge.rs b/src/cli/config_merge.rs index 37ed776d..669fd0af 100644 --- a/src/cli/config_merge.rs +++ b/src/cli/config_merge.rs @@ -361,6 +361,10 @@ pub fn merge_with_config(cli: &Cli, matches: &ArgMatches) -> OrthoResult { Ok(merged) } +#[cfg(test)] +#[path = "config_path_precedence_tests.rs"] +mod config_path_precedence_tests; + #[cfg(test)] #[path = "config_merge_tests.rs"] mod tests; diff --git a/src/cli/config_merge_tests.rs b/src/cli/config_merge_tests.rs index 6089d485..4797fd3a 100644 --- a/src/cli/config_merge_tests.rs +++ b/src/cli/config_merge_tests.rs @@ -331,65 +331,3 @@ fn env_config_path_returns_path_when_var_set() { let result = env_config_path(|name| env.var_os(name), "__NETSUKE_TEST_VAR"); assert_eq!(result, Some(std::path::PathBuf::from("/tmp/foo.toml"))); } - -// ----------------------------------------------------------------------- -// resolve_config_path -// ----------------------------------------------------------------------- - -#[rstest] -#[case::cli_wins_over_env( - (Some("/env/path.toml"), None), - Some("/cli/path.toml"), - Some("/cli/path.toml"), - "cli.config should take precedence over CONFIG_ENV_VAR" -)] -#[case::env_wins_over_legacy( - (Some("/env/path.toml"), Some("/legacy/path.toml")), - None, - Some("/env/path.toml"), - "CONFIG_ENV_VAR should take precedence over CONFIG_ENV_VAR_LEGACY" -)] -#[case::legacy_used_when_primary_absent( - (None, Some("/legacy/path.toml")), - None, - Some("/legacy/path.toml"), - "CONFIG_ENV_VAR_LEGACY should be used when primary env var absent" -)] -#[case::cli_wins_over_both_env_vars( - (Some("/env/path.toml"), Some("/legacy/path.toml")), - Some("/cli/path.toml"), - Some("/cli/path.toml"), - "cli.config should win over both env vars" -)] -fn resolve_config_path_precedence( - #[case] env_vars: (Option<&'static str>, Option<&'static str>), - #[case] cli_config: Option<&'static str>, - #[case] expected: Option<&'static str>, - #[case] msg: &'static str, -) { - let mut env = TestEnv::default(); - if let Some(value) = env_vars.0 { - env = env.with_var(CONFIG_ENV_VAR, value); - } - if let Some(value) = env_vars.1 { - env = env.with_var(CONFIG_ENV_VAR_LEGACY, value); - } - let cli = Cli { - config: cli_config.map(std::path::PathBuf::from), - ..Cli::default() - }; - assert_eq!( - resolve_config_path(&cli, |name| env.var_os(name)), - expected.map(std::path::PathBuf::from), - "{msg}", - ); -} - -#[test] -fn resolve_config_path_returns_none_when_all_absent() { - let cli = Cli::default(); - assert!( - resolve_config_path(&cli, |name| TestEnv::default().var_os(name)).is_none(), - "should return None when no selector is active" - ); -} diff --git a/src/cli/config_path_precedence_tests.rs b/src/cli/config_path_precedence_tests.rs new file mode 100644 index 00000000..3344aff3 --- /dev/null +++ b/src/cli/config_path_precedence_tests.rs @@ -0,0 +1,122 @@ +//! Tests for explicit config path selector precedence. + +use super::*; +use proptest::prelude::*; +use rstest::rstest; +use std::collections::HashMap; +use std::ffi::OsString; +use std::path::PathBuf; + +#[derive(Default)] +struct TestEnv { + values: HashMap<&'static str, OsString>, +} + +impl TestEnv { + fn with_var(mut self, name: &'static str, value: impl Into) -> Self { + self.values.insert(name, value.into()); + self + } + + fn var_os(&self, name: &str) -> Option { + self.values.get(name).cloned() + } +} + +fn precedence_winner( + cli_config: Option, + env_config: Option, + legacy_config: Option, +) -> Option { + cli_config.or(env_config).or(legacy_config) +} + +fn resolve_config_path_with_selectors( + cli_config: Option, + env_config: Option, + legacy_config: Option, +) -> Option { + let mut env = TestEnv::default(); + if let Some(value) = env_config { + env = env.with_var(CONFIG_ENV_VAR, value.into_os_string()); + } + if let Some(value) = legacy_config { + env = env.with_var(CONFIG_ENV_VAR_LEGACY, value.into_os_string()); + } + let cli = Cli { + config: cli_config, + ..Cli::default() + }; + resolve_config_path(&cli, |name| env.var_os(name)) +} + +/// For selectors S1 (`cli.config`), S2 (`NETSUKE_CONFIG`), and S3 +/// (`NETSUKE_CONFIG_PATH`), the resolved path must be S1 when present, else S2 +/// when present, else S3 when present, else `None`. These cases exhaustively +/// enumerate the 2^3 selector presence states. +#[rstest] +#[case::all_absent(None, None, None, None)] +#[case::legacy_only(None, None, Some("/legacy/path.toml"), Some("/legacy/path.toml"))] +#[case::env_only(None, Some("/env/path.toml"), None, Some("/env/path.toml"))] +#[case::env_wins_over_legacy( + None, + Some("/env/path.toml"), + Some("/legacy/path.toml"), + Some("/env/path.toml") +)] +#[case::cli_only(Some("/cli/path.toml"), None, None, Some("/cli/path.toml"))] +#[case::cli_wins_over_legacy_alone( + Some("/cli/path.toml"), + None, + Some("/legacy/path.toml"), + Some("/cli/path.toml") +)] +#[case::cli_wins_over_env( + Some("/cli/path.toml"), + Some("/env/path.toml"), + None, + Some("/cli/path.toml") +)] +#[case::cli_wins_over_both_env_vars( + Some("/cli/path.toml"), + Some("/env/path.toml"), + Some("/legacy/path.toml"), + Some("/cli/path.toml") +)] +fn resolve_config_path_precedence( + #[case] cli_config: Option<&'static str>, + #[case] env_config: Option<&'static str>, + #[case] legacy_config: Option<&'static str>, + #[case] expected: Option<&'static str>, +) { + assert_eq!( + resolve_config_path_with_selectors( + cli_config.map(PathBuf::from), + env_config.map(PathBuf::from), + legacy_config.map(PathBuf::from), + ), + expected.map(PathBuf::from), + ); +} + +fn path_selector() -> impl Strategy> { + proptest::option::of("[A-Za-z0-9._/-]{1,64}".prop_map(PathBuf::from)) +} + +proptest! { + #[test] + fn resolve_config_path_obeys_precedence_invariant( + cli_config in path_selector(), + env_config in path_selector(), + legacy_config in path_selector(), + ) { + let expected = precedence_winner( + cli_config.clone(), + env_config.clone(), + legacy_config.clone(), + ); + let actual = resolve_config_path_with_selectors(cli_config, env_config, legacy_config); + + prop_assert_eq!(actual, expected); + } +} diff --git a/tests/cli_tests/config_selection.rs b/tests/cli_tests/config_selection.rs index ef4197af..12072c19 100644 --- a/tests/cli_tests/config_selection.rs +++ b/tests/cli_tests/config_selection.rs @@ -260,6 +260,23 @@ fn write_optional_config( .with_cli_config(ConfigFile::new("cli.toml", "theme = \"unicode\"\n")) .with_env_config(ConfigFile::new("env.toml", "theme = \"ascii\"\n")), )] +#[case::config_flag_takes_precedence_over_legacy_env( + ConfigSelectionCase::new( + ThemePreference::Unicode, + "--config should win over NETSUKE_CONFIG_PATH", + ) + .with_cli_config(ConfigFile::new("cli.toml", "theme = \"unicode\"\n")) + .with_legacy_config(ConfigFile::new("legacy.toml", "theme = \"ascii\"\n")), +)] +#[case::config_flag_takes_precedence_over_both_env_vars( + ConfigSelectionCase::new( + ThemePreference::Unicode, + "--config should win over both config environment variables", + ) + .with_cli_config(ConfigFile::new("cli.toml", "theme = \"unicode\"\n")) + .with_env_config(ConfigFile::new("env.toml", "theme = \"ascii\"\n")) + .with_legacy_config(ConfigFile::new("legacy.toml", "theme = \"ascii\"\n")), +)] #[case::config_flag_values_still_overridden_by_cli_preferences( ConfigSelectionCase::new( ThemePreference::Ascii,