From 34d5b157fef653a2135eed138d904f15a705755d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 11:18:43 +0000 Subject: [PATCH] fix(test): replace fixed sleep with health check polling in CLI test server startup Co-authored-by: jnsgruk <668505+jnsgruk@users.noreply.github.com> --- tests/cli/helpers.rs | 47 ++++++++++++++------- tests/cli/roasters_cli.rs | 68 +++++++++++++++++------------- tests/cli/roasts_cli.rs | 50 ++++++++++++---------- tests/cli/tokens_cli.rs | 87 +++++++++++++++++++++++++-------------- 4 files changed, 158 insertions(+), 94 deletions(-) diff --git a/tests/cli/helpers.rs b/tests/cli/helpers.rs index 54092bd..f6520f7 100644 --- a/tests/cli/helpers.rs +++ b/tests/cli/helpers.rs @@ -12,7 +12,7 @@ pub fn setup() { .args(&["build", "--bin", "brewlog"]) .status() .expect("Failed to build brewlog binary"); - + assert!(status.success(), "Failed to compile brewlog"); }); } @@ -43,14 +43,14 @@ pub struct TestServer { impl TestServer { pub fn start() -> Self { setup(); - + let (temp_dir, db_url) = create_test_db(); let admin_password = "test_admin_password"; - + // Start server on a random port let port = portpicker::pick_unused_port().expect("No ports available"); let address = format!("http://127.0.0.1:{}", port); - + let process = Command::new(brewlog_bin()) .args(&["serve", "--port", &port.to_string(), "--database", &db_url]) .env("BREWLOG_ADMIN_PASSWORD", admin_password) @@ -59,10 +59,25 @@ impl TestServer { .stderr(Stdio::null()) .spawn() .expect("Failed to start brewlog server"); - - // Wait for server to be ready - std::thread::sleep(std::time::Duration::from_secs(2)); - + + // Wait for server to be ready with health check + let client = reqwest::blocking::Client::new(); + let health_url = format!("{}/api/v1/roasters", address); + let max_attempts = 30; // 30 seconds total + let mut attempts = 0; + + while attempts < max_attempts { + if client.get(&health_url).send().is_ok() { + break; + } + std::thread::sleep(std::time::Duration::from_millis(100)); + attempts += 1; + } + + if attempts >= max_attempts { + panic!("Server failed to start within 30 seconds"); + } + Self { address, admin_password: admin_password.to_string(), @@ -71,7 +86,7 @@ impl TestServer { _process: process, } } - + pub fn create_token(&self, name: &str) -> String { let output = Command::new(brewlog_bin()) .args(&["create-token", "--name", name, "--server", &self.address]) @@ -81,9 +96,13 @@ impl TestServer { .stderr(Stdio::piped()) .output() .expect("Failed to create token"); - - assert!(output.status.success(), "Failed to create token: {}", String::from_utf8_lossy(&output.stderr)); - + + assert!( + output.status.success(), + "Failed to create token: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout) .expect("Invalid UTF-8 in token output") .trim() @@ -101,11 +120,11 @@ impl Drop for TestServer { pub fn run_brewlog(args: &[&str], env: &[(&str, &str)]) -> std::process::Output { let mut cmd = Command::new(brewlog_bin()); cmd.args(args); - + for (key, value) in env { cmd.env(key, value); } - + cmd.output().expect("Failed to run brewlog command") } diff --git a/tests/cli/roasters_cli.rs b/tests/cli/roasters_cli.rs index 5c62240..0d4dae8 100644 --- a/tests/cli/roasters_cli.rs +++ b/tests/cli/roasters_cli.rs @@ -1,9 +1,9 @@ -use crate::helpers::{output_contains, run_brewlog, setup, TestServer}; +use crate::helpers::{TestServer, output_contains, run_brewlog, setup}; #[test] fn test_add_roaster_requires_authentication() { let server = TestServer::start(); - + let output = run_brewlog( &[ "add-roaster", @@ -16,15 +16,18 @@ fn test_add_roaster_requires_authentication() { ], &[], ); - - assert!(!output.status.success(), "add-roaster without auth should fail"); + + assert!( + !output.status.success(), + "add-roaster without auth should fail" + ); } #[test] fn test_add_roaster_with_authentication() { let server = TestServer::start(); let token = server.create_token("test-token"); - + let output = run_brewlog( &[ "add-roaster", @@ -37,27 +40,30 @@ fn test_add_roaster_with_authentication() { ], &[("BREWLOG_TOKEN", &token)], ); - - assert!(output.status.success(), "add-roaster with auth should succeed"); + + assert!( + output.status.success(), + "add-roaster with auth should succeed" + ); } #[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", "--server", &server.address], &[]); + + assert!( + output.status.success(), + "list-roasters should work without auth" ); - - assert!(output.status.success(), "list-roasters should work without auth"); } #[test] fn test_list_roasters_shows_added_roaster() { let server = TestServer::start(); let token = server.create_token("test-token"); - + // Add a roaster let add_output = run_brewlog( &[ @@ -71,23 +77,23 @@ fn test_list_roasters_shows_added_roaster() { ], &[("BREWLOG_TOKEN", &token)], ); - + assert!(add_output.status.success()); - + // List roasters - let list_output = run_brewlog( - &["list-roasters", "--server", &server.address], - &[], - ); - + let list_output = run_brewlog(&["list-roasters", "--server", &server.address], &[]); + assert!(list_output.status.success()); - assert!(output_contains(&list_output, "Example Roasters"), "Should list the added roaster"); + assert!( + output_contains(&list_output, "Example Roasters"), + "Should list the added roaster" + ); } #[test] fn test_delete_roaster_requires_authentication() { let server = TestServer::start(); - + let output = run_brewlog( &[ "delete-roaster", @@ -98,14 +104,17 @@ fn test_delete_roaster_requires_authentication() { ], &[], ); - - assert!(!output.status.success(), "delete-roaster without auth should fail"); + + assert!( + !output.status.success(), + "delete-roaster without auth should fail" + ); } #[test] fn test_update_roaster_requires_authentication() { let server = TestServer::start(); - + let output = run_brewlog( &[ "update-roaster", @@ -118,6 +127,9 @@ fn test_update_roaster_requires_authentication() { ], &[], ); - - assert!(!output.status.success(), "update-roaster without auth should fail"); + + assert!( + !output.status.success(), + "update-roaster without auth should fail" + ); } diff --git a/tests/cli/roasts_cli.rs b/tests/cli/roasts_cli.rs index 7dff2ef..014de06 100644 --- a/tests/cli/roasts_cli.rs +++ b/tests/cli/roasts_cli.rs @@ -1,9 +1,9 @@ -use crate::helpers::{output_contains, run_brewlog, setup, TestServer}; +use crate::helpers::{TestServer, output_contains, run_brewlog, setup}; #[test] fn test_add_roast_requires_authentication() { let server = TestServer::start(); - + let output = run_brewlog( &[ "add-roast", @@ -18,15 +18,18 @@ fn test_add_roast_requires_authentication() { ], &[], ); - - assert!(!output.status.success(), "add-roast without auth should fail"); + + assert!( + !output.status.success(), + "add-roast without auth should fail" + ); } #[test] fn test_add_roast_with_authentication() { let server = TestServer::start(); let token = server.create_token("test-token"); - + // First create a roaster let roaster_output = run_brewlog( &[ @@ -40,36 +43,36 @@ fn test_add_roast_with_authentication() { ], &[("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"], - &[], - ); - + let output = run_brewlog(&["add-roast", "--help"], &[]); + assert!(output.status.success()); - assert!(output_contains(&output, "Add a new roast"), "Help should work"); + assert!( + output_contains(&output, "Add a new roast"), + "Help should work" + ); } #[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", "--server", &server.address], &[]); + + assert!( + output.status.success(), + "list-roasts should work without auth" ); - - assert!(output.status.success(), "list-roasts should work without auth"); } #[test] fn test_delete_roast_requires_authentication() { let server = TestServer::start(); - + let output = run_brewlog( &[ "delete-roast", @@ -80,6 +83,9 @@ fn test_delete_roast_requires_authentication() { ], &[], ); - - assert!(!output.status.success(), "delete-roast without auth should fail"); + + assert!( + !output.status.success(), + "delete-roast without auth should fail" + ); } diff --git a/tests/cli/tokens_cli.rs b/tests/cli/tokens_cli.rs index a8f9199..5b1f33f 100644 --- a/tests/cli/tokens_cli.rs +++ b/tests/cli/tokens_cli.rs @@ -1,26 +1,38 @@ -use crate::helpers::{output_contains, run_brewlog, setup, TestServer}; +use crate::helpers::{TestServer, output_contains, run_brewlog, setup}; #[test] fn test_create_token_without_server_fails() { setup(); - + let output = run_brewlog( - &["create-token", "--name", "test-token", "--server", "http://localhost:9999"], + &[ + "create-token", + "--name", + "test-token", + "--server", + "http://localhost:9999", + ], &[], ); - + assert!(!output.status.success()); } #[test] fn test_create_token_with_valid_credentials() { let server = TestServer::start(); - + let output = run_brewlog( - &["create-token", "--name", "test-token", "--server", &server.address], + &[ + "create-token", + "--name", + "test-token", + "--server", + &server.address, + ], &[], ); - + assert!(output.status.success(), "create-token should succeed"); assert!(!output.stdout.is_empty(), "Should output a token"); } @@ -28,63 +40,78 @@ fn test_create_token_with_valid_credentials() { #[test] fn test_list_tokens_requires_authentication() { let server = TestServer::start(); - - let output = run_brewlog( - &["list-tokens", "--server", &server.address], - &[], + + let output = run_brewlog(&["list-tokens", "--server", &server.address], &[]); + + assert!( + !output.status.success(), + "list-tokens without auth should fail" ); - - assert!(!output.status.success(), "list-tokens without auth should fail"); } #[test] fn test_list_tokens_with_authentication() { let server = TestServer::start(); let token = server.create_token("test-token"); - + let output = run_brewlog( &["list-tokens", "--server", &server.address], &[("BREWLOG_TOKEN", &token)], ); - - assert!(output.status.success(), "list-tokens with auth should succeed"); - assert!(output_contains(&output, "test-token"), "Should list the created token"); + + assert!( + output.status.success(), + "list-tokens with auth should succeed" + ); + assert!( + output_contains(&output, "test-token"), + "Should list the created token" + ); } #[test] fn test_revoke_token_requires_authentication() { let server = TestServer::start(); - + let output = run_brewlog( - &["revoke-token", "--id", "some-id", "--server", &server.address], + &[ + "revoke-token", + "--id", + "some-id", + "--server", + &server.address, + ], &[], ); - - assert!(!output.status.success(), "revoke-token without auth should fail"); + + assert!( + !output.status.success(), + "revoke-token without auth should fail" + ); } #[test] fn test_revoke_token_with_authentication() { let server = TestServer::start(); let token = server.create_token("test-token"); - + // First list tokens to get the ID let list_output = run_brewlog( &["list-tokens", "--server", &server.address], &[("BREWLOG_TOKEN", &token)], ); - + 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 // Just verify that revoke command exists - let output = run_brewlog( - &["revoke-token", "--help"], - &[], - ); - + let output = run_brewlog(&["revoke-token", "--help"], &[]); + assert!(output.status.success(), "revoke-token --help should work"); - assert!(output_contains(&output, "Revoke"), "Help should mention revoke"); + assert!( + output_contains(&output, "Revoke"), + "Help should mention revoke" + ); }