Skip to content

Commit f0ecc9b

Browse files
committed
fix(types): stop the user-config tests writing to the real home directory
`test_user_config_save_and_load` called `UserConfig::save()`, which writes `~/.lash/config.toml` on the developer's machine. It restored afterwards with `UserConfig::default().save()`, which was wrong twice over: on the happy path it replaced whatever the user actually had with defaults, and if the test panicked or the process was killed the restore never ran at all, leaving `color_scheme = "Test Theme"` behind. No theme resolves that name, so every later `lash` invocation on the machine failed with `Color scheme 'Test Theme' not found`. A mutation-testing run surfaced this on 2026-08-09 by killing the suite mid-execution hundreds of times, but any interrupted `cargo test` does the same. `load_from(path)` and `save_to(path)` now take the config path explicitly; `load()` and `save()` are thin wrappers that pass the home-directory path. The tests point at a `TempDir` and never resolve `$HOME`. Also covered: saving into a directory that does not exist yet, and rejecting invalid TOML on load. `test_user_config_load_nonexistent` no longer carries its "assumes ~/.lash/config.toml doesn't exist or is valid" caveat, since it now reads a path it created. Audited the rest of the workspace for the same pattern. lash-cli's `Config::user_config_path` resolves a different location (`dirs::config_dir`) and its test only inspects the path. lash-tui's `apply_selected_theme` calls `save()` for real, which is correct, and no test exercises it.
1 parent e52c355 commit f0ecc9b

1 file changed

Lines changed: 66 additions & 27 deletions

File tree

‎crates/lash-types/src/config.rs‎

Lines changed: 66 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,18 @@ impl UserConfig {
332332
///
333333
/// Returns error if config file exists but is invalid
334334
pub fn load() -> Result<Self> {
335-
let config_path = Self::user_config_path()?;
335+
Self::load_from(Self::user_config_path()?)
336+
}
337+
338+
/// Load user configuration from an explicit path
339+
///
340+
/// If the file doesn't exist, returns default configuration.
341+
///
342+
/// # Errors
343+
///
344+
/// Returns error if config file exists but is invalid
345+
pub fn load_from(config_path: impl Into<PathBuf>) -> Result<Self> {
346+
let config_path = config_path.into();
336347

337348
if !config_path.exists() {
338349
return Ok(Self::default());
@@ -366,17 +377,30 @@ impl UserConfig {
366377
///
367378
/// Returns error if config cannot be written
368379
pub fn save(&self) -> Result<()> {
369-
let config_dir = Self::user_config_dir()?;
370-
let config_path = Self::user_config_path()?;
380+
self.save_to(Self::user_config_path()?)
381+
}
382+
383+
/// Save user configuration to an explicit path
384+
///
385+
/// Creates the parent directory if it doesn't exist. Uses atomic writes to
386+
/// prevent corruption.
387+
///
388+
/// # Errors
389+
///
390+
/// Returns error if config cannot be written
391+
pub fn save_to(&self, config_path: impl Into<PathBuf>) -> Result<()> {
392+
let config_path = config_path.into();
371393

372394
// Create directory if it doesn't exist
373-
if !config_dir.exists() {
374-
fs::create_dir_all(&config_dir).map_err(|e| LashError::IO {
375-
code: codes::E_IO_WRITE_ERROR,
376-
message: format!("Failed to create user config directory: {e}"),
377-
path: Some(config_dir.clone()),
378-
io_error: Some(e.to_string()),
379-
})?;
395+
if let Some(config_dir) = config_path.parent() {
396+
if !config_dir.exists() {
397+
fs::create_dir_all(config_dir).map_err(|e| LashError::IO {
398+
code: codes::E_IO_WRITE_ERROR,
399+
message: format!("Failed to create user config directory: {e}"),
400+
path: Some(config_dir.to_path_buf()),
401+
io_error: Some(e.to_string()),
402+
})?;
403+
}
380404
}
381405

382406
// Serialize to TOML
@@ -718,33 +742,48 @@ mod tests {
718742

719743
#[test]
720744
fn test_user_config_load_nonexistent() {
721-
// This test assumes ~/.lash/config.toml doesn't exist or is valid
722-
// If it exists, it should parse successfully
723-
let result = UserConfig::load();
724-
assert!(result.is_ok());
745+
let dir = TempDir::new().unwrap();
746+
let missing = dir.path().join("config.toml");
747+
748+
let loaded = UserConfig::load_from(&missing).unwrap();
749+
750+
assert_eq!(loaded, UserConfig::default());
751+
assert!(!missing.exists(), "load must not create the config file");
725752
}
726753

727754
#[test]
728755
fn test_user_config_save_and_load() {
729-
// Create a custom config
756+
let dir = TempDir::new().unwrap();
757+
let path = dir.path().join("config.toml");
758+
730759
let config = UserConfig {
731760
color_scheme: "Test Theme".to_string(),
732761
tree_view: TreeViewConfig::default(),
733762
};
763+
config.save_to(&path).unwrap();
734764

735-
// Save it
736-
let save_result = config.save();
737-
assert!(
738-
save_result.is_ok(),
739-
"Failed to save config: {:?}",
740-
save_result
741-
);
765+
let loaded = UserConfig::load_from(&path).unwrap();
742766

743-
// Load it back
744-
let loaded = UserConfig::load().unwrap();
745-
assert_eq!(loaded.color_scheme, "Test Theme");
767+
assert_eq!(loaded, config);
768+
}
769+
770+
#[test]
771+
fn test_user_config_save_creates_missing_parent_directory() {
772+
let dir = TempDir::new().unwrap();
773+
let path = dir.path().join("nested").join(".lash").join("config.toml");
774+
775+
UserConfig::default().save_to(&path).unwrap();
776+
777+
assert!(path.exists());
778+
assert_eq!(UserConfig::load_from(&path).unwrap(), UserConfig::default());
779+
}
780+
781+
#[test]
782+
fn test_user_config_load_rejects_invalid_toml() {
783+
let dir = TempDir::new().unwrap();
784+
let path = dir.path().join("config.toml");
785+
fs::write(&path, "color_scheme = ").unwrap();
746786

747-
// Restore default
748-
UserConfig::default().save().unwrap();
787+
assert!(UserConfig::load_from(&path).is_err());
749788
}
750789
}

0 commit comments

Comments
 (0)