diff --git a/.changeset/changesets/confusingly-privileged-topminnow.md b/.changeset/changesets/confusingly-privileged-topminnow.md new file mode 100644 index 0000000..9094c66 --- /dev/null +++ b/.changeset/changesets/confusingly-privileged-topminnow.md @@ -0,0 +1,5 @@ +--- +category: fixed +cargo-changeset: none +--- +Fixed env var testing methodology diff --git a/Cargo.lock b/Cargo.lock index f4564ea..3209354 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -159,6 +159,7 @@ dependencies = [ "semver", "serde_json", "strum", + "temp-env", "tempfile", "thiserror", ] @@ -1065,6 +1066,15 @@ version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6373607a59f0be73a39b6fe456b8192fcc3585f602af20751600e974dd455e77" +[[package]] +name = "lock_api" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "224399e74b87b5f3557511d98dff8b14089b3dadafcab6bb93eab67d3aace965" +dependencies = [ + "scopeguard", +] + [[package]] name = "log" version = "0.4.29" @@ -1132,6 +1142,29 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" +[[package]] +name = "parking_lot" +version = "0.12.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93857453250e3077bd71ff98b6a65ea6621a19bb0f559a85248955ac12c45a1a" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.9.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2621685985a2ebf1c516881c026032ac7deafcda1a2c9b7850dc81e3dfcb64c1" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall", + "smallvec", + "windows-link", +] + [[package]] name = "percent-encoding" version = "2.3.2" @@ -1319,6 +1352,15 @@ version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "63b8176103e19a2643978565ca18b50549f6101881c443590420e4dc998a3c69" +[[package]] +name = "redox_syscall" +version = "0.5.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed2bf2547551a7053d6fdfafda3f938979645c44812fbfcda098faae3f1a362d" +dependencies = [ + "bitflags 2.11.0", +] + [[package]] name = "ref-cast" version = "1.0.25" @@ -1444,6 +1486,12 @@ dependencies = [ "serde_json", ] +[[package]] +name = "scopeguard" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" + [[package]] name = "semver" version = "1.0.27" @@ -1632,6 +1680,15 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "temp-env" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96374855068f47402c3121c6eed88d29cb1de8f3ab27090e273e420bdabcf050" +dependencies = [ + "parking_lot", +] + [[package]] name = "tempfile" version = "3.27.0" diff --git a/crates/cargo-changeset/Cargo.toml b/crates/cargo-changeset/Cargo.toml index 1811df0..51c1469 100644 --- a/crates/cargo-changeset/Cargo.toml +++ b/crates/cargo-changeset/Cargo.toml @@ -40,3 +40,4 @@ indexmap = { workspace = true } semver = { workspace = true } serde_json = { workspace = true } indoc = "2.0.7" +temp-env = "0.3.6" diff --git a/crates/cargo-changeset/src/environment.rs b/crates/cargo-changeset/src/environment.rs index 136c7f9..e1335c1 100644 --- a/crates/cargo-changeset/src/environment.rs +++ b/crates/cargo-changeset/src/environment.rs @@ -54,109 +54,78 @@ fn detect_ci_env_var() -> Option { #[cfg(test)] mod tests { - use std::sync::Mutex; - use super::*; - static ENV_MUTEX: Mutex<()> = Mutex::new(()); - - fn with_env(vars: &[(&str, &str)], clear: &[&str], f: F) -> R - where - F: FnOnce() -> R, - { - let _guard = ENV_MUTEX.lock().expect("mutex poisoned"); - - let mut old_values: Vec<(&str, Option)> = Vec::new(); - - for var in clear { - old_values.push((var, std::env::var(var).ok())); - // SAFETY: Test code runs sequentially with ENV_MUTEX held. - unsafe { std::env::remove_var(var) }; - } - - for (key, value) in vars { - old_values.push((key, std::env::var(key).ok())); - // SAFETY: Test code runs sequentially with ENV_MUTEX held. - unsafe { std::env::set_var(key, value) }; - } - - let result = f(); - - for (key, old_value) in old_values { - match old_value { - // SAFETY: Test code runs sequentially with ENV_MUTEX held. - Some(v) => unsafe { std::env::set_var(key, v) }, - // SAFETY: Test code runs sequentially with ENV_MUTEX held. - None => unsafe { std::env::remove_var(key) }, - } - } - - result + fn env_vars( + extra: &[(&'static str, Option<&'static str>)], + ) -> Vec<(&'static str, Option<&'static str>)> { + let mut vars: Vec<(&str, Option<&str>)> = [ + "CI", + "GITHUB_ACTIONS", + "GITLAB_CI", + "CIRCLECI", + "TRAVIS", + "JENKINS_URL", + "BUILDKITE", + "TF_BUILD", + "CARGO_CHANGESET_NO_TTY", + "CARGO_CHANGESET_FORCE_TTY", + ] + .map(|var| (var, None)) + .to_vec(); + vars.extend_from_slice(extra); + vars } - const ALL_CI_VARS: &[&str] = &[ - "CI", - "GITHUB_ACTIONS", - "GITLAB_CI", - "CIRCLECI", - "TRAVIS", - "JENKINS_URL", - "BUILDKITE", - "TF_BUILD", - "CARGO_CHANGESET_NO_TTY", - "CARGO_CHANGESET_FORCE_TTY", - ]; - mod detect_ci_env_var { use super::*; #[test] fn returns_none_when_no_ci_vars_set() { - with_env(&[], ALL_CI_VARS, || { + temp_env::with_vars(env_vars(&[]), || { assert!(detect_ci_env_var().is_none()); }); } #[test] fn detects_ci() { - with_env(&[("CI", "true")], ALL_CI_VARS, || { + temp_env::with_vars(env_vars(&[("CI", Some("true"))]), || { assert_eq!(detect_ci_env_var(), Some("CI".to_string())); }); } #[test] fn detects_github_actions() { - with_env(&[("GITHUB_ACTIONS", "true")], ALL_CI_VARS, || { + temp_env::with_vars(env_vars(&[("GITHUB_ACTIONS", Some("true"))]), || { assert_eq!(detect_ci_env_var(), Some("GITHUB_ACTIONS".to_string())); }); } #[test] fn detects_gitlab_ci() { - with_env(&[("GITLAB_CI", "true")], ALL_CI_VARS, || { + temp_env::with_vars(env_vars(&[("GITLAB_CI", Some("true"))]), || { assert_eq!(detect_ci_env_var(), Some("GITLAB_CI".to_string())); }); } #[test] fn detects_circleci() { - with_env(&[("CIRCLECI", "true")], ALL_CI_VARS, || { + temp_env::with_vars(env_vars(&[("CIRCLECI", Some("true"))]), || { assert_eq!(detect_ci_env_var(), Some("CIRCLECI".to_string())); }); } #[test] fn detects_travis() { - with_env(&[("TRAVIS", "true")], ALL_CI_VARS, || { + temp_env::with_vars(env_vars(&[("TRAVIS", Some("true"))]), || { assert_eq!(detect_ci_env_var(), Some("TRAVIS".to_string())); }); } #[test] fn detects_jenkins() { - with_env( - &[("JENKINS_URL", "http://jenkins.local")], - ALL_CI_VARS, + temp_env::with_vars( + env_vars(&[("JENKINS_URL", Some("http://jenkins.local"))]), || { assert_eq!(detect_ci_env_var(), Some("JENKINS_URL".to_string())); }, @@ -165,14 +134,14 @@ mod tests { #[test] fn detects_buildkite() { - with_env(&[("BUILDKITE", "true")], ALL_CI_VARS, || { + temp_env::with_vars(env_vars(&[("BUILDKITE", Some("true"))]), || { assert_eq!(detect_ci_env_var(), Some("BUILDKITE".to_string())); }); } #[test] fn detects_azure_devops() { - with_env(&[("TF_BUILD", "True")], ALL_CI_VARS, || { + temp_env::with_vars(env_vars(&[("TF_BUILD", Some("True"))]), || { assert_eq!(detect_ci_env_var(), Some("TF_BUILD".to_string())); }); } @@ -183,13 +152,12 @@ mod tests { #[test] fn no_tty_takes_highest_priority() { - with_env( - &[ - ("CARGO_CHANGESET_NO_TTY", "1"), - ("CARGO_CHANGESET_FORCE_TTY", "1"), - ("CI", "true"), - ], - ALL_CI_VARS, + temp_env::with_vars( + env_vars(&[ + ("CARGO_CHANGESET_NO_TTY", Some("1")), + ("CARGO_CHANGESET_FORCE_TTY", Some("1")), + ("CI", Some("true")), + ]), || { assert_eq!( non_interactive_reason(), @@ -201,9 +169,11 @@ mod tests { #[test] fn force_tty_takes_priority_over_ci_detection() { - with_env( - &[("CI", "true"), ("CARGO_CHANGESET_FORCE_TTY", "1")], - ALL_CI_VARS, + temp_env::with_vars( + env_vars(&[ + ("CI", Some("true")), + ("CARGO_CHANGESET_FORCE_TTY", Some("1")), + ]), || { assert!(non_interactive_reason().is_none()); }, @@ -212,14 +182,17 @@ mod tests { #[test] fn force_tty_allows_interactivity_when_no_ci() { - with_env(&[("CARGO_CHANGESET_FORCE_TTY", "1")], ALL_CI_VARS, || { - assert!(non_interactive_reason().is_none()); - }); + temp_env::with_vars( + env_vars(&[("CARGO_CHANGESET_FORCE_TTY", Some("1"))]), + || { + assert!(non_interactive_reason().is_none()); + }, + ); } #[test] fn explicit_disable_returns_correct_reason() { - with_env(&[("CARGO_CHANGESET_NO_TTY", "1")], ALL_CI_VARS, || { + temp_env::with_vars(env_vars(&[("CARGO_CHANGESET_NO_TTY", Some("1"))]), || { assert_eq!( non_interactive_reason(), Some(NonInteractiveReason::ExplicitDisable) @@ -229,7 +202,7 @@ mod tests { #[test] fn ci_detection_returns_correct_env_var() { - with_env(&[("GITHUB_ACTIONS", "true")], ALL_CI_VARS, || { + temp_env::with_vars(env_vars(&[("GITHUB_ACTIONS", Some("true"))]), || { assert_eq!( non_interactive_reason(), Some(NonInteractiveReason::CiDetected { @@ -245,30 +218,35 @@ mod tests { #[test] fn returns_false_when_no_tty_set() { - with_env(&[("CARGO_CHANGESET_NO_TTY", "1")], ALL_CI_VARS, || { + temp_env::with_vars(env_vars(&[("CARGO_CHANGESET_NO_TTY", Some("1"))]), || { assert!(!is_interactive()); }); } #[test] fn returns_false_when_ci_detected() { - with_env(&[("CI", "true")], ALL_CI_VARS, || { + temp_env::with_vars(env_vars(&[("CI", Some("true"))]), || { assert!(!is_interactive()); }); } #[test] fn returns_true_when_force_tty_and_no_ci() { - with_env(&[("CARGO_CHANGESET_FORCE_TTY", "1")], ALL_CI_VARS, || { - assert!(is_interactive()); - }); + temp_env::with_vars( + env_vars(&[("CARGO_CHANGESET_FORCE_TTY", Some("1"))]), + || { + assert!(is_interactive()); + }, + ); } #[test] fn returns_true_when_force_tty_overrides_ci() { - with_env( - &[("CI", "true"), ("CARGO_CHANGESET_FORCE_TTY", "1")], - ALL_CI_VARS, + temp_env::with_vars( + env_vars(&[ + ("CI", Some("true")), + ("CARGO_CHANGESET_FORCE_TTY", Some("1")), + ]), || { assert!(is_interactive()); },