Skip to content

Commit

Permalink
feat: add 80% minimum code coverage requirement and improve test reli…
Browse files Browse the repository at this point in the history
…ability

- Added 80% minimum code coverage requirement to CI pipeline
- Added EnvGuard for reliable environment variable cleanup in tests
- Added tests for Anthropic provider and provider factory
- Fixed environment variable handling in config tests
- Updated cargo-tarpaulin installation check
  • Loading branch information
jamesbrink committed Feb 9, 2025
1 parent 0fd64cc commit 2d65da0
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 19 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ jobs:

- name: Generate coverage report
run: |
cargo tarpaulin --features testing --out Xml --output-dir coverage
cargo tarpaulin --features testing --out Xml --output-dir coverage --fail-under 80
- name: Upload coverage to Codecov
uses: codecov/codecov-action@v5
Expand Down
8 changes: 4 additions & 4 deletions scripts/check.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ NC='\033[0m' # No Color

echo -e "${BLUE}Running quality checks for Strainer...${NC}\n"

# Check if cargo-tarpaulin is installed
if ! command -v cargo-tarpaulin &> /dev/null; then
echo -e "${RED}cargo-tarpaulin is not installed. Installing...${NC}"
cargo install cargo-tarpaulin
# Check if cargo-tarpaulin is installed and working
if ! cargo tarpaulin --version &> /dev/null; then
echo -e "${BLUE}Installing or updating cargo-tarpaulin...${NC}"
cargo install --force cargo-tarpaulin
fi

# Function to run a command and check its status
Expand Down
38 changes: 38 additions & 0 deletions src/providers/anthropic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use anyhow::Result;

/// Provider implementation for Anthropic's API
#[allow(dead_code)]
#[derive(Debug)]
pub struct AnthropicProvider {
api_key: String,
base_url: String,
Expand Down Expand Up @@ -43,3 +44,40 @@ impl Provider for AnthropicProvider {
})
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_anthropic_provider_new() {
let config = ApiConfig {
api_key: Some("test_key".to_string()),
..Default::default()
};
let provider = AnthropicProvider::new(&config);
assert!(provider.is_ok());
}

#[test]
fn test_anthropic_provider_missing_key() {
let config = ApiConfig::default();
let provider = AnthropicProvider::new(&config);
assert!(provider.is_err());
}

#[test]
fn test_anthropic_provider_rate_limits() {
let config = ApiConfig {
api_key: Some("test_key".to_string()),
..Default::default()
};
let provider = AnthropicProvider::new(&config).unwrap();
let limits = provider.get_rate_limits();
assert!(limits.is_ok());
let limits = limits.unwrap();
assert_eq!(limits.requests_used, 0);
assert_eq!(limits.tokens_used, 0);
assert_eq!(limits.input_tokens_used, 0);
}
}
45 changes: 44 additions & 1 deletion src/providers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::config::ApiConfig;
use anyhow::Result;

/// Trait defining the interface for rate limit providers
pub trait Provider {
pub trait Provider: std::fmt::Debug {
/// Get the current rate limits from the provider
/// Get the current rate limits for the API provider
///
Expand Down Expand Up @@ -41,3 +41,46 @@ pub fn create_provider(config: &ApiConfig) -> Result<Box<dyn Provider>> {

pub mod anthropic;
pub mod rate_limiter;

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_create_anthropic_provider() {
let config = ApiConfig {
provider: String::from("anthropic"),
api_key: Some("test_key".to_string()),
..Default::default()
};
let provider = create_provider(&config);
assert!(provider.is_ok());
}

#[test]
fn test_create_unsupported_provider() {
let config = ApiConfig {
provider: String::from("unsupported"),
..Default::default()
};
let provider = create_provider(&config);
assert!(provider.is_err());
assert_eq!(
provider.unwrap_err().to_string(),
"Unsupported provider: unsupported"
);
}

#[test]
fn test_rate_limit_info_debug() {
let info = RateLimitInfo {
requests_used: 10,
tokens_used: 100,
input_tokens_used: 50,
};
let debug_str = format!("{info:?}");
assert!(debug_str.contains("requests_used: 10"));
assert!(debug_str.contains("tokens_used: 100"));
assert!(debug_str.contains("input_tokens_used: 50"));
}
}
2 changes: 1 addition & 1 deletion src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use anyhow::Result;
use std::collections::HashMap;
use std::sync::{Arc, Mutex};

#[derive(Default)]
#[derive(Default, Debug)]
pub struct MockProvider {
pub calls: Arc<Mutex<Vec<String>>>,
pub responses: Arc<Mutex<HashMap<String, RateLimitInfo>>>,
Expand Down
53 changes: 41 additions & 12 deletions tests/config_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,40 @@ fn test_config_from_file() -> Result<()> {
Ok(())
}

struct EnvGuard {
vars: Vec<&'static str>,
}

impl EnvGuard {
fn new(vars: Vec<&'static str>) -> Self {
// Clean up any existing values
for var in &vars {
env::remove_var(var);
}
Self { vars }
}
}

impl Drop for EnvGuard {
fn drop(&mut self) {
// Clean up on scope exit
for var in &self.vars {
env::remove_var(var);
}
}
}

#[test]
fn test_config_from_env() -> Result<()> {
let _guard = EnvGuard::new(vec![
"STRAINER_API_KEY",
"STRAINER_PROVIDER",
"STRAINER_BASE_URL",
"STRAINER_REQUESTS_PER_MINUTE",
"STRAINER_TOKENS_PER_MINUTE",
]);

// Set test environment variables
env::set_var("STRAINER_API_KEY", "env-test-key");
env::set_var("STRAINER_PROVIDER", "anthropic");
env::set_var("STRAINER_BASE_URL", "https://env.api.com");
Expand All @@ -72,13 +104,6 @@ fn test_config_from_env() -> Result<()> {
assert_eq!(config.limits.requests_per_minute, Some(30));
assert_eq!(config.limits.tokens_per_minute, Some(50_000));

// Cleanup
env::remove_var("STRAINER_API_KEY");
env::remove_var("STRAINER_PROVIDER");
env::remove_var("STRAINER_BASE_URL");
env::remove_var("STRAINER_REQUESTS_PER_MINUTE");
env::remove_var("STRAINER_TOKENS_PER_MINUTE");

Ok(())
}

Expand Down Expand Up @@ -140,6 +165,13 @@ fn test_config_validation() {

#[test]
fn test_load_with_env_override() -> Result<()> {
let _guard = EnvGuard::new(vec![
"STRAINER_API_KEY",
"STRAINER_TOKENS_PER_MINUTE",
"STRAINER_BASE_URL",
"STRAINER_REQUESTS_PER_MINUTE",
]);

let dir = tempdir()?;
let config_path = dir.path().join("strainer.toml");

Expand All @@ -150,14 +182,15 @@ fn test_load_with_env_override() -> Result<()> {
base_url = "https://file.api.com"
[limits]
requests_per_minute = 60
requests_per_minute = 30
"#;

fs::write(&config_path, config_content)?;

// Set environment variables
env::set_var("STRAINER_API_KEY", "env-key");
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()?;
Expand All @@ -168,10 +201,6 @@ fn test_load_with_env_override() -> Result<()> {
// Restore original directory
env::set_current_dir(original_dir)?;

// Clean up environment
env::remove_var("STRAINER_API_KEY");
env::remove_var("STRAINER_TOKENS_PER_MINUTE");

assert_eq!(config.api.api_key, Some("env-key".to_string()));
assert_eq!(
config.api.base_url,
Expand Down

0 comments on commit 2d65da0

Please sign in to comment.