diff --git a/crates/edgezero-adapter/src/cli_support.rs b/crates/edgezero-adapter/src/cli_support.rs index 94ace71..a5e664d 100644 --- a/crates/edgezero-adapter/src/cli_support.rs +++ b/crates/edgezero-adapter/src/cli_support.rs @@ -223,4 +223,23 @@ mod tests { let found = find_manifest_upwards(&child, "demo.toml").expect("manifest"); assert_eq!(found, root.join("demo.toml")); } + + #[test] + fn run_native_cli_missing_program_surfaces_install_hint() { + let err = run_native_cli("edgezero-no-such-program-xyz", &[], "install the thing") + .expect_err("missing program must error"); + assert!(err.contains("install the thing"), "got: {err}"); + } + + #[test] + fn run_native_cli_nonzero_exit_is_error() { + // `false` exits non-zero on every supported CI host (unix/macOS). + let err = run_native_cli("false", &[], "hint").expect_err("non-zero exit must error"); + // Pin the exit-status branch specifically — `!is_empty()` would + // also pass for the wrong (not-found / spawn) branch. + assert!( + err.contains("exited with status"), + "expected the exit-status branch, got: {err}" + ); + } } diff --git a/crates/edgezero-adapter/src/registry.rs b/crates/edgezero-adapter/src/registry.rs index 6a24ce8..fcc5bfa 100644 --- a/crates/edgezero-adapter/src/registry.rs +++ b/crates/edgezero-adapter/src/registry.rs @@ -626,4 +626,67 @@ mod tests { "expected Unsupported variant from default local impl" ); } + + #[test] + fn push_context_new_is_prod_with_no_paths() { + let ctx = AdapterPushContext::new(); + assert!(!ctx.local); + assert_eq!(ctx.manifest_adapter_deploy_cmd, None); + assert_eq!(ctx.runtime_config_path, None); + } + + #[test] + fn push_context_builders_set_each_field() { + let path = Path::new("runtime-config.toml"); + let ctx = AdapterPushContext::new() + .with_local(true) + .with_manifest_adapter_deploy_cmd("spin cloud deploy") + .with_runtime_config_path(path); + assert!(ctx.local); + assert_eq!(ctx.manifest_adapter_deploy_cmd, Some("spin cloud deploy")); + assert_eq!(ctx.runtime_config_path, Some(path)); + } + + #[test] + fn default_validation_and_kind_methods_are_noops() { + // `FIRST` overrides only `execute` + `name`, so every other + // method here exercises the trait's default impl. + assert!(FIRST.merged_id_kinds().is_empty()); + assert!(FIRST.single_store_kinds().is_empty()); + assert_eq!( + FIRST.validate_adapter_manifest(Path::new("/tmp"), None, None), + Ok(()) + ); + assert_eq!( + FIRST.validate_app_config_keys(&["greeting", "service.timeout_ms"]), + Ok(()) + ); + let entry = TypedSecretEntry::new("vault", "api_token", "demo_api_token"); + assert_eq!(FIRST.validate_typed_secrets(&[entry]), Ok(())); + } + + #[test] + fn default_push_config_entries_error_names_the_adapter() { + // Unlike the no-op defaults above, the push defaults RETURN AN + // ERROR that interpolates the adapter name — load-bearing for CLI + // UX, so assert the message content, not just `is_err`. + let root = Path::new("/tmp"); + let store = ResolvedStoreId::from_logical("app_config"); + let ctx = AdapterPushContext::new(); + + let err = FIRST + .push_config_entries(root, None, None, &store, &[], &ctx, false) + .expect_err("default push must be unsupported"); + assert!(err.contains("dummy"), "should name the adapter: {err}"); + assert!(err.contains("does not implement"), "msg: {err}"); + + let local_err = FIRST + .push_config_entries_local(root, None, None, &store, &[], &ctx, false) + .expect_err("default local push must be unsupported"); + assert!( + local_err.contains("dummy"), + "should name the adapter: {local_err}" + ); + assert!(local_err.contains("--local"), "msg: {local_err}"); + } } diff --git a/crates/edgezero-cli/src/bin/check_no_nested_app_config.rs b/crates/edgezero-cli/src/bin/check_no_nested_app_config.rs index 1aa71c4..0f9788b 100644 --- a/crates/edgezero-cli/src/bin/check_no_nested_app_config.rs +++ b/crates/edgezero-cli/src/bin/check_no_nested_app_config.rs @@ -351,3 +351,227 @@ fn main() { println!("check_no_nested_app_config: OK"); } + +#[cfg(test)] +mod tests { + use super::*; + + fn known(names: &[&str]) -> HashSet { + names.iter().map(|name| String::from(*name)).collect() + } + + fn ty(src: &str) -> Type { + syn::parse_str(src).expect("type parse") + } + + #[test] + fn struct_derives_app_config_detects_path_suffixed_derive() { + let item: syn::ItemStruct = + syn::parse_str("#[derive(Debug, edgezero_core::AppConfig)] struct C { x: u8 }") + .expect("struct parse"); + assert!(struct_derives_app_config(&item)); + } + + #[test] + fn struct_derives_app_config_false_without_it() { + let item: syn::ItemStruct = + syn::parse_str("#[derive(Debug)] struct C { x: u8 }").expect("struct parse"); + assert!(!struct_derives_app_config(&item)); + } + + #[test] + fn type_contains_app_config_unwraps_nested_wrappers() { + let set = known(&["ChildConfig"]); + assert_eq!( + type_contains_app_config_struct(&ty("ChildConfig"), &set).as_deref(), + Some("ChildConfig") + ); + assert_eq!( + type_contains_app_config_struct(&ty("Option>>"), &set).as_deref(), + Some("ChildConfig") + ); + } + + #[test] + fn type_contains_app_config_none_for_unrelated_types() { + let set = known(&["ChildConfig"]); + assert_eq!(type_contains_app_config_struct(&ty("String"), &set), None); + assert_eq!( + type_contains_app_config_struct(&ty("Vec"), &set), + None + ); + } + + #[test] + fn type_contains_app_config_unwraps_every_container_arm() { + // Each of these exercises a distinct arm of the recursive match + // (Rc/Arc path-wrappers, array, slice, reference, paren, tuple in + // either position, and cross-arm combinations). A regression that + // deleted any arm — e.g. stops detecting `(ChildConfig, u8)` — + // would defeat the CI gate and ship green without these. + let set = known(&["ChildConfig"]); + for src in [ + "Rc", + "Arc", + "[ChildConfig; 4]", // Type::Array + "[ChildConfig]", // Type::Slice + "&ChildConfig", // Type::Reference + "(ChildConfig)", // Type::Paren + "(ChildConfig, u8)", // tuple, first position + "(u8, ChildConfig)", // tuple, second position + "Vec<(String, Arc)>", // path -> tuple -> path + "Option<[Box; 2]>", // path -> array -> path + ] { + assert_eq!( + type_contains_app_config_struct(&ty(src), &set).as_deref(), + Some("ChildConfig"), + "should detect ChildConfig in `{src}`" + ); + } + } + + #[test] + fn type_contains_app_config_negatives_including_unwrapped_generic() { + let set = known(&["ChildConfig"]); + for src in ["String", "Vec", "(u8, String)", "[u8; 4]"] { + assert_eq!( + type_contains_app_config_struct(&ty(src), &set), + None, + "should NOT detect ChildConfig in `{src}`" + ); + } + // Documented limitation: arbitrary generics (not the whitelisted + // transparent wrappers) are NOT unwrapped, so an AppConfig buried + // in a `HashMap` value is intentionally not flagged. + assert_eq!( + type_contains_app_config_struct(&ty("HashMap"), &set), + None + ); + } + + #[test] + fn struct_derives_app_config_handles_bare_ident_and_multi_derive() { + let bare: syn::ItemStruct = + syn::parse_str("#[derive(AppConfig)] struct C { x: u8 }").expect("parse"); + assert!(struct_derives_app_config(&bare), "bare `AppConfig` ident"); + + let multi: syn::ItemStruct = + syn::parse_str("#[derive(Debug)] #[derive(Clone, AppConfig)] struct C { x: u8 }") + .expect("parse"); + assert!( + struct_derives_app_config(&multi), + "AppConfig in a second derive attribute" + ); + } + + #[test] + fn struct_derives_app_config_rejects_suffix_collision_and_non_derive() { + let suffix: syn::ItemStruct = + syn::parse_str("#[derive(AppConfigExt)] struct C { x: u8 }").expect("parse"); + assert!( + !struct_derives_app_config(&suffix), + "must be an exact ident match, not a substring" + ); + + let non_derive: syn::ItemStruct = + syn::parse_str("#[repr(C)] struct C { x: u8 }").expect("parse"); + assert!(!struct_derives_app_config(&non_derive)); + } + + // --- end-to-end detection: the two-pass visitor pipeline --- + // Drives the collectors directly (no file IO) so the binary's actual + // job — scan source, flag a real nested-AppConfig, count it — is + // proven, not just the leaf helpers. + + fn violations_in(src: &str) -> usize { + let file: syn::File = syn::parse_str(src).expect("source parses"); + let mut collector = AppConfigStructCollector::new(); + collector.visit_file(&file); + let mut visitor = + NestedAppConfigVisitor::new(Path::new("test.rs"), &collector.app_config_structs); + visitor.visit_file(&file); + visitor.violations + } + + #[test] + fn detects_nested_app_config_through_wrapper() { + let src = " + #[derive(edgezero_core::AppConfig)] + struct Inner { secret: String } + #[derive(edgezero_core::AppConfig)] + struct Outer { inner: Option } + "; + assert_eq!(violations_in(src), 1); + } + + #[test] + fn no_violation_when_field_type_is_not_app_config() { + let src = " + struct Plain { x: u8 } + #[derive(edgezero_core::AppConfig)] + struct Outer { plain: Plain } + "; + assert_eq!(violations_in(src), 0); + } + + #[test] + fn no_violation_when_outer_struct_is_not_app_config() { + let src = " + #[derive(edgezero_core::AppConfig)] + struct Inner { secret: String } + struct Outer { inner: Inner } + "; + assert_eq!(violations_in(src), 0); + } + + #[test] + fn counts_multiple_nested_violations() { + let src = " + #[derive(edgezero_core::AppConfig)] + struct Inner { secret: String } + #[derive(edgezero_core::AppConfig)] + struct Outer { a: Inner, b: Vec } + "; + assert_eq!(violations_in(src), 2); + } + + #[test] + fn detects_violation_regardless_of_definition_order() { + // Inner defined AFTER Outer: the two-pass (collect-all-then-check) + // design must still catch it; a naive single pass would miss it. + let src = " + #[derive(edgezero_core::AppConfig)] + struct Outer { inner: Inner } + #[derive(edgezero_core::AppConfig)] + struct Inner { secret: String } + "; + assert_eq!(violations_in(src), 1); + } + + #[test] + fn no_false_positive_from_appconfig_in_doc_comment() { + // The walk inspects field TYPES only — `AppConfig` appearing in a + // doc comment is not a violation (the module's headline property). + let src = " + #[derive(edgezero_core::AppConfig)] + struct Outer { + /// mentions AppConfig in prose, but the field type is String + note: String, + } + "; + assert_eq!(violations_in(src), 0); + } + + #[test] + fn detects_nested_in_tuple_struct_unnamed_field() { + // Exercises the `` field-name branch + Span::call_site + // fallback in the visitor. + let src = " + #[derive(edgezero_core::AppConfig)] + struct Inner { secret: String } + #[derive(edgezero_core::AppConfig)] + struct Outer(Inner); + "; + assert_eq!(violations_in(src), 1); + } +} diff --git a/crates/edgezero-core/src/compression.rs b/crates/edgezero-core/src/compression.rs index ee4bf1a..7ad9132 100644 --- a/crates/edgezero-core/src/compression.rs +++ b/crates/edgezero-core/src/compression.rs @@ -107,4 +107,26 @@ mod tests { assert_eq!(decoded, b"hello brotli"); } + + #[test] + fn decode_gzip_stream_surfaces_error_on_invalid_input() { + let garbage = b"this is definitely not a gzip member".to_vec(); + let stream = stream::iter(vec![Ok::, io::Error>(garbage)]); + let result = + block_on(async { decode_gzip_stream(stream).try_collect::>().await }); + assert!(result.is_err(), "invalid gzip must decode to an error"); + } + + #[test] + fn decode_brotli_stream_surfaces_error_on_invalid_input() { + // A high-bit-set lead byte is not a valid brotli stream prefix. + let garbage = vec![0xFF_u8; 64]; + let stream = stream::iter(vec![Ok::, io::Error>(garbage)]); + let result = block_on(async { + decode_brotli_stream(stream) + .try_collect::>() + .await + }); + assert!(result.is_err(), "invalid brotli must decode to an error"); + } } diff --git a/crates/edgezero-core/src/handler.rs b/crates/edgezero-core/src/handler.rs index 17fd483..f47af20 100644 --- a/crates/edgezero-core/src/handler.rs +++ b/crates/edgezero-core/src/handler.rs @@ -38,3 +38,49 @@ where Arc::new(self) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::body::Body; + use crate::http::{request_builder, Method, StatusCode}; + use crate::params::PathParams; + use futures::executor::block_on; + + fn ctx() -> RequestContext { + let request = request_builder() + .method(Method::GET) + .uri("/") + .body(Body::empty()) + .expect("request"); + RequestContext::new(request, PathParams::default()) + } + + #[test] + fn into_handler_wraps_closure_and_call_runs_it() { + async fn ok(_ctx: RequestContext) -> Result<&'static str, EdgeError> { + Ok("hi") + } + let handler = ok.into_handler(); + let response = block_on(handler.call(ctx())).expect("ok response"); + assert_eq!(response.status(), StatusCode::OK); + // Prove the closure's return value actually flowed through + // `into_response` — not just that *some* default-OK response came + // back. A bridge that dropped the body would still be status 200. + assert_eq!(response.body().as_bytes(), Some(&b"hi"[..])); + } + + #[test] + fn call_propagates_handler_error() { + async fn boom(_ctx: RequestContext) -> Result<&'static str, EdgeError> { + // `EdgeError::internal` takes `E: Into`; a bare + // `&str` does not satisfy that bound, so wrap with `anyhow!`. + Err(EdgeError::internal(anyhow::anyhow!("boom"))) + } + let handler = boom.into_handler(); + let Err(error) = block_on(handler.call(ctx())) else { + panic!("expected error"); + }; + assert_eq!(error.status(), StatusCode::INTERNAL_SERVER_ERROR); + } +} diff --git a/crates/edgezero-macros/src/app.rs b/crates/edgezero-macros/src/app.rs index 4858d5a..3d2474f 100644 --- a/crates/edgezero-macros/src/app.rs +++ b/crates/edgezero-macros/src/app.rs @@ -251,7 +251,7 @@ fn route_for_method(method: &str, path: &LitStr, handler: &syn::ExprPath) -> Tok #[cfg(test)] mod tests { - use super::parse_handler_path; + use super::{build_route_tokens, parse_handler_path, Manifest}; #[test] fn parse_handler_path_accepts_absolute_crate_path() { @@ -273,4 +273,93 @@ mod tests { "error message should echo the offending input, got: {err}" ); } + + #[test] + fn build_route_tokens_propagates_invalid_handler_path() { + // A manifest that parses cleanly but declares a handler whose + // Rust path is invalid: `build_route_tokens` must surface the + // `parse_handler_path` error rather than panic or silently skip. + let manifest: Manifest = toml::from_str( + r#" +[app] +name = "demo" +entry = "crates/demo-core" + +[[triggers.http]] +path = "/" +methods = ["GET"] +handler = "1bad::handler" +"#, + ) + .expect("manifest TOML should parse"); + let err = build_route_tokens(&manifest).expect_err("invalid handler must error"); + assert!( + err.contains("invalid handler path"), + "error should propagate from parse_handler_path, got: {err}" + ); + } + + #[test] + fn build_route_tokens_emits_one_token_per_method() { + let manifest: Manifest = toml::from_str( + r#" +[app] +name = "demo" +entry = "crates/demo-core" + +[[triggers.http]] +path = "/" +methods = ["GET", "POST", "PUT"] +handler = "crate::handlers::root" +"#, + ) + .expect("manifest TOML should parse"); + let tokens = build_route_tokens(&manifest).expect("valid manifest builds routes"); + // One route token per (trigger × method): 1 trigger × 3 methods. + assert_eq!(tokens.len(), 3); + } + + #[test] + fn build_route_tokens_skips_trigger_without_handler() { + let manifest: Manifest = toml::from_str( + r#" +[app] +name = "demo" +entry = "crates/demo-core" + +[[triggers.http]] +path = "/has" +methods = ["GET"] +handler = "crate::handlers::root" + +[[triggers.http]] +path = "/none" +methods = ["GET"] +"#, + ) + .expect("manifest TOML should parse"); + let tokens = build_route_tokens(&manifest).expect("builds"); + // The handler-less trigger hits the `else { continue }` and + // contributes no routes. + assert_eq!(tokens.len(), 1); + } + + #[test] + fn build_route_tokens_defaults_to_get_when_methods_absent() { + let manifest: Manifest = toml::from_str( + r#" +[app] +name = "demo" +entry = "crates/demo-core" + +[[triggers.http]] +path = "/" +handler = "crate::handlers::root" +"#, + ) + .expect("manifest TOML should parse"); + let tokens = build_route_tokens(&manifest).expect("builds"); + // No `methods` key → `Trigger::methods()` defaults to `["GET"]`. + assert_eq!(tokens.len(), 1); + } } diff --git a/docs/superpowers/specs/2026-06-16-test-coverage-gap.md b/docs/superpowers/specs/2026-06-16-test-coverage-gap.md index 5a086f6..2841120 100644 --- a/docs/superpowers/specs/2026-06-16-test-coverage-gap.md +++ b/docs/superpowers/specs/2026-06-16-test-coverage-gap.md @@ -6,7 +6,7 @@ > checkbox (`- [ ]`) syntax for tracking. **Date:** 2026-06-16 -**Status:** v1 — Draft, revised through reviewer round 6 +**Status:** v1 — Draft, revised through round 9 (self-review hardening) **Author:** (TBD) **Branch under assessment:** `feature/extensible-cli` @@ -38,7 +38,7 @@ over-reported gaps. Three claims were checked and **rejected**: | Rejected claim | Reality (verified) | | --------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `edgezero-cli` `config push` is untested | `config.rs` has a full push suite — `raw_push_axum_writes_local_config_json`, `raw_push_dry_run_does_not_write`, `raw_push_flattens_nested_tables_into_dotted_keys`, `raw_push_json_encodes_arrays`, store-id resolution + error cases (`config.rs:1600-1790`). | +| `edgezero-cli` `config push` is untested | `config.rs` has a full push suite — `typed_push_writes_blob_envelope_to_local_config_file`, `typed_push_dry_run_does_not_write`, `typed_push_envelope_sha_matches_hand_computed_hash`, `typed_push_runs_validator_and_errors_on_bad_config`, `run_config_push_is_stub_pointer_in_bundled_binary`, plus adapter/strict checks (`config.rs:2366` onward). | | `core/compression.rs` has "only 2 tests, major gap" | Two tests is the full happy-path matrix (gzip + brotli). The real, narrow gap is the **error path**, not presence. | | `axum/service.rs` (680 LOC) has zero tests | It has `#[tokio::test]` cases (`service.rs:451,528,558`) the marker scan undercounted because of the attribute form. Well covered. | @@ -55,8 +55,11 @@ count); the CLI, macros, and adapter CLI surfaces are all substantially tested. The genuine gaps fall into three buckets: -1. **One** host-testable source file with real logic and _zero_ - coverage: `edgezero-core/src/handler.rs`. +1. **Two** host-testable source files with real logic and _zero_ + coverage: `edgezero-core/src/handler.rs` and the pure `syn` helpers in + `edgezero-cli/src/bin/check_no_nested_app_config.rs` (the + nested-`AppConfig` CI checker; host-compilable behind the + `nested-app-config-check` feature). 2. A small set of **error/edge-path depth gaps** in modules that already have happy-path tests. 3. The **runtime-bound portions of adapter platform code** (Spin / @@ -64,8 +67,13 @@ buckets: Fastly logger init) that **cannot be host unit-tested**. Note the pure helper/conversion paths in these crates _are_ already host- and contract-tested (see the Tier-3 split in §4); only the SDK/runtime - paths remain. That runtime-bound code only compiles for `wasm32-*` - and/or behind platform SDKs. This is by + paths remain. The gating differs by adapter: **Spin and Cloudflare** + modules are `#[cfg(all(feature=…, target_arch="wasm32"))]` so they + never build on the host at all; **Fastly** modules are + `#[cfg(feature="fastly")]` only (no `target_arch` gate) — they DO + compile on the host, but their live paths call the Fastly compute SDK + that isn't meaningfully exercisable off-platform. Either way the live + paths are out of host-unit-test scope. This is by design per CLAUDE.md ("Don't write tests that require a network connection or platform credentials") and is the scope of each adapter's contract and/or integration tests, not host unit tests. @@ -104,6 +112,7 @@ module at all**. | Gap | File | Why it matters | | ------------------------------------------------ | ------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `DynHandler::call` + `IntoHandler::into_handler` | `crates/edgezero-core/src/handler.rs:9-40` | The closure→handler bridge and the `fut.await?.into_response()` error-propagation line (`handler.rs:22`) are the spine of every route. No test exercises them directly today. Pure host logic — trivially testable with `block_on`. | +| `struct_derives_app_config` + `type_contains_app_config_struct` | `crates/edgezero-cli/src/bin/check_no_nested_app_config.rs:103,196` | The recursive `syn` AST walk that the nested-`AppConfig` CI gate depends on (unwraps `Option`/`Vec`/`Box`/`Rc`/`Arc` to any depth). 353 LOC, zero tests. Host-compilable behind the `nested-app-config-check` feature; testable with `syn::parse_str`. Because it's a `bin`, tests must be an inline `#[cfg(test)]` module (free fns aren't importable from `tests/`). | ### Tier 2 — host-testable depth gaps (should close) @@ -113,23 +122,28 @@ module at all**. | `run_native_cli` error paths | `crates/edgezero-adapter/src/cli_support.rs:82-99` | Happy path is covered; the `ErrorKind::NotFound` → install-hint branch (`:84-92`) and the non-zero-exit branch (`:93-98`) are not. | | Decompression error path | `crates/edgezero-core/src/compression.rs:15-60` | `decode_gzip_stream` / `decode_brotli_stream` test only valid input; malformed input → `Err` is unexercised. | -### Tier 2 backlog — verify-then-test (lower confidence) +### Tier 2 backlog — verify-then-test (RESOLVED in this PR) -These are plausible depth gaps surfaced during assessment but **not yet -verified** to the point of writing assertions. Each task is: confirm the -code path exists as described, then add a test mirroring the module's -existing idiom. Do **not** write a test against an assumed API. +These were plausible depth gaps surfaced during assessment but not yet +verified to the point of writing assertions. Both have now been verified +against the code and closed: -| Candidate gap | File | Verify first | -| ------------------------------- | ----------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `Adapter` trait default methods | `crates/edgezero-adapter/src/registry.rs:211,375,401` | `merged_id_kinds` (default `&[]`), `validate_app_config_keys`/`validate_typed_secrets` (default `Ok`) — needs a minimal `Adapter` test-double overriding the required `execute` **and** `name` (both are required; reuse the existing `TestAdapter` at `registry.rs:459` rather than writing a new double). | -| `app!` macro error handling | `crates/edgezero-macros/src/app.rs` | Only 2 codegen tests; missing-file / invalid-TOML diagnostics may be untested — confirm and add a trybuild compile-fail fixture if so. | +| Candidate gap | File | Resolution | +| ------------------------------- | ----------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Adapter` trait default methods | `crates/edgezero-adapter/src/registry.rs` | **Closed.** Verified the no-op defaults (`merged_id_kinds`/`single_store_kinds` → `&[]`; `validate_adapter_manifest`/`validate_app_config_keys`/`validate_typed_secrets` → `Ok`) and covered them via the existing `FIRST` `TestAdapter` (inherits all defaults) — test `default_validation_and_kind_methods_are_noops`. | +| `app!` macro error handling | `crates/edgezero-macros/src/app.rs` | **Closed (partial, by design).** The three `expand_app` file-level diagnostics (read/parse/validate) embed the **absolute** manifest path in their messages, so a trybuild `.stderr` fixture would be machine-dependent and flaky — deliberately NOT added. The cleanly-testable seam is `build_route_tokens`, which had no direct test; covered its error propagation via `build_route_tokens_propagates_invalid_handler_path` (toml-built `Manifest` with a bad handler). | -### Tier 3 — wasm-gated platform code (out of host-unit-test scope) +### Tier 3 — runtime-bound platform code (out of host-unit-test scope) -Cannot be closed with host unit tests; see §8 for strategy. **Note the -split**: the pure/helper conversion paths in these crates are _already_ -host- or wasm-contract-tested (see the per-row citations) — only the +Cannot be closed with host unit tests; see §8 for strategy. **Gating is +not uniform:** Spin and Cloudflare modules are +`#[cfg(all(feature=…, target_arch="wasm32"))]` (never built on host); +Fastly modules are `#[cfg(feature="fastly")]` only and DO compile on the +host, but their live paths call the Fastly compute SDK — so "out of +host-unit-test scope" is the shared conclusion, via different routes. +**Note the split**: the pure/helper conversion paths in these crates are +_already_ host- or wasm-contract-tested (see the per-row citations) — +only the SDK/runtime-bound paths remain. | Area | Already host/contract-tested | Genuinely runtime-bound (Tier 3) | @@ -220,12 +234,11 @@ mod tests { Err(EdgeError::internal(anyhow::anyhow!("boom"))) } let handler = boom.into_handler(); - // `block_on(...).expect_err(...)` would also compile (`Body` does - // impl `Debug`), but `match` reads clearer for panic-on-`Ok` and - // avoids depending on that impl. - let error = match block_on(handler.call(ctx())) { - Ok(_) => panic!("expected error"), - Err(error) => error, + // `let...else` (clippy `manual_let_else` requires it over `match` + // here). `expect_err` would also compile — `Body` does impl + // `Debug` — but this avoids depending on that. + let Err(error) = block_on(handler.call(ctx())) else { + panic!("expected error"); }; assert_eq!(error.status(), StatusCode::INTERNAL_SERVER_ERROR); } @@ -246,6 +259,81 @@ git add crates/edgezero-core/src/handler.rs git commit -m "test(core): cover DynHandler::call and IntoHandler bridge" ``` +### Task 1b: Cover the nested-`AppConfig` checker helpers (Tier 1) + +**Files:** + +- Modify/Test: `crates/edgezero-cli/src/bin/check_no_nested_app_config.rs` (append an inline test module — it's a `bin`, so a `tests/` file can't reach its free fns) + +- [ ] **Step 1: Add characterization tests** for the two pure `syn` + helpers (the recursive-unwrap logic is the whole reason this CI gate + exists, per the blob-app-config spec §3.3.1.2): + +```rust +#[cfg(test)] +mod tests { + use super::*; + use std::collections::HashSet; + + fn known(names: &[&str]) -> HashSet { + names.iter().map(|name| String::from(*name)).collect() + } + + fn ty(src: &str) -> syn::Type { + syn::parse_str(src).expect("type parse") + } + + #[test] + fn struct_derives_app_config_detects_path_suffixed_derive() { + let item: syn::ItemStruct = + syn::parse_str("#[derive(Debug, edgezero_core::AppConfig)] struct C { x: u8 }") + .expect("struct parse"); + assert!(struct_derives_app_config(&item)); + } + + #[test] + fn struct_derives_app_config_false_without_it() { + let item: syn::ItemStruct = + syn::parse_str("#[derive(Debug)] struct C { x: u8 }").expect("struct parse"); + assert!(!struct_derives_app_config(&item)); + } + + #[test] + fn type_contains_app_config_unwraps_nested_wrappers() { + let set = known(&["ChildConfig"]); + assert_eq!( + type_contains_app_config_struct(&ty("ChildConfig"), &set).as_deref(), + Some("ChildConfig") + ); + assert_eq!( + type_contains_app_config_struct(&ty("Option>>"), &set).as_deref(), + Some("ChildConfig") + ); + } + + #[test] + fn type_contains_app_config_none_for_unrelated_types() { + let set = known(&["ChildConfig"]); + assert_eq!(type_contains_app_config_struct(&ty("String"), &set), None); + assert_eq!(type_contains_app_config_struct(&ty("Vec"), &set), None); + } +} +``` + +- [ ] **Step 2: Run** (the bin only builds with its feature) + +Run: `cargo test -p edgezero-cli --features nested-app-config-check --bin check_no_nested_app_config` +Expected: all four tests PASS. (`use super::*` brings the helpers plus +the `syn` / `Type` / `HashSet` imports already at the top of the bin into +scope.) + +- [ ] **Step 3: Commit** + +```bash +git add crates/edgezero-cli/src/bin/check_no_nested_app_config.rs +git commit -m "test(cli): cover nested-AppConfig checker syn helpers" +``` + ### Task 2: Cover `AdapterPushContext` builder (Tier 2) **Files:** @@ -319,7 +407,10 @@ git commit -m "test(adapter): cover AdapterPushContext builder" - [ ] **Step 2: Run** -Run: `cargo test -p edgezero-adapter run_native_cli` +Run: `cargo test -p edgezero-adapter --features cli run_native_cli` — the +`cli_support` module is `#[cfg(feature = "cli")]`, so a scoped `-p` run +needs the feature explicitly (the workspace test enables it via feature +unification). Expected: PASS. - [ ] **Step 3: Commit** @@ -352,7 +443,7 @@ git commit -m "test(adapter): cover run_native_cli not-found and non-zero-exit p #[test] fn decode_brotli_stream_surfaces_error_on_invalid_input() { // A high-bit-set lead byte is not a valid brotli stream prefix. - let garbage = vec![0xFFu8; 64]; + let garbage = vec![0xFF_u8; 64]; let stream = stream::iter(vec![Ok::, io::Error>(garbage)]); let result = block_on(async { decode_brotli_stream(stream).try_collect::>().await @@ -375,12 +466,17 @@ git add crates/edgezero-core/src/compression.rs git commit -m "test(core): cover gzip and brotli decode error paths" ``` -### Task 5: Tier-2 backlog (verify-then-test) +### Task 5: Tier-2 backlog (verify-then-test) — DONE -For each row in §4 "Tier 2 backlog": first read the cited code to -confirm the path exists as described; if it does, add one test mirroring -the module's existing idiom and commit; if it does not, note it in the -v2 changelog and drop it. **Do not** write a test against an assumed API. +Both backlog rows were verified against the code and closed in this PR +(see §4 "Tier 2 backlog" for the resolutions): + +- **`Adapter` trait defaults** → `default_validation_and_kind_methods_are_noops` + in `crates/edgezero-adapter/src/registry.rs`. +- **`app!` macro error handling** → `build_route_tokens_propagates_invalid_handler_path` + in `crates/edgezero-macros/src/app.rs`. The `expand_app` file-level + diagnostics were verified untested but deliberately left uncovered (their + messages embed absolute paths, making a trybuild fixture flaky). --- @@ -429,15 +525,16 @@ above (not the already-covered conversions). ## 9. Acceptance gate -- [ ] Tasks 1–4 land; all five project CI gates (CLAUDE.md) are green: +- [x] Tasks 1, 1b, 2–4 land; all five project CI gates (CLAUDE.md) are green: 1. `cargo fmt --all -- --check` 2. `cargo clippy --workspace --all-targets --all-features -- -D warnings` 3. `cargo test --workspace --all-targets` 4. `cargo check --workspace --all-targets --features "fastly cloudflare spin"` 5. `cargo check -p edgezero-adapter-spin --target wasm32-wasip2 --features spin` -- [ ] `handler.rs` no longer appears in the `cfg(test)=0` host-testable +- [x] Neither `handler.rs` nor `check_no_nested_app_config.rs` appears in + the `cfg(test)=0` host-testable file list. -- [ ] Tier-2 backlog (Task 5) rows are each either closed with a test or +- [x] Tier-2 backlog (Task 5) rows are each either closed with a test or recorded as "not a gap" in the v2 changelog. - [ ] Tier 3 disposition (Q1) is decided and recorded. @@ -533,3 +630,41 @@ scope. (3) Added Cloudflare's response-conversion wasm contract coverage (`cloudflare/tests/contract.rs:201`) to the Tier-3 table for completeness, and annotated each conversion cite as host vs. wasm contract. Also softened the §4 intro to "host- or wasm-contract-tested". + +**Deep review (round 7):** multi-agent review vs. committed branch code. +Five verified fixes: (1) Task 1 compile blocker — +`EdgeError::internal("boom")` → `EdgeError::internal(anyhow::anyhow!("boom"))` +(`internal` needs `E: Into`). (2) Missed Tier-1 gap — +`check_no_nested_app_config.rs` (`syn` helpers, 0 tests, host-testable) +added to §2/§4 + Task 1b. (3) Fastly is `#[cfg(feature="fastly")]` only, +not `wasm32`-gated; §2/§4 reworded. (4) §1 config-push cited nonexistent +`raw_push_*` tests → corrected to real `typed_push_*` at `config.rs:2366+`. +(5) `Body` does impl `Debug` (`body.rs:160`); Task 1's `match` kept as +style, false rationale removed. Prettier passes; §9 matches the five +CLAUDE.md gates. + +**Tier-2 backlog closed (round 8):** Task 5's two verify-then-test rows +were resolved in-PR. (1) `Adapter` trait defaults — +`default_validation_and_kind_methods_are_noops` exercises the no-op +defaults via the existing `FIRST` `TestAdapter`. (2) `app!` macro — +verified the `expand_app` file-level diagnostics are untested but flaky +to trybuild (absolute paths in messages), so covered the cleanly-testable +seam instead: `build_route_tokens_propagates_invalid_handler_path`. Both +pass; full gate suite green. + +**Self-review hardening (round 9):** an adversarial review (mutation-lens ++ branch-coverage) of the added tests found weak/missing cases; all +addressed. (1) `nested-AppConfig` checker — added the previously untested +recursion arms of `type_contains_app_config_struct` (Rc/Arc/tuple/array/ +slice/reference/paren + cross-wrapper combos + an unwrapped-generic +negative), `struct_derives_app_config` bare-ident / multi-derive / +suffix-collision cases, and — the biggest gap — an **end-to-end** suite +driving the two-pass `NestedAppConfigVisitor` over parsed source (positive +through a wrapper, negatives, multi-count, definition-order, doc-comment +non-false-positive, tuple-struct unnamed field): 4 → 15 tests. (2) +`build_route_tokens` happy path — per-method token count, handler-less +skip, GET-default. (3) `registry` — the error-returning `push_config_entries` +/ `push_config_entries_local` defaults (assert the adapter-name message, +not just `is_err`). (4) Strengthened weak assertions: `handler` now checks +the response **body** flows through (not just status 200); `run_native_cli` +non-zero test pins the `exited with status` branch. Full gate suite green.