Merge branch 'lukebakken-fix/misc'

Author: michaelklishin

AGENTS.md

@@ -13,8 +13,8 @@ cargo build
 
 cargo fmt --all
 
-cargo nextest run --all-features
-cargo clippy --all-features
+RUSTFLAGS="-D warnings" cargo nextest run --all-features
+RUSTFLAGS="-D warnings" cargo clippy --all-features
 ```
 
 To [filter](https://nexte.st/docs/filtersets/) tests with `cargo nextest`:
@@ -26,7 +26,7 @@ cargo nextest run -E "test(test_name)"
 ### Test Node Configuration
 
 Test suites require a RabbitMQ node running on `localhost:15672` with `rabbitmq_management` plugin enabled.
-`bin/ci/before_build.sh` is a script that demonstrates how the node should be configured.
+See [CONTRIBUTING.md](./CONTRIBUTING.md) for step-by-step Docker setup instructions.
 
 
 ## Key Files
@@ -78,7 +78,7 @@ Test suites require a RabbitMQ node running on `localhost:15672` with `rabbitmq_
  * Add integration tests to `tests/integration/`, unit tests to `tests/unit/`, property-based tests to `tests/proptests/`
  * Never add tests in the implementation files
  * At the end of each task, run `cargo fmt --all`
- * At the end of each task, run `cargo clippy --all` and fix any warnings it might emit
+ * At the end of each task, run `RUSTFLAGS="-D warnings" cargo clippy --all-features` and fix any warnings it might emit
 
 ## Comments
 

CONTRIBUTING.md

@@ -2,37 +2,89 @@
 
 ## Running Tests
 
