Skip to content

Commit

Permalink
fix: improve config testing and path handling
Browse files Browse the repository at this point in the history
- Add proper home directory expansion using dirs crate
- Add DirGuard for safer directory changes in tests
- Fix test isolation issues with environment variables
- Improve test organization and imports
- Fix config loading to handle paths consistently across environments
  • Loading branch information
jamesbrink committed Feb 9, 2025
1 parent 2f7dddd commit 460522e
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 21 deletions.
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,5 @@ serde_json = "1.0"
toml = "0.8"
reqwest = { version = "0.12", features = ["rustls-tls"] }
anyhow = "1.0"
nix = { version = "0.29", features = ["signal", "process"] }
nix = { version = "0.29", features = ["signal", "process"] }
dirs = "5.0"
12 changes: 7 additions & 5 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,17 +214,19 @@ impl Config {
/// This function will return an error if:
/// - Configuration validation fails
pub fn load() -> Result<Self> {
let home_dir = dirs::home_dir().unwrap_or_else(|| PathBuf::from("."));

let config_paths = [
"strainer.toml",
"~/.config/strainer/config.toml",
"/etc/strainer/config.toml",
PathBuf::from("strainer.toml"),
home_dir.join(".config/strainer/config.toml"),
PathBuf::from("/etc/strainer/config.toml"),
];

let mut config = Self::default();

// Load from file if found
for path in config_paths {
if let Ok(file_config) = Self::from_file(&PathBuf::from(path)) {
for path in &config_paths {
if let Ok(file_config) = Self::from_file(path) {
config = file_config;
break;
}
Expand Down
63 changes: 48 additions & 15 deletions tests/config_test.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use std::{env, fs};
use std::{env, fs, path::PathBuf};

use anyhow::Result;
use serde_json::json;
use strainer::config::Config;
use tempfile::tempdir;

use strainer::config::Config;

#[test]
fn test_default_config() {
let config = Config::default();
Expand Down Expand Up @@ -76,6 +77,25 @@ impl Drop for EnvGuard {
}
}

struct DirGuard {
original_dir: PathBuf,
}

impl DirGuard {
fn new() -> Result<Self> {
Ok(Self {
original_dir: env::current_dir()?,
})
}
}

impl Drop for DirGuard {
fn drop(&mut self) {
// Restore original directory on scope exit, ignore errors in drop
let _ = env::set_current_dir(&self.original_dir);
}
}

#[test]
fn test_config_from_env() -> Result<()> {
// First, clear any existing environment variables
Expand Down Expand Up @@ -105,15 +125,31 @@ fn test_config_from_env() -> Result<()> {
"STRAINER_TOKENS_PER_MINUTE",
]);

// Create a fresh config from environment
let config = Config::from_env()?;
// Test 1: Direct environment config
let env_config = Config::from_env()?;

// Verify the config values directly
assert_eq!(config.api.api_key, Some("env-key".to_string()));
assert_eq!(config.api.provider, "anthropic");
assert_eq!(config.api.base_url, Some("https://env.api.com".to_string()));
assert_eq!(config.limits.requests_per_minute, Some(30));
assert_eq!(config.limits.tokens_per_minute, Some(50_000));
// Verify direct environment values
assert_eq!(env_config.api.api_key, Some("env-key".to_string()));
assert_eq!(env_config.api.provider, "anthropic");
assert_eq!(
env_config.api.base_url,
Some("https://env.api.com".to_string())
);
assert_eq!(env_config.limits.requests_per_minute, Some(30));
assert_eq!(env_config.limits.tokens_per_minute, Some(50_000));

// Test 2: Full config loading process
let loaded_config = Config::load()?;

// Verify loaded config values are same as environment
assert_eq!(loaded_config.api.api_key, Some("env-key".to_string()));
assert_eq!(loaded_config.api.provider, "anthropic");
assert_eq!(
loaded_config.api.base_url,
Some("https://env.api.com".to_string())
);
assert_eq!(loaded_config.limits.requests_per_minute, Some(30));
assert_eq!(loaded_config.limits.tokens_per_minute, Some(50_000));

Ok(())
}
Expand Down Expand Up @@ -204,15 +240,12 @@ fn test_load_with_env_override() -> Result<()> {
env::set_var("STRAINER_TOKENS_PER_MINUTE", "50000");
env::set_var("STRAINER_REQUESTS_PER_MINUTE", "60");

// Change to the temporary directory
let original_dir = env::current_dir()?;
// Create directory guard and change to temp directory
let _dir_guard = DirGuard::new()?;
env::set_current_dir(dir.path())?;

let config = Config::load()?;

// Restore original directory
env::set_current_dir(original_dir)?;

// Environment variables should override file values
assert_eq!(config.api.api_key, Some("env-key".to_string()));
assert_eq!(config.api.base_url, Some("https://env.api.com".to_string()));
Expand Down

0 comments on commit 460522e

Please sign in to comment.