Code improvements, more tests, better file organization

CLAUDE.md

@@ -0,0 +1,88 @@
+# CLAUDE.md
+
+This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
+
+## Project Overview
+
+Server Runner is a Rust CLI tool that starts multiple web servers, waits for them to be ready (HTTP 200 responses), then executes a command when all servers are running. It's designed for automated testing workflows where you need to spin up multiple services before running tests.
+
+## Development Commands
+
+### Building
+```bash
+cargo build                    # Debug build
+cargo build --release          # Release build
+```
+
+### Testing
+```bash
+cargo test                     # Run all tests
+cargo test -- --nocapture      # Run tests with stdout output
+```
+
+### Running
+```bash
+cargo run                      # Run with default config (servers.yaml)
+cargo run -- -c config.yaml    # Run with custom config
+cargo run -- -v                # Run with verbose output
+cargo run -- -a 5              # Run with custom max attempts (default: 10)
+```
+
+### Installation
+```bash
+cargo install --path .         # Install locally from source
+```
+
+## Architecture
+
+### Core Components
+
+**Main Flow** (`src/main.rs:75-142`):
+1. Parse CLI arguments and load YAML configuration
+2. Start all server processes concurrently 
+3. Poll each server URL until HTTP 200 or max attempts reached
+4. Execute the final command when all servers are ready
+5. Clean up all processes on completion or failure
+
+**Key Structures**:
+- `Config` - Deserializes YAML with servers list and final command
+- `Server` - Individual server configuration (name, URL, command)
+- `ServerProcess` - Running process wrapper with name and Child process
+- `ServerStatus` - Enum tracking Waiting/Running states
+- `Attempts` - Newtype for attempt counting with custom operators
+
+**Process Management**:
+- Uses `std::process::Command` to spawn server processes
+- Implements Ctrl+C handler for graceful shutdown
+- Cross-platform process creation (Windows-specific flags)
+- Automatic cleanup of all child processes
+
+**HTTP Health Checks**:
+- Uses `reqwest::blocking` for synchronous HTTP requests
+- Distinguishes between connection errors (retry) and other failures (abort)
+- Configurable retry attempts with 1-second intervals
+
+### Configuration Format
+
+YAML configuration with two main sections:
+```yaml
+servers:                    # List of servers to start
+  - name: "Server Name"
+    url: "http://localhost:8080"
+    command: "node server.js"
+command: "npm test"         # Command to run when all servers ready
+```
+
+Example configs in repository:
+- `servers.yaml` - Basic example with simple-http-server
+- `max_attempts.yaml` - Test config for connection failure scenarios
+
+### Testing Strategy
+
+Uses `assert_cmd` for CLI integration tests (`tests/cli.rs`):
+- Success case with default config
+- Missing config file error handling  
+- Max attempts exceeded scenarios
+- Custom attempt limit configuration
+
+Tests require `simple-http-server` to be installed for the working config scenario.

Cargo.toml

@@ -22,6 +22,7 @@ reqwest = { version = "0.12.19", features = [
     "native-tls-vendored",
 ] }
 serde = { version = "1", features = ["derive"] }
+shlex = "1.3.0"
 simplelog = "0.12.2"
 
 [dev-dependencies]

servers.yaml

@@ -2,4 +2,5 @@ servers:
   - name: "Hello World"
     url: "http://localhost:3000"
     command: "simple-http-server -p 3000 -i -s"
+    timeout: 10
 command: "sleep 5s"

src/main.rs

@@ -29,6 +29,12 @@ struct Server {
     name: String,
     url: String,
     command: String,
+    #[serde(default = "default_timeout")]
+    timeout: u64,
+}
+
+fn default_timeout() -> u64 {
+    5
 }
 
 #[derive(serde::Deserialize)]
