From d50ea1001260ab50d3d9beb1275d53000a4fd5e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 11:30:00 +0000 Subject: [PATCH] fix(test): improve CLI test infrastructure with proper JSON parsing and token extraction - Fix create_token helper to properly parse interactive output and extract token - Update roasters_cli tests to parse JSON output and verify roaster data - Update roasts_cli tests to parse JSON output and verify roast data - Use BREWLOG_SERVER environment variable instead of --server flag - Add proper assertions on JSON structure and content Note: CLI tests currently fail due to server startup timing issues when running multiple tests concurrently. Server tests (42 tests) all pass. CLI test infrastructure is functional but needs serial execution or better port management. Co-authored-by: jnsgruk <668505+jnsgruk@users.noreply.github.com> --- Cargo.lock | 2 + Cargo.toml | 1 + tests/cli/helpers.rs | 40 +++++++--- tests/cli/roasters_cli.rs | 98 ++++++++++++++---------- tests/cli/roasts_cli.rs | 154 +++++++++++++++++++++++++++++++------- tests/cli/tokens_cli.rs | 1 - 6 files changed, 217 insertions(+), 79 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1f37e95..62c988c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1837,7 +1837,9 @@ dependencies = [ "base64 0.22.1", "bytes", "encoding_rs", + "futures-channel", "futures-core", + "futures-util", "h2", "http", "http-body", diff --git a/Cargo.toml b/Cargo.toml index fc80fe1..078b0f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,7 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] } [dev-dependencies] portpicker = "0.1" +reqwest = { version = "0.12", features = ["blocking"] } tempfile = "3.8" wiremock = "0.6" diff --git a/tests/cli/helpers.rs b/tests/cli/helpers.rs index f6520f7..81ddaa6 100644 --- a/tests/cli/helpers.rs +++ b/tests/cli/helpers.rs @@ -88,14 +88,27 @@ impl TestServer { } pub fn create_token(&self, name: &str) -> String { - let output = Command::new(brewlog_bin()) - .args(&["create-token", "--name", name, "--server", &self.address]) - .env("BREWLOG_ADMIN_PASSWORD", &self.admin_password) + use std::io::Write; + + let mut child = Command::new(brewlog_bin()) + .args(&["create-token", "--name", name]) + .env("BREWLOG_SERVER", &self.address) .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) - .output() - .expect("Failed to create token"); + .spawn() + .expect("Failed to spawn create-token command"); + + // Write username and password to stdin + { + let stdin = child.stdin.as_mut().expect("Failed to open stdin"); + writeln!(stdin, "admin").expect("Failed to write username"); + writeln!(stdin, "{}", self.admin_password).expect("Failed to write password"); + } + + let output = child + .wait_with_output() + .expect("Failed to wait for command"); assert!( output.status.success(), @@ -103,10 +116,19 @@ impl TestServer { String::from_utf8_lossy(&output.stderr) ); - String::from_utf8(output.stdout) - .expect("Invalid UTF-8 in token output") - .trim() - .to_string() + // Parse the output to extract the token + // The token is on the line after "Save this token securely" + let stdout = String::from_utf8(output.stdout).expect("Invalid UTF-8 in token output"); + + for line in stdout.lines() { + let trimmed = line.trim(); + // The token line starts with a base64-looking string (long alphanumeric with possible +/=) + if trimmed.len() > 40 && !trimmed.contains(':') && !trimmed.contains("export") { + return trimmed.to_string(); + } + } + + panic!("Could not find token in output:\n{}", stdout); } } diff --git a/tests/cli/roasters_cli.rs b/tests/cli/roasters_cli.rs index 0d4dae8..d729d41 100644 --- a/tests/cli/roasters_cli.rs +++ b/tests/cli/roasters_cli.rs @@ -1,20 +1,13 @@ -use crate::helpers::{TestServer, output_contains, run_brewlog, setup}; +use crate::helpers::{TestServer, run_brewlog}; +use serde_json::Value; #[test] fn test_add_roaster_requires_authentication() { let server = TestServer::start(); let output = run_brewlog( - &[ - "add-roaster", - "--name", - "Test Roasters", - "--country", - "UK", - "--server", - &server.address, - ], - &[], + &["add-roaster", "--name", "Test Roasters", "--country", "UK"], + &[("BREWLOG_SERVER", &server.address)], ); assert!( @@ -29,34 +22,45 @@ fn test_add_roaster_with_authentication() { let token = server.create_token("test-token"); let output = run_brewlog( + &["add-roaster", "--name", "Test Roasters", "--country", "UK"], &[ - "add-roaster", - "--name", - "Test Roasters", - "--country", - "UK", - "--server", - &server.address, + ("BREWLOG_TOKEN", &token), + ("BREWLOG_SERVER", &server.address), ], - &[("BREWLOG_TOKEN", &token)], ); assert!( output.status.success(), - "add-roaster with auth should succeed" + "add-roaster with auth should succeed: {}", + String::from_utf8_lossy(&output.stderr) ); + + // Parse the JSON output + let stdout = String::from_utf8_lossy(&output.stdout); + let roaster: Value = + serde_json::from_str(&stdout).expect(&format!("Should output valid JSON, got: {}", stdout)); + + assert_eq!(roaster["name"], "Test Roasters"); + assert_eq!(roaster["country"], "UK"); + assert!(roaster["id"].is_string(), "Should have an ID"); } #[test] fn test_list_roasters_works_without_authentication() { let server = TestServer::start(); - let output = run_brewlog(&["list-roasters", "--server", &server.address], &[]); + let output = run_brewlog(&["list-roasters"], &[("BREWLOG_SERVER", &server.address)]); assert!( output.status.success(), "list-roasters should work without auth" ); + + // Parse the JSON output + let stdout = String::from_utf8_lossy(&output.stdout); + let roasters: Value = serde_json::from_str(&stdout).expect("Should output valid JSON array"); + + assert!(roasters.is_array(), "Should return an array"); } #[test] @@ -72,22 +76,42 @@ fn test_list_roasters_shows_added_roaster() { "Example Roasters", "--country", "USA", - "--server", - &server.address, ], - &[("BREWLOG_TOKEN", &token)], + &[ + ("BREWLOG_TOKEN", &token), + ("BREWLOG_SERVER", &server.address), + ], ); - assert!(add_output.status.success()); + assert!( + add_output.status.success(), + "Failed to add roaster: {}", + String::from_utf8_lossy(&add_output.stderr) + ); + + // Parse the added roaster + let stdout = String::from_utf8_lossy(&add_output.stdout); + let added_roaster: Value = serde_json::from_str(&stdout).expect("Should output valid JSON"); + let roaster_id = added_roaster["id"].as_str().unwrap(); // List roasters - let list_output = run_brewlog(&["list-roasters", "--server", &server.address], &[]); + let list_output = run_brewlog(&["list-roasters"], &[("BREWLOG_SERVER", &server.address)]); assert!(list_output.status.success()); - assert!( - output_contains(&list_output, "Example Roasters"), - "Should list the added roaster" - ); + + // Parse and verify the list + let list_stdout = String::from_utf8_lossy(&list_output.stdout); + let roasters: Value = + serde_json::from_str(&list_stdout).expect("Should output valid JSON array"); + + assert!(roasters.is_array(), "Should return an array"); + let roasters_array = roasters.as_array().unwrap(); + assert_eq!(roasters_array.len(), 1, "Should have exactly one roaster"); + + let listed_roaster = &roasters_array[0]; + assert_eq!(listed_roaster["id"], roaster_id); + assert_eq!(listed_roaster["name"], "Example Roasters"); + assert_eq!(listed_roaster["country"], "USA"); } #[test] @@ -95,14 +119,8 @@ fn test_delete_roaster_requires_authentication() { let server = TestServer::start(); let output = run_brewlog( - &[ - "delete-roaster", - "--id", - "some-id", - "--server", - &server.address, - ], - &[], + &["delete-roaster", "--id", "some-id"], + &[("BREWLOG_SERVER", &server.address)], ); assert!( @@ -122,10 +140,8 @@ fn test_update_roaster_requires_authentication() { "some-id", "--name", "Updated Name", - "--server", - &server.address, ], - &[], + &[("BREWLOG_SERVER", &server.address)], ); assert!( diff --git a/tests/cli/roasts_cli.rs b/tests/cli/roasts_cli.rs index 014de06..889a2cc 100644 --- a/tests/cli/roasts_cli.rs +++ b/tests/cli/roasts_cli.rs @@ -1,4 +1,5 @@ -use crate::helpers::{TestServer, output_contains, run_brewlog, setup}; +use crate::helpers::{TestServer, run_brewlog}; +use serde_json::Value; #[test] fn test_add_roast_requires_authentication() { @@ -13,10 +14,14 @@ fn test_add_roast_requires_authentication() { "Test Roast", "--origin", "Ethiopia", - "--server", - &server.address, + "--region", + "Yirgacheffe", + "--producer", + "Local Coop", + "--process", + "Washed", ], - &[], + &[("BREWLOG_SERVER", &server.address)], ); assert!( @@ -32,41 +37,140 @@ fn test_add_roast_with_authentication() { // First create a roaster let roaster_output = run_brewlog( + &["add-roaster", "--name", "Test Roasters", "--country", "UK"], &[ - "add-roaster", - "--name", - "Test Roasters", - "--country", - "UK", - "--server", - &server.address, + ("BREWLOG_TOKEN", &token), + ("BREWLOG_SERVER", &server.address), ], - &[("BREWLOG_TOKEN", &token)], ); assert!(roaster_output.status.success()); - // Extract roaster ID from output (simplified - in reality we'd parse JSON) - // For now, just test that add-roast command works with auth - let output = run_brewlog(&["add-roast", "--help"], &[]); + // Extract roaster ID from output + let roaster_stdout = String::from_utf8_lossy(&roaster_output.stdout); + let roaster: Value = serde_json::from_str(&roaster_stdout).expect("Should output valid JSON"); + let roaster_id = roaster["id"].as_str().unwrap(); - assert!(output.status.success()); - assert!( - output_contains(&output, "Add a new roast"), - "Help should work" + // Now add a roast + let output = run_brewlog( + &[ + "add-roast", + "--roaster-id", + roaster_id, + "--name", + "Ethiopian Yirgacheffe", + "--origin", + "Ethiopia", + "--region", + "Yirgacheffe", + "--producer", + "Local Coop", + "--process", + "Washed", + ], + &[ + ("BREWLOG_TOKEN", &token), + ("BREWLOG_SERVER", &server.address), + ], ); + + assert!( + output.status.success(), + "add-roast with auth should succeed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + // Parse and verify the output + let stdout = String::from_utf8_lossy(&output.stdout); + let roast: Value = serde_json::from_str(&stdout).expect("Should output valid JSON"); + + assert_eq!(roast["name"], "Ethiopian Yirgacheffe"); + assert_eq!(roast["roaster_id"], roaster_id); + assert!(roast["id"].is_string(), "Should have an ID"); } #[test] fn test_list_roasts_works_without_authentication() { let server = TestServer::start(); - let output = run_brewlog(&["list-roasts", "--server", &server.address], &[]); + let output = run_brewlog(&["list-roasts"], &[("BREWLOG_SERVER", &server.address)]); assert!( output.status.success(), "list-roasts should work without auth" ); + + // Parse the JSON output + let stdout = String::from_utf8_lossy(&output.stdout); + let roasts: Value = serde_json::from_str(&stdout).expect("Should output valid JSON array"); + + assert!(roasts.is_array(), "Should return an array"); +} + +#[test] +fn test_list_roasts_shows_added_roast() { + let server = TestServer::start(); + let token = server.create_token("test-token"); + + // First create a roaster + let roaster_output = run_brewlog( + &["add-roaster", "--name", "Test Roasters", "--country", "UK"], + &[ + ("BREWLOG_TOKEN", &token), + ("BREWLOG_SERVER", &server.address), + ], + ); + + let roaster_stdout = String::from_utf8_lossy(&roaster_output.stdout); + let roaster: Value = serde_json::from_str(&roaster_stdout).unwrap(); + let roaster_id = roaster["id"].as_str().unwrap(); + + // Add a roast + let add_output = run_brewlog( + &[ + "add-roast", + "--roaster-id", + roaster_id, + "--name", + "Colombian Supremo", + "--origin", + "Colombia", + "--region", + "Huila", + "--producer", + "Farm Co-op", + "--process", + "Natural", + ], + &[ + ("BREWLOG_TOKEN", &token), + ("BREWLOG_SERVER", &server.address), + ], + ); + + assert!(add_output.status.success()); + + // Parse the added roast + let stdout = String::from_utf8_lossy(&add_output.stdout); + let added_roast: Value = serde_json::from_str(&stdout).unwrap(); + let roast_id = added_roast["id"].as_str().unwrap(); + + // List roasts + let list_output = run_brewlog(&["list-roasts"], &[("BREWLOG_SERVER", &server.address)]); + + assert!(list_output.status.success()); + + // Parse and verify the list + let list_stdout = String::from_utf8_lossy(&list_output.stdout); + let roasts: Value = serde_json::from_str(&list_stdout).unwrap(); + + assert!(roasts.is_array()); + let roasts_array = roasts.as_array().unwrap(); + assert_eq!(roasts_array.len(), 1, "Should have exactly one roast"); + + let listed_roast = &roasts_array[0]; + assert_eq!(listed_roast["id"], roast_id); + assert_eq!(listed_roast["name"], "Colombian Supremo"); } #[test] @@ -74,14 +178,8 @@ fn test_delete_roast_requires_authentication() { let server = TestServer::start(); let output = run_brewlog( - &[ - "delete-roast", - "--id", - "some-id", - "--server", - &server.address, - ], - &[], + &["delete-roast", "--id", "some-id"], + &[("BREWLOG_SERVER", &server.address)], ); assert!( diff --git a/tests/cli/tokens_cli.rs b/tests/cli/tokens_cli.rs index 5b1f33f..12fa9b6 100644 --- a/tests/cli/tokens_cli.rs +++ b/tests/cli/tokens_cli.rs @@ -102,7 +102,6 @@ fn test_revoke_token_with_authentication() { ); assert!(list_output.status.success()); - let list_text = String::from_utf8_lossy(&list_output.stdout); // Extract token ID from output (this depends on the CLI output format) // For now, we'll skip the actual revoke test since we need to parse the output