From 03dd6ea868473fd1b03075af2a9ccc03f68f064a Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Wed, 22 Jul 2026 16:42:45 +0800 Subject: [PATCH 1/2] General tidy up --- .../rules/doc-summary-ends-with-period.yaml | 6 +++ services/storage/src/routes.rs | 6 +-- services/storage/tests/put.rs | 22 +++++----- services/ws-server/tests/tls.rs | 43 ++++++++++++++++++- 4 files changed, 62 insertions(+), 15 deletions(-) diff --git a/config/ast-grep/rules/doc-summary-ends-with-period.yaml b/config/ast-grep/rules/doc-summary-ends-with-period.yaml index bb474de..9bcc3f6 100644 --- a/config/ast-grep/rules/doc-summary-ends-with-period.yaml +++ b/config/ast-grep/rules/doc-summary-ends-with-period.yaml @@ -29,6 +29,11 @@ files: - libs/test-helpers/src/lib.rs - libs/ws-runner-common/tests/config.rs - services/modules/tests/api_modules.rs + - services/storage/src/lib.rs + - services/storage/src/routes.rs + - services/storage/src/tty_image.rs + - services/storage/tests/image_logging.rs + - services/storage/tests/put.rs - services/websockify/src/lib.rs - services/websockify/tests/relay.rs - services/ws-modules/except1/src/lib.rs @@ -38,6 +43,7 @@ files: - services/ws-server/src/main.rs - services/ws-server/src/net.rs - services/ws-server/tests/net.rs + - services/ws-server/tests/tls.rs - services/ws-wasi-runner/tests/modules.rs - services/ws-wasi-runner/tests/otel_propagation.rs - services/ws-wasi-runner/tests/vector_otlp_relay.rs diff --git a/services/storage/src/routes.rs b/services/storage/src/routes.rs index 15bbb35..067d793 100644 --- a/services/storage/src/routes.rs +++ b/services/storage/src/routes.rs @@ -116,10 +116,10 @@ where Ok(HttpResponse::Ok().finish()) } -/// Render a thumbnail of the image at `path` directly to stdout, so the operator actually *sees* what was -/// stored rather than just its filename and byte count. +/// Render a thumbnail of the image at `path` directly to stdout. /// -/// This bypasses `tracing` entirely and writes straight to the terminal: the escape sequences (ANSI +/// This lets the operator actually *see* what was stored, rather than just its filename and byte count. It +/// bypasses `tracing` entirely and writes straight to the terminal: the escape sequences (ANSI /// truecolor half-block art -- see the `tty_image` module) are tty-only presentation, not structured log /// data, and would otherwise get shipped to the OTLP log exporter as log-record noise. Decode/render /// failures (a corrupt upload, a non-terminal stdout) only cost tty visibility, not the request, so they're diff --git a/services/storage/tests/put.rs b/services/storage/tests/put.rs index 8362e20..436711b 100644 --- a/services/storage/tests/put.rs +++ b/services/storage/tests/put.rs @@ -55,13 +55,13 @@ async fn rejects_unknown_agent_with_404() { assert_eq!(resp.status(), StatusCode::NOT_FOUND); } -/// `actix-router` doesn't decode `%2F` in path captures (it would collide -/// with segment matching), so a regular `PUT /storage/a/b%2Fc` request -/// can't deliver a multi-component filename into the handler. Bypass the -/// router and invoke the handler directly with the parameters that the -/// filter expects to reject (`nested/path.txt` -> 2 components). This still -/// exercises the same `StorageError::InvalidFilename` -> 400 path that the -/// `ResponseError` impl wires up. +/// Invokes the handler directly, since a multi-component filename can't otherwise reach it. +/// +/// `actix-router` doesn't decode `%2F` in path captures (it would collide with segment matching), so a +/// regular `PUT /storage/a/b%2Fc` request can't deliver a multi-component filename into the handler. Bypass +/// the router and invoke the handler directly with the parameters that the filter expects to reject +/// (`nested/path.txt` -> 2 components). This still exercises the same `StorageError::InvalidFilename` -> 400 +/// path that the `ResponseError` impl wires up. #[actix_rt::test] async fn rejects_multi_component_filename_with_400() { let tmp = tempfile::tempdir().unwrap(); @@ -107,9 +107,11 @@ async fn writes_file_for_registered_agent() { assert_eq!(written, body); } -/// An image PUT under `cargo test` has no real terminal on stdout, so `viuer`'s decode-and-render step -/// (triggered by the `.png` extension) is expected to fail internally. The route must still store the file -/// and return 200 regardless -- tty display is a best-effort side effect, never a reason to fail the upload. +/// Storing an image must succeed even though tty rendering cannot, since it is a best-effort side effect. +/// +/// An image PUT under `cargo test` has no real terminal on stdout, so `tty_image::render`'s decode-and-render +/// step (triggered by the `.png` extension) is expected to fail internally. The route must still store the +/// file and return 200 regardless -- tty display is a best-effort side effect, never a reason to fail the upload. #[actix_rt::test] async fn stores_an_image_and_returns_200_even_though_tty_rendering_cannot_succeed_in_tests() { let tmp = tempfile::tempdir().unwrap(); diff --git a/services/ws-server/tests/tls.rs b/services/ws-server/tests/tls.rs index 3be61ea..21f5b23 100644 --- a/services/ws-server/tests/tls.rs +++ b/services/ws-server/tests/tls.rs @@ -16,17 +16,56 @@ fn generate_write_load_and_build_server_config() { cert.exists() && key.exists(), "generate_tls_certs must write both PEM files" ); + + // The freshly-written PEMs must load back into the exact same der bytes that were generated. + let (loaded_cert, loaded_key) = load_tls_certs(&cert, &key); + assert_eq!(gen_cert, loaded_cert, "reloaded cert der must match the generated der"); + assert_eq!( + gen_key.secret_der(), + loaded_key.secret_der(), + "reloaded key der must match the generated der" + ); + let from_generated = build_tls_server_config(gen_cert, gen_key); assert!( from_generated.alpn_protocols.is_empty(), "no ALPN protocols are configured by default" ); - // The freshly-written PEMs must load back into a der pair that also builds a valid config. - let (loaded_cert, loaded_key) = load_tls_certs(&cert, &key); let from_loaded = build_tls_server_config(loaded_cert, loaded_key); assert!( from_loaded.alpn_protocols.is_empty(), "reloaded config also has no ALPN protocols" ); } + +#[test] +#[should_panic(expected = "NotFound")] +fn load_tls_certs_missing_cert_file_panics() { + let dir = tempdir().unwrap(); + let cert = dir.path().join("missing-cert.pem"); + let key = dir.path().join("missing-key.pem"); + drop(load_tls_certs(&cert, &key)); +} + +#[test] +#[should_panic(expected = "NotFound")] +fn load_tls_certs_missing_key_file_panics() { + let dir = tempdir().unwrap(); + let cert = dir.path().join("cert.pem"); + let key = dir.path().join("key.pem"); + drop(generate_tls_certs(&cert, &key)); + fs_err::remove_file(&key).unwrap(); + + drop(load_tls_certs(&cert, &key)); +} + +#[test] +#[should_panic(expected = "InconsistentKeys")] +fn build_tls_server_config_with_mismatched_key_panics() { + let dir = tempdir().unwrap(); + let (cert_a, _key_a) = generate_tls_certs(&dir.path().join("a-cert.pem"), &dir.path().join("a-key.pem")); + let (_cert_b, key_b) = generate_tls_certs(&dir.path().join("b-cert.pem"), &dir.path().join("b-key.pem")); + + drop(build_tls_server_config(cert_a, key_b)); +} From 2a0f5d2c4363f59fc5b29406dfd838ba0cda639a Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Wed, 22 Jul 2026 19:23:12 +0800 Subject: [PATCH 2/2] more tests --- .mise/config.macos.toml | 13 +++ Cargo.lock | 1 + .../rules/doc-summary-ends-with-period.yaml | 5 ++ .../modules/tests/configure_missing_root.rs | 15 ++++ services/modules/tests/get_module_file.rs | 15 ++++ services/ws-server/tests/configure_app.rs | 41 +++++++++ services/ws-server/tests/health.rs | 22 +++++ services/ws/Cargo.toml | 1 + services/ws/tests/load_registry.rs | 90 +++++++++++++++++++ 9 files changed, 203 insertions(+) create mode 100644 services/modules/tests/configure_missing_root.rs create mode 100644 services/modules/tests/get_module_file.rs create mode 100644 services/ws-server/tests/configure_app.rs create mode 100644 services/ws-server/tests/health.rs create mode 100644 services/ws/tests/load_registry.rs diff --git a/.mise/config.macos.toml b/.mise/config.macos.toml index 78446d3..cfd0759 100644 --- a/.mise/config.macos.toml +++ b/.mise/config.macos.toml @@ -17,6 +17,17 @@ lld_flag = "-C link-arg=-fuse-ld={{ vars.conda_lld }}/bin/ld64.lld" # Xcode's libclang (the lib dir beside `xcrun`'s clang). # clang-sys doesn't always auto-find it on CI. The system libclang knows the SDK, so no bindgen args. mac_libclang = "{{ exec(command='dirname $(dirname $(xcrun --find clang))') }}/lib" +# Matches rustc's own per-arch default deployment target, so cc-rs compiles native C/asm to the same minimum. +# +# `rustc -Z unstable-options --print target-spec-json` describes aarch64-apple-darwin as "11.0+, Big Sur+" and +# x86_64-apple-darwin as "10.12+, Sierra+". cc-rs (the `cc` crate) reads this env var when compiling aws-lc- +# sys/tree-sitter's C and assembly sources, but without it clang stamps those .o files with the full installed +# SDK version instead -- e.g. `ld64.lld: .../libaws_lc_sys-*.rlib(*-jitterentropy-base.o) has version 26.5.0, +# which is newer than target minimum of 11.0.0`, one line per object file, surfaced as a build warning by +# rustc's `linker_messages` lint. Setting it here makes cc-rs target the same minimum rustc's own codegen +# already uses, so the .o files' stamped version and the linked binary's requested minimum agree and ld64.lld +# has nothing to warn about -- fixing the mismatch instead of just silencing the lint that reports it. +macosx_deployment_target = "{% if arch() == 'arm64' %}11.0{% else %}10.12{% endif %}" [env] # lld_flag points Apple's clang at ld64.lld (see [vars]). @@ -24,7 +35,9 @@ mac_libclang = "{{ exec(command='dirname $(dirname $(xcrun --find clang))') }}/l # loader finds them. Applies to every rustc call, including the `cargo:` source-build subprocess. CARGO_TARGET_AARCH64_APPLE_DARWIN_RUSTFLAGS = "{{ vars.lld_flag }} {{ vars.rpath_flag }} {{ vars.pylib_flag }}" CARGO_TARGET_X86_64_APPLE_DARWIN_RUSTFLAGS = "{{ vars.lld_flag }} {{ vars.rpath_flag }} {{ vars.pylib_flag }}" +# See the macosx_deployment_target comment in [vars] above. LIBCLANG_PATH = "{{ vars.mac_libclang }}" +MACOSX_DEPLOYMENT_TARGET = "{{ vars.macosx_deployment_target }}" # Pin SDKROOT so cc-rs and other build scripts find the macOS SDK. # This covers headers like stdlib.h regardless of which `cc` resolves first. With the `_.path` prepend below # putting Xcode CLT's /usr/bin/cc ahead of any brew gcc wrapper, the cc-rs invocation no longer inherits SDK diff --git a/Cargo.lock b/Cargo.lock index 4b788b6..fe5e3ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4682,6 +4682,7 @@ dependencies = [ "serde_default", "serde_json", "serde_yaml", + "tempfile", "tokio", "tracing", "uuid", diff --git a/config/ast-grep/rules/doc-summary-ends-with-period.yaml b/config/ast-grep/rules/doc-summary-ends-with-period.yaml index 9bcc3f6..17041a0 100644 --- a/config/ast-grep/rules/doc-summary-ends-with-period.yaml +++ b/config/ast-grep/rules/doc-summary-ends-with-period.yaml @@ -29,6 +29,8 @@ files: - libs/test-helpers/src/lib.rs - libs/ws-runner-common/tests/config.rs - services/modules/tests/api_modules.rs + - services/modules/tests/configure_missing_root.rs + - services/modules/tests/get_module_file.rs - services/storage/src/lib.rs - services/storage/src/routes.rs - services/storage/src/tty_image.rs @@ -42,6 +44,8 @@ files: - services/ws-server/src/lib.rs - services/ws-server/src/main.rs - services/ws-server/src/net.rs + - services/ws-server/tests/configure_app.rs + - services/ws-server/tests/health.rs - services/ws-server/tests/net.rs - services/ws-server/tests/tls.rs - services/ws-wasi-runner/tests/modules.rs @@ -50,6 +54,7 @@ files: - services/ws-web-runner/build.rs - services/ws-web-runner/tests/modules.rs - services/ws/tests/config.rs + - services/ws/tests/load_registry.rs - utilities/int-gen/src/error.rs - utilities/int-gen/src/lib.rs - utilities/int-gen/src/zig.rs diff --git a/services/modules/tests/configure_missing_root.rs b/services/modules/tests/configure_missing_root.rs new file mode 100644 index 0000000..bf64b05 --- /dev/null +++ b/services/modules/tests/configure_missing_root.rs @@ -0,0 +1,15 @@ +//! Covers `configure`'s fail-fast panic when `config.root` names a module that isn't among the +//! scanned `config.paths` -- a misconfiguration that must surface at startup, not as a silent 404. +#![cfg(test)] + +use actix_web::{App, test}; +use et_modules_service::{ModulesConfig, configure}; + +#[actix_rt::test] +#[should_panic(expected = "Root module 'nonexistent-root' not found")] +async fn configure_panics_when_root_module_is_missing() { + let tmp = tempfile::tempdir().unwrap(); + let config = ModulesConfig::new(vec![tmp.path().to_path_buf()], "nonexistent-root".to_string()); + + let _app = test::init_service(App::new().configure(|cfg| configure(cfg, &config))).await; +} diff --git a/services/modules/tests/get_module_file.rs b/services/modules/tests/get_module_file.rs new file mode 100644 index 0000000..6f8d8c3 --- /dev/null +++ b/services/modules/tests/get_module_file.rs @@ -0,0 +1,15 @@ +//! Covers `get_module_file`: a fake stub that exists only to host the utoipa route annotation for +//! `et-int-gen`, since the real `GET /modules/{name}/{path}` route is served by the `actix_files::Files` +//! mounts `configure` registers, not by Rust code. Pins its stub response so a future change to it is +//! deliberate, not accidental. +#![cfg(test)] +#![cfg(feature = "openapi-spec")] + +use actix_web::http::StatusCode; +use et_modules_service::routes::get_module_file; + +#[test] +fn returns_not_implemented() { + let resp = get_module_file(); + assert_eq!(resp.status(), StatusCode::NOT_IMPLEMENTED); +} diff --git a/services/ws-server/tests/configure_app.rs b/services/ws-server/tests/configure_app.rs new file mode 100644 index 0000000..66d2569 --- /dev/null +++ b/services/ws-server/tests/configure_app.rs @@ -0,0 +1,41 @@ +//! Covers `no_content` and `configure_app`'s route wiring: favicon, health, and delegation to the +//! sub-services `configure_app` chains together. +#![cfg(test)] + +use actix_web::http::StatusCode; +use actix_web::{App, test, web}; +use et_storage_service::StorageConfig; +use et_ws_server::config::Config; +use et_ws_server::{WsAgentRegistry, configure_app, no_content}; +use tempfile::tempdir; + +#[actix_rt::test] +async fn no_content_returns_204() { + let resp = no_content().await; + assert_eq!(resp.status(), StatusCode::NO_CONTENT); +} + +#[actix_rt::test] +async fn configure_app_wires_favicon_health_and_modules() { + let storage_dir = tempdir().unwrap(); + let mut config = Config::default(); + config.storage = StorageConfig::new(storage_dir.path().to_path_buf()); + + let registry = web::Data::new(WsAgentRegistry::default()); + let app = test::init_service(App::new().configure(|cfg| configure_app(cfg, registry, &config))).await; + + let favicon_req = test::TestRequest::get().uri("/favicon.ico").to_request(); + let favicon_resp = test::call_service(&app, favicon_req).await; + assert_eq!(favicon_resp.status(), StatusCode::NO_CONTENT); + + let health_req = test::TestRequest::get().uri("/health").to_request(); + let health_resp = test::call_service(&app, health_req).await; + assert!(health_resp.status().is_success()); + + // `/modules/` only resolves if `et_modules_service::configure` found and served the root module, + // which only runs if every configure call before it in `configure_app` (ws, storage, websockify) + // succeeded -- so a 200 here is proof the whole wiring chain ran. + let modules_req = test::TestRequest::get().uri("/modules/").to_request(); + let modules_resp = test::call_service(&app, modules_req).await; + assert_eq!(modules_resp.status(), StatusCode::OK); +} diff --git a/services/ws-server/tests/health.rs b/services/ws-server/tests/health.rs new file mode 100644 index 0000000..8f13583 --- /dev/null +++ b/services/ws-server/tests/health.rs @@ -0,0 +1,22 @@ +//! Covers the `/health` liveness-probe route: status code and response body shape. +#![cfg(test)] + +use actix_web::http::header; +use actix_web::{App, test, web}; +use et_ws_server::health; +use et_ws_server::routes::HealthResponse; + +#[actix_rt::test] +async fn health_returns_200_with_healthy_status() { + let app = test::init_service(App::new().route("/health", web::get().to(health))).await; + + let req = test::TestRequest::get().uri("/health").to_request(); + let resp = test::call_service(&app, req).await; + + assert!(resp.status().is_success()); + assert_eq!(resp.headers().get(header::CONTENT_TYPE).unwrap(), "application/json"); + + let body: HealthResponse = test::read_body_json(resp).await; + assert_eq!(body.status, "healthy"); + assert_eq!(body.service, "ws-server"); +} diff --git a/services/ws/Cargo.toml b/services/ws/Cargo.toml index 5c4c67e..5415438 100644 --- a/services/ws/Cargo.toml +++ b/services/ws/Cargo.toml @@ -30,6 +30,7 @@ uuid.workspace = true [dev-dependencies] et-test-helpers.workspace = true serde-env.workspace = true +tempfile.workspace = true [lints] workspace = true diff --git a/services/ws/tests/load_registry.rs b/services/ws/tests/load_registry.rs new file mode 100644 index 0000000..d42e6bc --- /dev/null +++ b/services/ws/tests/load_registry.rs @@ -0,0 +1,90 @@ +//! Covers `load_registry`: the missing-file empty-registry fallback, a save/load round trip through the +//! session-less `BareRecord` shape (state, last-known IP, pending direct messages survive; sessions don't), +//! and the malformed-YAML error path. +#![cfg(test)] + +use edge_toolkit::ws::{AgentConnectionState, AgentSummary, ConnectStatus}; +use edge_toolkit::ws_server::RegistryError; +use et_ws_service::{WsAgentRegistry, load_registry}; +use tempfile::tempdir; + +#[test] +fn missing_file_yields_an_empty_registry() { + let dir = tempdir().unwrap(); + let path = dir.path().join("no-such-registry.yaml"); + + let registry = load_registry(&path).unwrap(); + + assert!(registry.list_agents().is_empty()); +} + +#[test] +fn round_trips_state_ip_and_pending_messages_but_not_sessions() { + let dir = tempdir().unwrap(); + let path = dir.path().join("registry.yaml"); + + let registry = WsAgentRegistry::default(); + let (tx1, _rx1) = tokio::sync::mpsc::unbounded_channel(); + let (tx2, _rx2) = tokio::sync::mpsc::unbounded_channel(); + let (agent_1_id, agent_1_status) = registry.connect_agent(None, "agent-1".to_string(), "10.0.0.5", tx1); + let (agent_2_id, agent_2_status) = registry.connect_agent(None, "agent-2".to_string(), "10.0.0.6", tx2); + assert_eq!( + (agent_1_id.as_str(), agent_1_status), + ("agent-1", ConnectStatus::Assigned) + ); + assert_eq!( + (agent_2_id.as_str(), agent_2_status), + ("agent-2", ConnectStatus::Assigned) + ); + let queued = registry.queue_direct( + "msg-1".to_string(), + "agent-1", + "agent-2", + "2026-07-22T00:00:00Z".to_string(), + serde_json::json!({"hello": "world"}), + ); + assert!(queued.is_some(), "agent-2 must exist to receive the queued message"); + registry.mark_disconnected("agent-2"); + registry.save(&path).unwrap(); + + let loaded = load_registry(&path).unwrap(); + + assert_eq!( + loaded.list_agents(), + vec![ + AgentSummary::new( + "agent-1".to_string(), + AgentConnectionState::Connected, + Some("10.0.0.5".to_string()) + ), + AgentSummary::new( + "agent-2".to_string(), + AgentConnectionState::Disconnected, + Some("10.0.0.6".to_string()) + ), + ] + ); + assert!( + loaded.agent_session("agent-1").is_none(), + "sessions are never persisted, so a reloaded agent must have none even though it was Connected" + ); + + let pending = loaded.pending_messages_for("agent-2"); + assert_eq!(pending.len(), 1); + assert_eq!(pending[0].message_id, "msg-1"); + assert_eq!(pending[0].from_agent_id, "agent-1"); + assert_eq!(pending[0].message, serde_json::json!({"hello": "world"})); +} + +#[test] +fn malformed_yaml_is_a_registry_error() { + let dir = tempdir().unwrap(); + let path = dir.path().join("registry.yaml"); + fs_err::write(&path, "- this\n- is\n- a list, not a map of agents\n").unwrap(); + + match load_registry(&path) { + Err(RegistryError::Yaml(_)) => {} + Err(other) => panic!("expected RegistryError::Yaml, got {other:?}"), + Ok(_) => panic!("expected an error for malformed YAML"), + } +}