-While tests support the standard `cargo test` option, another option
-for running tests is [`cargo nextest`](https://nexte.st/).
+Most tests require a locally running RabbitMQ node with default ports.
+This can be a locally installed package, a [generic binary build](https://rabbitmq.com/docs/install-generic-unix)
+or a container that uses the [community OCI image](https://github.com/docker-library/rabbitmq).
 
-### Test Structure
+### Prerequisites
 
-Tests are organized into three directories under `tests/`:
+Install [cargo-nextest](https://nexte.st/):
 
- * `tests/integration/`: integration tests that drive the CLI binary and require a running RabbitMQ node
- * `tests/unit/`: unit tests with no external dependencies
- * `tests/proptests/`: property-based tests
+```bash
+cargo install cargo-nextest
+```
+
+### Step 1: Start RabbitMQ
+
+```bash
+docker run -d --name rabbitmq \
+  -p 15672:15672 \
+  -p 5672:5672 \
+  rabbitmq:4-management
+```
+
+Wait for the node to boot:
+
+```bash
+sleep 15
+```
+
+### Step 2: Pre-configure the Node
+
+Run the setup script using the Docker exec variant of `rabbitmqctl`:
+
+```bash
+RUST_HTTP_API_CLIENT_RABBITMQCTL=DOCKER:rabbitmq bin/ci/before_build.sh
+```
+
+This enables the required plugins (management, shovel, federation, stream), creates test users,
+sets up the `rust/rabbitmqadmin` virtual host, sets the cluster name,
+and enables all stable feature flags.
 
-### Run All Tests
+Wait for the changes to apply:
 
-``` bash
+```bash
+sleep 10
+```
+
+### Step 3: Run All Tests
+
+```bash
 NEXTEST_RETRIES=3 cargo nextest run --all-features
 ```
 
+`NEXTEST_RETRIES=3` retries each failing test up to 3 times.
+
+### Stopping the Node
+
+```bash
+docker stop rabbitmq && docker rm rabbitmq
+```
+
+---
+
+## Test Structure
+
+Tests are organized into three directories under `tests/`:
+
+ * `tests/integration/`: integration tests that drive the CLI binary and require a running RabbitMQ node
+ * `tests/unit/`: unit tests with no external dependencies
+ * `tests/proptests/`: property-based tests
+
 ### Run Only Unit and Property-Based Tests (No Local RabbitMQ Node Needed)
 
-``` bash
+```bash
 cargo nextest run -E 'binary(unit) or binary(proptests)'
 ```
 
 ### Run Only Integration Tests
 
-``` bash
+```bash
 NEXTEST_RETRIES=3 cargo nextest run -E 'binary(integration)'
 ```
 
 ### Run a Specific Test
 
-``` bash
+```bash
 NEXTEST_RETRIES=3 cargo nextest run -E "test(test_list_all_vhost_limits)"
 ```

bin/ci/before_build_tls.sh

@@ -81,7 +81,7 @@ if [ -n "$CONTAINER_ID" ]; then
     echo "    -p 15671:15671 -p 15672:15672 -p 5672:5672 \\"
     echo "    -v ${CERTS_DIR}:/certs:ro \\"
     echo "    -v ${RABBITMQ_CONF}:/etc/rabbitmq/rabbitmq.conf:ro \\"
-    echo "    rabbitmq:4.0-management"
+    echo "    rabbitmq:4-management"
 fi
 
 # Enable management plugin (should already be enabled in the management image)

src/errors.rs

@@ -40,22 +40,20 @@ pub struct HealthCheckInfo {
 
 #[derive(thiserror::Error, Debug)]
 pub enum CommandRunError {
-    #[error("Asked to run an unknown command '{command} {subcommand}")]
+    #[error("Asked to run an unknown command '{command} {subcommand}'")]
     UnknownCommandTarget { command: String, subcommand: String },
     #[error("Missing required argument: {name}")]
     MissingRequiredArgument { name: String },
     #[error("Invalid value for argument '{name}': {message}")]
     InvalidArgumentValue { name: String, message: String },
     #[error(
-        "Local TLS certificate file at {local_path} does not exist, cannot be read or passed as a PEM file: {cause}"
+        "Local TLS certificate file at {local_path} could not be parsed as a PEM file: {cause}"
     )]
     CertificateFileCouldNotBeLoaded1 {
         local_path: String,
         cause: reqwest::Error,
     },
-    #[error(
-        "Local TLS certificate file at {local_path} does not exist, cannot be read or passed as a PEM file: {cause}"
-    )]
+    #[error("Local TLS certificate file at {local_path} could not be read: {cause}")]
     CertificateFileCouldNotBeLoaded2 {
         local_path: String,
         cause: rustls::pki_types::pem::Error,
@@ -72,7 +70,7 @@ pub enum CommandRunError {
     CertificateFileInvalidPem { local_path: String, details: String },
     #[error("TLS private key file at {local_path} contains an unsupported key type or format")]
     PrivateKeyFileUnsupported { local_path: String },
-    #[error("TLS certificate and private key files do not match")]
+    #[error("TLS certificate {cert_path} and private key {key_path} do not match")]
     CertificateKeyMismatch { cert_path: String, key_path: String },
     #[error("{}", format_client_error(&.0.status_code, &.0.error_details))]
     ClientError(Box<HttpErrorInfo>),
@@ -90,11 +88,11 @@ pub enum CommandRunError {
     MissingArgumentValue { property: String },
     #[error("Unsupported argument value for property (field) {property}")]
     UnsupportedArgumentValue { property: String },
-    #[error("This request produces an invalid HTTP header value")]
+    #[error("This request produces an invalid HTTP header value: {error}")]
     InvalidHeaderValue { error: InvalidHeaderValue },
-    #[error("Response is incompatible with the target data type")]
+    #[error("Response is incompatible with the target data type: {error}")]
     IncompatibleBody { error: ConversionError },
-    #[error("Encountered an error when performing an HTTP request")]
+    #[error("Encountered an error when performing an HTTP request: {error}")]
     RequestError { error: reqwest::Error },
     #[error("Failed to build HTTP client: {0}")]
     HttpClientBuildError(reqwest::Error),

src/output.rs

@@ -308,7 +308,7 @@ impl<'a> ResultHandler<'a> {
                 println!("{}", table);
             }
             Err(ref e) => {
-                println!("Error: {:?}", e);
+                eprintln!("{}", e);
                 self.exit_code = Some(ExitCode::Unavailable);
             }
         }
@@ -318,6 +318,13 @@ impl<'a> ResultHandler<'a> {
         eprintln!("{}", error);
         let code = match error {
             CommandRunError::UnknownCommandTarget { .. } => ExitCode::Usage,
+            CommandRunError::MissingRequiredArgument { .. } => ExitCode::Usage,
+            CommandRunError::InvalidArgumentValue { .. } => ExitCode::Usage,
+            CommandRunError::ConflictingOptions { .. } => ExitCode::Usage,
+            CommandRunError::MissingOptions { .. } => ExitCode::Usage,
+            CommandRunError::MissingArgumentValue { .. } => ExitCode::Usage,
+            CommandRunError::UnsupportedArgumentValue { .. } => ExitCode::Usage,
+            CommandRunError::InvalidBaseUri { .. } => ExitCode::Usage,
             CommandRunError::CertificateFileCouldNotBeLoaded1 { .. } => ExitCode::DataErr,
             CommandRunError::CertificateFileCouldNotBeLoaded2 { .. } => ExitCode::DataErr,
             CommandRunError::CertificateFileNotFound { .. } => ExitCode::DataErr,
@@ -328,7 +335,16 @@ impl<'a> ResultHandler<'a> {
             CommandRunError::IoError { .. } => ExitCode::DataErr,
             CommandRunError::FailureDuringExecution { .. } => ExitCode::DataErr,
             CommandRunError::HttpClientBuildError { .. } => ExitCode::DataErr,
-            _ => ExitCode::Usage,
+            CommandRunError::ClientError { .. } => ExitCode::DataErr,
+            CommandRunError::ServerError { .. } => ExitCode::DataErr,
+            CommandRunError::NotFound => ExitCode::DataErr,
+            CommandRunError::InvalidHeaderValue { .. } => ExitCode::DataErr,
+            CommandRunError::IncompatibleBody { .. } => ExitCode::DataErr,
+            CommandRunError::RequestError { .. } => ExitCode::DataErr,
+            CommandRunError::JsonParseError { .. } => ExitCode::DataErr,
+            CommandRunError::Other => ExitCode::DataErr,
+            // HealthCheckFailed is handled separately in health_check_result
+            CommandRunError::HealthCheckFailed { .. } => ExitCode::Unavailable,
         };
         self.exit_code = Some(code);
     }
@@ -542,7 +558,7 @@ impl ProgressReporter for QuietProgressReporter {
         // Silent
     }
 
-    fn finish_operation(&mut self, total: usize) {
-        println!("Completed: {} items processed", total);
+    fn finish_operation(&mut self, _total: usize) {
+        // Silent
     }
 }

tests/integration/health_check_tests.rs

@@ -15,6 +15,7 @@
 use crate::test_helpers::output_includes;
 use crate::test_helpers::{run_fails, run_succeeds};
 use std::error::Error;
+
 #[test]
 fn test_health_check_local_alarms() -> Result<(), Box<dyn Error>> {
     run_succeeds(["health_check", "local_alarms"]).stdout(output_includes("passed"));
@@ -72,3 +73,21 @@ fn test_health_check_protocol_listener_fails() -> Result<(), Box<dyn Error>> {
 
     Ok(())
 }
+
+#[test]
+fn test_health_check_unreachable_host_produces_useful_error() -> Result<(), Box<dyn Error>> {
+    // Port 19999 is not expected to have anything listening on localhost
+    run_fails([
+        "--host",
+        "localhost",
+        "--port",
+        "19999",
+        "health_check",
+        "cluster_wide_alarms",
+    ])
+    .stderr(output_includes(
+        "Encountered an error when performing an HTTP request:",
+    ));
+
+    Ok(())
+}

tests/integration/vhosts_delete_multiple_tests.rs

@@ -382,3 +382,27 @@ fn test_vhosts_delete_multiple_protects_deletion_protected_vhosts() -> Result<()
 
     Ok(())
 }
+
+#[test]
+fn test_vhosts_delete_multiple_quiet_produces_no_output() -> Result<(), Box<dyn Error>> {
+    let prefix = "rabbitmqadmin.test-vhosts-delete-multiple-quiet";
+
+    delete_vhosts_with_prefix(prefix).ok();
+
+    for i in 1..=3 {
+        let vh_name = format!("{}-{}", prefix, i);
+        run_succeeds(["vhosts", "declare", "--name", &vh_name]);
+    }
+
+    run_succeeds([
+        "--quiet",
+        "--non-interactive",
+        "vhosts",
+        "delete_multiple",
+        "--name-pattern",
+        prefix,
+    ])
+    .stdout(predicates::str::is_empty());
+
+    Ok(())
+}