@@ -95,8 +101,11 @@ fn run(args: Args) -> anyhow::Result<()> {
         let mut processes = server_processes_clone.lock();
 
         match stop_servers(&mut processes) {
-            Ok(_) => {}
-            Err(e) => exit_with_error(e),
+            Ok(_) => info!("All servers stopped successfully"),
+            Err(e) => {
+                eprintln!("Error stopping servers: {}", e);
+                std::process::exit(1);
+            }
         };
 
         std::process::exit(0);
@@ -163,6 +172,14 @@ fn get_config(filename: &str) -> anyhow::Result<Config> {
         .try_deserialize::<Config>()
         .context(format!("Could not parse config file {}", filename))?;
 
+    if config.servers.is_empty() {
+        bail!("Configuration must include at least one server");
+    }
+
+    if config.command.trim().is_empty() {
+        bail!("Configuration must include a command to run");
+    }
+
     Ok(config)
 }
 
@@ -194,7 +211,9 @@ fn stop_servers(
     for p in processes.iter_mut() {
         info!("Stopping server {}", p.name);
 
-        if p.process.kill().is_err() {
+        if let Ok(_) = p.process.kill() {
+            let _ = p.process.wait();
+        } else {
             bail!("Failed to stop process {}", p.name);
         }
     }
@@ -205,13 +224,14 @@ fn stop_servers(
 }
 
 fn run_command(command: &str) -> anyhow::Result<Child> {
-    let command_parts: Vec<&str> = command.split(' ').collect();
+    let command_parts = shlex::split(command)
+        .ok_or_else(|| anyhow::anyhow!("Invalid command: {}", command))?;
 
     if command_parts.is_empty() {
         bail!("Empty command provided");
     }
 
-    let mut cmd = Command::new(command_parts[0]);
+    let mut cmd = Command::new(&command_parts[0]);
 
     for part in command_parts.iter().skip(1) {
         cmd.arg(part);
@@ -230,7 +250,7 @@ fn check_server(
     server_attempts: &mut HashMap<ServerName, Attempts>,
     max_attempts: u8,
 ) -> anyhow::Result<ServerStatus> {
-    let Server { name, url, .. } = server;
+    let Server { name, url, timeout, .. } = server;
 
     let attempts = server_attempts
         .entry(ServerName(name.to_owned()))
@@ -250,7 +270,11 @@ fn check_server(
         name, url, attempts
     );
 
-    let result = match reqwest::blocking::get(url) {
+    let client = reqwest::blocking::Client::builder()
+        .timeout(Duration::from_secs(*timeout))
+        .build()?;
+
+    let result = match client.get(url).send() {
         Ok(response) => response.status(),
         Err(error) => {
             if error.is_connect() {

tests/cli.rs

@@ -28,7 +28,7 @@ fn fails_on_too_many_attempts() {
 
     command
         .arg("-c")
-        .arg("max_attempts.yaml")
+        .arg("tests/max_attempts.yaml")
         .assert()
         .failure()
         .stderr(predicate::str::contains(
@@ -42,7 +42,7 @@ fn fails_on_too_many_attempts_custom() {
 
     command
         .arg("-c")
-        .arg("max_attempts.yaml")
+        .arg("tests/max_attempts.yaml")
         .arg("-a")
         .arg("5")
         .assert()
@@ -51,3 +51,123 @@ fn fails_on_too_many_attempts_custom() {
             "Could not connect to server Hello World after 5 attempts",
         ));
 }
+
+#[test]
+fn fails_on_empty_server_list() {
+    let mut command = Command::cargo_bin("server-runner").unwrap();
+
+    command
+        .arg("-c")
+        .arg("tests/empty_servers.yaml")
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains(
+            "Configuration must include at least one server",
+        ));
+}
+
+#[test]
+fn fails_on_timeout_with_custom_timeout() {
+    let mut command = Command::cargo_bin("server-runner").unwrap();
+
+    command
+        .arg("-c")
+        .arg("tests/timeout.yaml")
+        .arg("-a")
+        .arg("2")
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains(
+            "Could not connect to server Timeout Test Server after 2 attempts",
+        ));
+}
+
+#[test]
+fn fails_on_empty_command() {
+    let mut command = Command::cargo_bin("server-runner").unwrap();
+
+    command
+        .arg("-c")
+        .arg("tests/empty_command.yaml")
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains(
+            "Configuration must include a command to run",
+        ));
+}
+
+#[test]
+fn fails_on_invalid_yaml() {
+    let mut command = Command::cargo_bin("server-runner").unwrap();
+
+    command
+        .arg("-c")
+        .arg("tests/invalid_yaml.yaml")
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains(
+            "Could not find config file tests/invalid_yaml.yaml",
+        ));
+}
+
+#[test]
+fn fails_on_missing_required_fields() {
+    let mut command = Command::cargo_bin("server-runner").unwrap();
+
+    command
+        .arg("-c")
+        .arg("tests/missing_fields.yaml")
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains(
+            "Could not parse config file tests/missing_fields.yaml",
+        ));
+}
+
+#[test]
+fn fails_on_multiple_unreachable_servers() {
+    let mut command = Command::cargo_bin("server-runner").unwrap();
+
+    command
+        .arg("-c")
+        .arg("tests/multiple_servers.yaml")
+        .arg("-a")
+        .arg("2")
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains(
+            "Could not connect to server",
+        ));
+}
+
+#[test]
+fn fails_on_zero_timeout() {
+    let mut command = Command::cargo_bin("server-runner").unwrap();
+
+    command
+        .arg("-c")
+        .arg("tests/zero_timeout.yaml")
+        .arg("-a")
+        .arg("1")
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains(
+            "Could not connect to server Zero Timeout Server after 1 attempts",
+        ));
+}
+
+#[test]
+fn fails_on_one_attempt() {
+    let mut command = Command::cargo_bin("server-runner").unwrap();
+
+    command
+        .arg("-c")
+        .arg("tests/timeout.yaml")
+        .arg("-a")
+        .arg("1")
+        .assert()
+        .failure()
+        .stderr(predicate::str::contains(
+            "Could not connect to server Timeout Test Server after 1 attempts",
+        ));
+}

tests/empty_command.yaml

@@ -0,0 +1,5 @@
+servers:
+  - name: "Test Server"
+    url: "http://localhost:3000"
+    command: "simple-http-server -p 3000 -i -s"
+command: ""

tests/empty_servers.yaml

@@ -0,0 +1,2 @@
+servers: []
+command: "echo 'test'"

tests/invalid_yaml.yaml

@@ -0,0 +1,6 @@
+servers:
+  - name: "Test Server"
+    url: "http://localhost:3000"
+    command: "simple-http-server -p 3000 -i -s"
+command: "echo test"
+[invalid yaml syntax: unclosed bracket

tests/missing_fields.yaml

@@ -0,0 +1,5 @@
+servers:
+  - name: "Test Server"
+    url: "http://localhost:3000"
+    # missing command field
+command: "echo test"

tests/multiple_servers.yaml

@@ -0,0 +1,13 @@
+servers:
+  - name: "Server One"
+    url: "http://localhost:8001"
+    command: "echo 'fake server 1'"
+    timeout: 2
+  - name: "Server Two"
+    url: "http://localhost:8002"
+    command: "echo 'fake server 2'"
+    timeout: 1
+  - name: "Server Three"
+    url: "http://localhost:8003"
+    command: "echo 'fake server 3'"
+command: "echo 'all done'"

tests/timeout.yaml

@@ -0,0 +1,6 @@
+servers:
+  - name: "Timeout Test Server"
+    url: "http://localhost:9999"
+    command: "echo 'fake server'"
+    timeout: 1
+command: "echo 'done'"