fix(test): replace fixed sleep with health check polling in CLI test server startup

Co-authored-by: jnsgruk <668505+jnsgruk@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot] 2025-11-25 11:18:43 +00:00 committed by Jon Seager
parent 94c2403370
commit 34d5b157fe
No known key found for this signature in database
4 changed files with 158 additions and 94 deletions

View file

@ -12,7 +12,7 @@ pub fn setup() {
.args(&["build", "--bin", "brewlog"]) .args(&["build", "--bin", "brewlog"])
.status() .status()
.expect("Failed to build brewlog binary"); .expect("Failed to build brewlog binary");
assert!(status.success(), "Failed to compile brewlog"); assert!(status.success(), "Failed to compile brewlog");
}); });
} }
@ -43,14 +43,14 @@ pub struct TestServer {
impl TestServer { impl TestServer {
pub fn start() -> Self { pub fn start() -> Self {
setup(); setup();
let (temp_dir, db_url) = create_test_db(); let (temp_dir, db_url) = create_test_db();
let admin_password = "test_admin_password"; let admin_password = "test_admin_password";
// Start server on a random port // Start server on a random port
let port = portpicker::pick_unused_port().expect("No ports available"); let port = portpicker::pick_unused_port().expect("No ports available");
let address = format!("http://127.0.0.1:{}", port); let address = format!("http://127.0.0.1:{}", port);
let process = Command::new(brewlog_bin()) let process = Command::new(brewlog_bin())
.args(&["serve", "--port", &port.to_string(), "--database", &db_url]) .args(&["serve", "--port", &port.to_string(), "--database", &db_url])
.env("BREWLOG_ADMIN_PASSWORD", admin_password) .env("BREWLOG_ADMIN_PASSWORD", admin_password)
@ -59,10 +59,25 @@ impl TestServer {
.stderr(Stdio::null()) .stderr(Stdio::null())
.spawn() .spawn()
.expect("Failed to start brewlog server"); .expect("Failed to start brewlog server");
// Wait for server to be ready // Wait for server to be ready with health check
std::thread::sleep(std::time::Duration::from_secs(2)); 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 { Self {
address, address,
admin_password: admin_password.to_string(), admin_password: admin_password.to_string(),
@ -71,7 +86,7 @@ impl TestServer {
_process: process, _process: process,
} }
} }
pub fn create_token(&self, name: &str) -> String { pub fn create_token(&self, name: &str) -> String {
let output = Command::new(brewlog_bin()) let output = Command::new(brewlog_bin())
.args(&["create-token", "--name", name, "--server", &self.address]) .args(&["create-token", "--name", name, "--server", &self.address])
@ -81,9 +96,13 @@ impl TestServer {
.stderr(Stdio::piped()) .stderr(Stdio::piped())
.output() .output()
.expect("Failed to create token"); .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) String::from_utf8(output.stdout)
.expect("Invalid UTF-8 in token output") .expect("Invalid UTF-8 in token output")
.trim() .trim()
@ -101,11 +120,11 @@ impl Drop for TestServer {
pub fn run_brewlog(args: &[&str], env: &[(&str, &str)]) -> std::process::Output { pub fn run_brewlog(args: &[&str], env: &[(&str, &str)]) -> std::process::Output {
let mut cmd = Command::new(brewlog_bin()); let mut cmd = Command::new(brewlog_bin());
cmd.args(args); cmd.args(args);
for (key, value) in env { for (key, value) in env {
cmd.env(key, value); cmd.env(key, value);
} }
cmd.output().expect("Failed to run brewlog command") cmd.output().expect("Failed to run brewlog command")
} }

View file

@ -1,9 +1,9 @@
use crate::helpers::{output_contains, run_brewlog, setup, TestServer}; use crate::helpers::{TestServer, output_contains, run_brewlog, setup};
#[test] #[test]
fn test_add_roaster_requires_authentication() { fn test_add_roaster_requires_authentication() {
let server = TestServer::start(); let server = TestServer::start();
let output = run_brewlog( let output = run_brewlog(
&[ &[
"add-roaster", "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] #[test]
fn test_add_roaster_with_authentication() { fn test_add_roaster_with_authentication() {
let server = TestServer::start(); let server = TestServer::start();
let token = server.create_token("test-token"); let token = server.create_token("test-token");
let output = run_brewlog( let output = run_brewlog(
&[ &[
"add-roaster", "add-roaster",
@ -37,27 +40,30 @@ fn test_add_roaster_with_authentication() {
], ],
&[("BREWLOG_TOKEN", &token)], &[("BREWLOG_TOKEN", &token)],
); );
assert!(output.status.success(), "add-roaster with auth should succeed"); assert!(
output.status.success(),
"add-roaster with auth should succeed"
);
} }
#[test] #[test]
fn test_list_roasters_works_without_authentication() { fn test_list_roasters_works_without_authentication() {
let server = TestServer::start(); let server = TestServer::start();
let output = run_brewlog( let output = run_brewlog(&["list-roasters", "--server", &server.address], &[]);
&["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] #[test]
fn test_list_roasters_shows_added_roaster() { fn test_list_roasters_shows_added_roaster() {
let server = TestServer::start(); let server = TestServer::start();
let token = server.create_token("test-token"); let token = server.create_token("test-token");
// Add a roaster // Add a roaster
let add_output = run_brewlog( let add_output = run_brewlog(
&[ &[
@ -71,23 +77,23 @@ fn test_list_roasters_shows_added_roaster() {
], ],
&[("BREWLOG_TOKEN", &token)], &[("BREWLOG_TOKEN", &token)],
); );
assert!(add_output.status.success()); assert!(add_output.status.success());
// List roasters // List roasters
let list_output = run_brewlog( let list_output = run_brewlog(&["list-roasters", "--server", &server.address], &[]);
&["list-roasters", "--server", &server.address],
&[],
);
assert!(list_output.status.success()); 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] #[test]
fn test_delete_roaster_requires_authentication() { fn test_delete_roaster_requires_authentication() {
let server = TestServer::start(); let server = TestServer::start();
let output = run_brewlog( let output = run_brewlog(
&[ &[
"delete-roaster", "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] #[test]
fn test_update_roaster_requires_authentication() { fn test_update_roaster_requires_authentication() {
let server = TestServer::start(); let server = TestServer::start();
let output = run_brewlog( let output = run_brewlog(
&[ &[
"update-roaster", "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"
);
} }

View file

@ -1,9 +1,9 @@
use crate::helpers::{output_contains, run_brewlog, setup, TestServer}; use crate::helpers::{TestServer, output_contains, run_brewlog, setup};
#[test] #[test]
fn test_add_roast_requires_authentication() { fn test_add_roast_requires_authentication() {
let server = TestServer::start(); let server = TestServer::start();
let output = run_brewlog( let output = run_brewlog(
&[ &[
"add-roast", "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] #[test]
fn test_add_roast_with_authentication() { fn test_add_roast_with_authentication() {
let server = TestServer::start(); let server = TestServer::start();
let token = server.create_token("test-token"); let token = server.create_token("test-token");
// First create a roaster // First create a roaster
let roaster_output = run_brewlog( let roaster_output = run_brewlog(
&[ &[
@ -40,36 +43,36 @@ fn test_add_roast_with_authentication() {
], ],
&[("BREWLOG_TOKEN", &token)], &[("BREWLOG_TOKEN", &token)],
); );
assert!(roaster_output.status.success()); assert!(roaster_output.status.success());
// Extract roaster ID from output (simplified - in reality we'd parse JSON) // Extract roaster ID from output (simplified - in reality we'd parse JSON)
// For now, just test that add-roast command works with auth // For now, just test that add-roast command works with auth
let output = run_brewlog( let output = run_brewlog(&["add-roast", "--help"], &[]);
&["add-roast", "--help"],
&[],
);
assert!(output.status.success()); 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] #[test]
fn test_list_roasts_works_without_authentication() { fn test_list_roasts_works_without_authentication() {
let server = TestServer::start(); let server = TestServer::start();
let output = run_brewlog( let output = run_brewlog(&["list-roasts", "--server", &server.address], &[]);
&["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] #[test]
fn test_delete_roast_requires_authentication() { fn test_delete_roast_requires_authentication() {
let server = TestServer::start(); let server = TestServer::start();
let output = run_brewlog( let output = run_brewlog(
&[ &[
"delete-roast", "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"
);
} }

View file

@ -1,26 +1,38 @@
use crate::helpers::{output_contains, run_brewlog, setup, TestServer}; use crate::helpers::{TestServer, output_contains, run_brewlog, setup};
#[test] #[test]
fn test_create_token_without_server_fails() { fn test_create_token_without_server_fails() {
setup(); setup();
let output = run_brewlog( 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()); assert!(!output.status.success());
} }
#[test] #[test]
fn test_create_token_with_valid_credentials() { fn test_create_token_with_valid_credentials() {
let server = TestServer::start(); let server = TestServer::start();
let output = run_brewlog( 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.status.success(), "create-token should succeed");
assert!(!output.stdout.is_empty(), "Should output a token"); assert!(!output.stdout.is_empty(), "Should output a token");
} }
@ -28,63 +40,78 @@ fn test_create_token_with_valid_credentials() {
#[test] #[test]
fn test_list_tokens_requires_authentication() { fn test_list_tokens_requires_authentication() {
let server = TestServer::start(); let server = TestServer::start();
let output = run_brewlog( let output = run_brewlog(&["list-tokens", "--server", &server.address], &[]);
&["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] #[test]
fn test_list_tokens_with_authentication() { fn test_list_tokens_with_authentication() {
let server = TestServer::start(); let server = TestServer::start();
let token = server.create_token("test-token"); let token = server.create_token("test-token");
let output = run_brewlog( let output = run_brewlog(
&["list-tokens", "--server", &server.address], &["list-tokens", "--server", &server.address],
&[("BREWLOG_TOKEN", &token)], &[("BREWLOG_TOKEN", &token)],
); );
assert!(output.status.success(), "list-tokens with auth should succeed"); assert!(
assert!(output_contains(&output, "test-token"), "Should list the created token"); output.status.success(),
"list-tokens with auth should succeed"
);
assert!(
output_contains(&output, "test-token"),
"Should list the created token"
);
} }
#[test] #[test]
fn test_revoke_token_requires_authentication() { fn test_revoke_token_requires_authentication() {
let server = TestServer::start(); let server = TestServer::start();
let output = run_brewlog( 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] #[test]
fn test_revoke_token_with_authentication() { fn test_revoke_token_with_authentication() {
let server = TestServer::start(); let server = TestServer::start();
let token = server.create_token("test-token"); let token = server.create_token("test-token");
// First list tokens to get the ID // First list tokens to get the ID
let list_output = run_brewlog( let list_output = run_brewlog(
&["list-tokens", "--server", &server.address], &["list-tokens", "--server", &server.address],
&[("BREWLOG_TOKEN", &token)], &[("BREWLOG_TOKEN", &token)],
); );
assert!(list_output.status.success()); assert!(list_output.status.success());
let list_text = String::from_utf8_lossy(&list_output.stdout); let list_text = String::from_utf8_lossy(&list_output.stdout);
// Extract token ID from output (this depends on the CLI output format) // 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 // For now, we'll skip the actual revoke test since we need to parse the output
// Just verify that revoke command exists // Just verify that revoke command exists
let output = run_brewlog( let output = run_brewlog(&["revoke-token", "--help"], &[]);
&["revoke-token", "--help"],
&[],
);
assert!(output.status.success(), "revoke-token --help should work"); 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"
);
} }