Skip to content

Add route_request dispatch coverage#689

Open
ChristianPavilonis wants to merge 3 commits into
mainfrom
test-route-request-dispatch-451
Open

Add route_request dispatch coverage#689
ChristianPavilonis wants to merge 3 commits into
mainfrom
test-route-request-dispatch-451

Conversation

@ChristianPavilonis
Copy link
Copy Markdown
Collaborator

Summary

  • Add focused Fastly adapter route_request dispatch coverage.
  • Cover static TSJS success and unknown bundle handling.
  • Cover public discovery and unauthenticated admin dispatch.
  • Cover auction dispatch through consent-dependent setup.
  • Cover unknown-route publisher fallback dispatch without configured consent persistence.

Closes #451

Tests

  • cargo test -p trusted-server-adapter-fastly route_tests -- --nocapture
  • cargo fmt --all -- --check
  • cargo test --workspace
  • cargo clippy --workspace --all-targets --all-features -- -D warnings

@ChristianPavilonis ChristianPavilonis marked this pull request as ready for review May 13, 2026 19:06
Copy link
Copy Markdown
Collaborator

@prk-Jr prk-Jr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

This PR adds focused Fastly adapter coverage for route_request. The overall direction is useful, but one assertion currently does not prove the route branch it is intended to cover because another branch can produce the same failure status.

CI Status

  • cargo fmt: PASS
  • cargo test: PASS
  • vitest: PASS
  • integration tests: PASS
  • browser integration tests: PASS
  • CodeQL: PASS
  • analysis jobs: PASS

Comment thread crates/trusted-server-adapter-fastly/src/route_tests.rs
Comment thread crates/trusted-server-adapter-fastly/src/route_tests.rs Outdated
Comment thread crates/trusted-server-adapter-fastly/src/route_tests.rs
Copy link
Copy Markdown
Collaborator

@aram356 aram356 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Decomposing the previous monolithic test into focused per-route dispatch tests is a clean improvement, and the prior blocking concerns from @prk-Jr (auction passing through fallback / ambient DNS dependency) are fully addressed in 3f7d3333 — the "Invalid banner size" body assertion and the .invalid origin URL both make the intended failure modes unambiguous. CI is green and the new tests pass locally.

Requesting changes on one point only: switching auction_route_dispatches_to_auction_handler to consent_store = None proves handler dispatch but drops the regression assertion that runtime_services_for_consent_route actually gates the /auction route. Only the publisher-fallback gate is now under test. A small companion test restores parity with the original coverage — see the inline 🌱.

Non-blocking

🌱 seedling

  • Lost regression coverage for "auction returns 503 when consent_store is misconfigured"route_tests.rs:296 (inline)

♻️ refactor

  • Collapse the four-layer settings helper into tworoute_tests.rs:128 (inline)

🤔 thinking

  • .invalid 502 is consistent with Proxy but not exclusive to itroute_tests.rs:322 (inline)

⛏ nitpick

  • !is_empty() is satisfied by any non-empty bodyroute_tests.rs:243 (inline)

CI Status

  • cargo fmt: PASS
  • cargo clippy: PASS (verified locally on trusted-server-adapter-fastly)
  • cargo test: PASS (verified locally — 7/7 new tests pass)
  • vitest: PASS
  • format-docs / format-typescript: PASS
  • browser integration tests / integration tests: PASS
  • CodeQL: PASS

Comment thread crates/trusted-server-adapter-fastly/src/route_tests.rs
Comment thread crates/trusted-server-adapter-fastly/src/route_tests.rs Outdated
Comment thread crates/trusted-server-adapter-fastly/src/route_tests.rs
Comment thread crates/trusted-server-adapter-fastly/src/route_tests.rs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add integration coverage for route_request dispatch behavior

3 participants