diff --git a/Cargo.lock b/Cargo.lock index de6d8d4..6746d87 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2144,6 +2144,7 @@ dependencies = [ "rstest", "serde", "serde_json", + "serial_test", "tempfile", "tokio", "tracing", @@ -2285,6 +2286,15 @@ version = "1.0.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "28d3b2b1366ec20994f1fd18c3c594f05c5dd4bc44d8bb0c1c632c8d6829481f" +[[package]] +name = "scc" +version = "2.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46e6f046b7fef48e2660c57ed794263155d713de679057f2d0c169bfc6e756cc" +dependencies = [ + "sdd", +] + [[package]] name = "schannel" version = "0.1.28" @@ -2324,6 +2334,12 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "sdd" +version = "3.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "490dcfcbfef26be6800d11870ff2df8774fa6e86d047e3e8c8a76b25655e41ca" + [[package]] name = "security-framework" version = "2.11.1" @@ -2469,6 +2485,31 @@ dependencies = [ "syn", ] +[[package]] +name = "serial_test" +version = "3.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b258109f244e1d6891bf1053a55d63a5cd4f8f4c30cf9a1280989f80e7a1fa9" +dependencies = [ + "futures", + "log", + "once_cell", + "parking_lot", + "scc", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "3.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d69265a08751de7844521fd15003ae0a888e035773ba05695c5c759a6f89eef" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "sha2" version = "0.10.9" diff --git a/Cargo.toml b/Cargo.toml index e70a435..40b21d2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,7 @@ tracing-subscriber = "0.3" [dependencies.nix] version = "0.30.1" -features = ["fs"] +features = ["fs", "resource"] [dependencies.clap] version = "4.5" @@ -46,6 +46,7 @@ features = [ [dev-dependencies] rstest = "0.24" +serial_test = "3.2" tempfile = "3.13" [dev-dependencies.cargo-husky] diff --git a/rustfmt.toml b/rustfmt.toml index a3f43d4..e523805 100644 --- a/rustfmt.toml +++ b/rustfmt.toml @@ -2,6 +2,8 @@ edition = "2024" style_edition = "2024" comment_width = 100 format_code_in_doc_comments = true +format_macro_bodies = true +format_macro_matchers = true imports_granularity = "Crate" imports_layout = "HorizontalVertical" wrap_comments = true diff --git a/src/setup.rs b/src/setup.rs index 040bbc3..5f7df13 100644 --- a/src/setup.rs +++ b/src/setup.rs @@ -66,7 +66,7 @@ pub struct Args { /// IRC Username pub username: Option, - #[arg(long)] + #[arg(long = "no-tls")] /// Whether or not to use TLS when connecting to the IRC server. pub use_tls: Option, } @@ -87,6 +87,15 @@ pub async fn init() -> Result { // variables if applicable. let args = Args::parse(); + let settings = make_config(args)?; + + Ok(Setup { config: settings }) +} + +/// Create a configuration object from arguments. +/// +/// This is exposed for testing purposes. +pub fn make_config(args: Args) -> Result { // Use default config location unless specified. let config_location: PathBuf = if let Some(ref path) = args.config_file { path.to_owned() @@ -100,7 +109,7 @@ pub async fn init() -> Result { info!("Starting."); - let settings = Config::builder() + Config::builder() .add_source(config::File::with_name(&config_location.to_string_lossy()).required(false)) .add_source(config::Environment::with_prefix("BOT")) // Doing all of these overrides provides a unified access point for options, @@ -110,15 +119,14 @@ pub async fn init() -> Result { .set_override_option("chroot-dir", args.chroot_dir.clone())? .set_override_option("command-path", args.command_dir.clone())? .set_override_option("model", args.model.clone())? + .set_override_option("nick-password", args.nick_password.clone())? .set_override_option("instruct", args.instruct.clone())? .set_override_option("channels", args.channels.clone())? .set_override_option("server", args.server.clone())? - .set_override_option("port", args.port.clone())? // FIXME: Make this a default here not in clap. + .set_override_option("port", args.port.clone())? .set_override_option("nickname", args.nickname.clone())? .set_override_option("username", args.username.clone())? - .set_override_option("use_tls", args.use_tls)? + .set_override_option("use-tls", args.use_tls)? .build() - .wrap_err("Couldn't read configuration settings.")?; - - Ok(Setup { config: settings }) + .wrap_err("Couldn't read configuration settings.") } diff --git a/tests/setup_test.rs b/tests/setup_test.rs new file mode 100644 index 0000000..e23bc62 --- /dev/null +++ b/tests/setup_test.rs @@ -0,0 +1,556 @@ +use robotnik::setup::{Args, make_config}; +use serial_test::serial; +use std::{fs, path::PathBuf}; +use tempfile::TempDir; + +/// Helper to create a temporary config file +fn create_config_file(dir: &TempDir, content: &str) -> PathBuf { + let config_path = dir.path().join("config.toml"); + fs::write(&config_path, content).unwrap(); + config_path +} + +/// Helper to parse config using environment and config file +async fn parse_config_from_file(config_path: &PathBuf) -> config::Config { + config::Config::builder() + .add_source(config::File::with_name(&config_path.to_string_lossy()).required(true)) + .build() + .unwrap() +} + +#[tokio::test] +#[serial] +async fn test_setup_make_config_overrides() { + let temp = TempDir::new().unwrap(); + let config_content = "\ +api-key = \"file-key\" +model = \"file-model\" +port = 6667 +"; + let config_path = create_config_file(&temp, config_content); + + // Construct Args with overrides + let args = Args { + api_key: Some("cli-key".to_string()), + base_url: None, /* Should fail if required and not in file/env? No, base-url is optional + * in args */ + chroot_dir: None, + command_dir: None, + instruct: None, + model: None, // Should fallback to file + channels: None, + config_file: Some(config_path), + server: None, // Should use default or file? Args has default "irc.libera.chat" + port: Some("9999".to_string()), + nickname: None, + nick_password: None, + username: None, + use_tls: None, + }; + + let config = make_config(args).expect("Failed to make config"); + + // Check overrides + assert_eq!(config.get_string("api-key").unwrap(), "cli-key"); + assert_eq!(config.get_string("port").unwrap(), "9999"); + assert_eq!(config.get_int("port").unwrap(), 9999); + + // Check fallback to file + assert_eq!(config.get_string("model").unwrap(), "file-model"); +} + +#[tokio::test] +async fn test_config_file_loads_all_settings() { + let temp = TempDir::new().unwrap(); + let config_content = "\ +api-key = \"test-api-key-123\" +base-url = \"https://api.test.com\" +chroot-dir = \"/test/chroot\" +command-path = \"/test/commands\" +model = \"test-model\" +instruct = \"Test instructions\" +server = \"test.irc.server\" +port = 6667 +channels = [\"#test1\", \"#test2\"] +username = \"testuser\" +nickname = \"testnick\" +use-tls = false +"; + + let config_path = create_config_file(&temp, config_content); + let config = parse_config_from_file(&config_path).await; + + // Verify all settings are loaded correctly + assert_eq!(config.get_string("api-key").unwrap(), "test-api-key-123"); + assert_eq!( + config.get_string("base-url").unwrap(), + "https://api.test.com" + ); + assert_eq!(config.get_string("chroot-dir").unwrap(), "/test/chroot"); + assert_eq!(config.get_string("command-path").unwrap(), "/test/commands"); + assert_eq!(config.get_string("model").unwrap(), "test-model"); + assert_eq!(config.get_string("instruct").unwrap(), "Test instructions"); + assert_eq!(config.get_string("server").unwrap(), "test.irc.server"); + assert_eq!(config.get_int("port").unwrap(), 6667); + + let channels: Vec = config.get("channels").unwrap(); + assert_eq!(channels, vec!["#test1", "#test2"]); + + assert_eq!(config.get_string("username").unwrap(), "testuser"); + assert_eq!(config.get_string("nickname").unwrap(), "testnick"); + assert_eq!(config.get_bool("use-tls").unwrap(), false); +} + +#[tokio::test] +async fn test_config_file_partial_settings() { + let temp = TempDir::new().unwrap(); + // Only provide required settings + let config_content = "\ +api-key = \"minimal-key\" +base-url = \"https://minimal.api.com\" +model = \"minimal-model\" +server = \"minimal.server\" +port = 6697 +channels = [\"#minimal\"] +"; + + let config_path = create_config_file(&temp, config_content); + let config = parse_config_from_file(&config_path).await; + + // Verify required settings are loaded + assert_eq!(config.get_string("api-key").unwrap(), "minimal-key"); + assert_eq!( + config.get_string("base-url").unwrap(), + "https://minimal.api.com" + ); + assert_eq!(config.get_string("model").unwrap(), "minimal-model"); + + // Verify optional settings are not present + assert!(config.get_string("chroot-dir").is_err()); + assert!(config.get_string("instruct").is_err()); + assert!(config.get_string("username").is_err()); +} + +#[tokio::test] +#[serial] +async fn test_config_with_environment_variables() { + // NOTE: This test documents a limitation in setup.rs + // setup.rs uses Environment::with_prefix("BOT") without a separator + // This means BOT_API_KEY maps to "api_key", NOT "api-key" + // Since config.toml uses kebab-case, environment variables won't override properly + // This is a known issue in the current implementation + + let temp = TempDir::new().unwrap(); + let config_content = "\ +api_key = \"file-api-key\" +base_url = \"https://file.api.com\" +model = \"file-model\" +"; + + let config_path = create_config_file(&temp, config_content); + + // Set environment variables (with BOT_ prefix as setup.rs uses) + unsafe { + std::env::set_var("BOT_API_KEY", "env-api-key"); + std::env::set_var("BOT_MODEL", "env-model"); + } + + let config = config::Config::builder() + .add_source(config::File::with_name(&config_path.to_string_lossy()).required(true)) + .add_source(config::Environment::with_prefix("BOT")) + .build() + .unwrap(); + + // Environment variables should override file settings (when using underscore keys) + assert_eq!(config.get_string("api_key").unwrap(), "env-api-key"); + assert_eq!(config.get_string("model").unwrap(), "env-model"); + // File setting should be used when no env var + assert_eq!( + config.get_string("base_url").unwrap(), + "https://file.api.com" + ); + + // Cleanup + unsafe { + std::env::remove_var("BOT_API_KEY"); + std::env::remove_var("BOT_MODEL"); + } +} + +#[tokio::test] +async fn test_command_line_overrides_config_file() { + let temp = TempDir::new().unwrap(); + let config_content = "\ +api-key = \"file-api-key\" +base-url = \"https://file.api.com\" +model = \"file-model\" +server = \"file.server\" +port = 6667 +channels = [\"#file\"] +nickname = \"filenick\" +username = \"fileuser\" +"; + + let config_path = create_config_file(&temp, config_content); + + // Simulate command-line arguments overriding config file + let config = config::Config::builder() + .add_source(config::File::with_name(&config_path.to_string_lossy()).required(true)) + .set_override_option("api-key", Some("cli-api-key".to_string())) + .unwrap() + .set_override_option("model", Some("cli-model".to_string())) + .unwrap() + .set_override_option("server", Some("cli.server".to_string())) + .unwrap() + .set_override_option("nickname", Some("clinick".to_string())) + .unwrap() + .build() + .unwrap(); + + // Command-line values should override file settings + assert_eq!(config.get_string("api-key").unwrap(), "cli-api-key"); + assert_eq!(config.get_string("model").unwrap(), "cli-model"); + assert_eq!(config.get_string("server").unwrap(), "cli.server"); + assert_eq!(config.get_string("nickname").unwrap(), "clinick"); + + // Non-overridden values should come from file + assert_eq!( + config.get_string("base-url").unwrap(), + "https://file.api.com" + ); + assert_eq!(config.get_string("username").unwrap(), "fileuser"); + assert_eq!(config.get_int("port").unwrap(), 6667); +} + +#[tokio::test] +#[serial] +async fn test_command_line_overrides_environment_and_file() { + let temp = TempDir::new().unwrap(); + let config_content = "\ +api_key = \"file-api-key\" +model = \"file-model\" +base_url = \"https://file.api.com\" +"; + + let config_path = create_config_file(&temp, config_content); + + // Set environment variable + unsafe { + std::env::set_var("BOT_API_KEY", "env-api-key"); + } + + // Build config with all three sources + let config = config::Config::builder() + .add_source(config::File::with_name(&config_path.to_string_lossy()).required(true)) + .add_source(config::Environment::with_prefix("BOT")) + .set_override_option("api_key", Some("cli-api-key".to_string())) + .unwrap() + .build() + .unwrap(); + + // Command-line should win over both environment and file + assert_eq!(config.get_string("api_key").unwrap(), "cli-api-key"); + + // Cleanup + unsafe { + std::env::remove_var("BOT_API_KEY"); + } +} + +#[tokio::test] +#[serial] +async fn test_precedence_order() { + // Test: CLI > Environment > Config File > Defaults + // Using underscore keys to match how setup.rs actually works + let temp = TempDir::new().unwrap(); + let config_content = "\ +api_key = \"file-key\" +base_url = \"https://file-url.com\" +model = \"file-model\" +server = \"file-server\" +"; + + let config_path = create_config_file(&temp, config_content); + + // Set environment variables + unsafe { + std::env::set_var("BOT_BASE_URL", "https://env-url.com"); + std::env::set_var("BOT_MODEL", "env-model"); + } + + let config = config::Config::builder() + .add_source(config::File::with_name(&config_path.to_string_lossy()).required(true)) + .add_source(config::Environment::with_prefix("BOT")) + .set_override_option("model", Some("cli-model".to_string())) + .unwrap() + .build() + .unwrap(); + + // CLI overrides everything + assert_eq!(config.get_string("model").unwrap(), "cli-model"); + + // Environment overrides file + assert_eq!( + config.get_string("base_url").unwrap(), + "https://env-url.com" + ); + + // File is used when no env or CLI + assert_eq!(config.get_string("api_key").unwrap(), "file-key"); + assert_eq!(config.get_string("server").unwrap(), "file-server"); + + // Cleanup + unsafe { + std::env::remove_var("BOT_BASE_URL"); + std::env::remove_var("BOT_MODEL"); + } +} + +#[tokio::test] +async fn test_boolean_use_tls_setting() { + let temp = TempDir::new().unwrap(); + + // Test with use-tls = true (kebab-case as in config.toml) + let config_content_true = r#" +use-tls = true +"#; + let config_path = create_config_file(&temp, config_content_true); + let config = parse_config_from_file(&config_path).await; + assert_eq!(config.get_bool("use-tls").unwrap(), true); + + // Test with use-tls = false + let config_content_false = r#" +use-tls = false +"#; + let config_path = create_config_file(&temp, config_content_false); + let config = parse_config_from_file(&config_path).await; + assert_eq!(config.get_bool("use-tls").unwrap(), false); +} + +#[tokio::test] +async fn test_use_tls_naming_inconsistency() { + // This test documents a bug: setup.rs uses "use_tls" (underscore) + // but config.toml uses "use-tls" (kebab-case) + let temp = TempDir::new().unwrap(); + let config_content = r#" +use-tls = true +"#; + + let config_path = create_config_file(&temp, config_content); + + // Build config the way setup.rs does it + let config = config::Config::builder() + .add_source(config::File::with_name(&config_path.to_string_lossy()).required(true)) + // setup.rs line 119 uses "use_tls" (underscore) instead of "use-tls" (kebab) + .set_override_option("use_tls", Some(false)) + .unwrap() + .build() + .unwrap(); + + // This should read from the override (false), not the file (true) + // But due to the naming mismatch, it might not work as expected + // The config file uses "use-tls" but the override uses "use_tls" + + // With kebab-case (matches config.toml) + assert_eq!(config.get_bool("use-tls").unwrap(), true); + + // With underscore (matches setup.rs override) + assert_eq!(config.get_bool("use_tls").unwrap(), false); +} + +#[tokio::test] +async fn test_channels_as_array() { + let temp = TempDir::new().unwrap(); + let config_content = "\ +channels = [\"#chan1\", \"#chan2\", \"#chan3\"] +"; + + let config_path = create_config_file(&temp, config_content); + let config = parse_config_from_file(&config_path).await; + + let channels: Vec = config.get("channels").unwrap(); + assert_eq!(channels.len(), 3); + assert_eq!(channels[0], "#chan1"); + assert_eq!(channels[1], "#chan2"); + assert_eq!(channels[2], "#chan3"); +} + +#[tokio::test] +async fn test_channels_override_from_cli() { + let temp = TempDir::new().unwrap(); + let config_content = "\ +channels = [\"#file1\", \"#file2\"] +"; + + let config_path = create_config_file(&temp, config_content); + + let cli_channels = vec![ + "#cli1".to_string(), + "#cli2".to_string(), + "#cli3".to_string(), + ]; + + let config = config::Config::builder() + .add_source(config::File::with_name(&config_path.to_string_lossy()).required(true)) + .set_override_option("channels", Some(cli_channels.clone())) + .unwrap() + .build() + .unwrap(); + + let channels: Vec = config.get("channels").unwrap(); + assert_eq!(channels, cli_channels); + assert_eq!(channels.len(), 3); +} + +#[tokio::test] +async fn test_port_as_integer() { + let temp = TempDir::new().unwrap(); + let config_content = r#" +port = 6697 +"#; + + let config_path = create_config_file(&temp, config_content); + let config = parse_config_from_file(&config_path).await; + + // Port should be readable as both integer and string + assert_eq!(config.get_int("port").unwrap(), 6697); + assert_eq!(config.get_string("port").unwrap(), "6697"); +} + +#[tokio::test] +async fn test_port_override_from_cli_as_string() { + // setup.rs passes port as Option from clap + let temp = TempDir::new().unwrap(); + let config_content = r#" +port = 6667 +"#; + + let config_path = create_config_file(&temp, config_content); + + let config = config::Config::builder() + .add_source(config::File::with_name(&config_path.to_string_lossy()).required(true)) + .set_override_option("port", Some("9999".to_string())) + .unwrap() + .build() + .unwrap(); + + // CLI override should work + assert_eq!(config.get_string("port").unwrap(), "9999"); + assert_eq!(config.get_int("port").unwrap(), 9999); +} + +#[tokio::test] +async fn test_missing_required_fields_fails() { + let temp = TempDir::new().unwrap(); + // Create config without required api-key + let config_content = r#" +model = "test-model" +"#; + + let config_path = create_config_file(&temp, config_content); + let config = parse_config_from_file(&config_path).await; + + // Should fail when trying to get required field + assert!(config.get_string("api-key").is_err()); +} + +#[tokio::test] +async fn test_optional_instruct_field() { + let temp = TempDir::new().unwrap(); + let config_content = r#" +instruct = "Custom bot instructions" +"#; + + let config_path = create_config_file(&temp, config_content); + let config = parse_config_from_file(&config_path).await; + + assert_eq!( + config.get_string("instruct").unwrap(), + "Custom bot instructions" + ); +} + +#[tokio::test] +async fn test_command_path_field() { + // command-path is in config.toml but not used anywhere in the code + let temp = TempDir::new().unwrap(); + let config_content = r#" +command-path = "/custom/commands" +"#; + + let config_path = create_config_file(&temp, config_content); + let config = parse_config_from_file(&config_path).await; + + assert_eq!( + config.get_string("command-path").unwrap(), + "/custom/commands" + ); +} + +#[tokio::test] +async fn test_chroot_dir_field() { + let temp = TempDir::new().unwrap(); + let config_content = r#" +chroot-dir = "/var/lib/bot/root" +"#; + + let config_path = create_config_file(&temp, config_content); + let config = parse_config_from_file(&config_path).await; + + assert_eq!( + config.get_string("chroot-dir").unwrap(), + "/var/lib/bot/root" + ); +} + +#[tokio::test] +async fn test_empty_config_file() { + let temp = TempDir::new().unwrap(); + let config_content = ""; + + let config_path = create_config_file(&temp, config_content); + + // Should build successfully but have no values + let config = parse_config_from_file(&config_path).await; + + assert!(config.get_string("api-key").is_err()); + assert!(config.get_string("model").is_err()); +} + +#[tokio::test] +async fn test_all_cli_override_keys_match_config_format() { + // This test documents which override keys in setup.rs match the config.toml format + let temp = TempDir::new().unwrap(); + let config_content = "\ +api-key = \"test\" +base-url = \"https://test.com\" +chroot-dir = \"/test\" +command-path = \"/cmds\" +model = \"test-model\" +instruct = \"test\" +channels = [\"#test\"] +server = \"test.server\" +port = 6697 +nickname = \"test\" +username = \"test\" +use-tls = true +"; + + let config_path = create_config_file(&temp, config_content); + let config = parse_config_from_file(&config_path).await; + + // All these should work with kebab-case (as in config.toml) + assert!(config.get_string("api-key").is_ok()); + assert!(config.get_string("base-url").is_ok()); + assert!(config.get_string("chroot-dir").is_ok()); + assert!(config.get_string("command-path").is_ok()); + assert!(config.get_string("model").is_ok()); + assert!(config.get_string("instruct").is_ok()); + let channels_result: Result, _> = config.get("channels"); + assert!(channels_result.is_ok()); + assert!(config.get_string("server").is_ok()); + assert!(config.get_int("port").is_ok()); + assert!(config.get_string("nickname").is_ok()); + assert!(config.get_string("username").is_ok()); + assert!(config.get_bool("use-tls").is_ok()); +}