Skip to content

Add auction endpoint request handling coverage#690

Open
ChristianPavilonis wants to merge 3 commits into
mainfrom
test-auction-endpoint-452
Open

Add auction endpoint request handling coverage#690
ChristianPavilonis wants to merge 3 commits into
mainfrom
test-auction-endpoint-452

Conversation

@ChristianPavilonis
Copy link
Copy Markdown
Collaborator

@ChristianPavilonis ChristianPavilonis commented May 13, 2026

Summary

  • Add route-level coverage for /auction request handling
  • Return 400 Bad Request for malformed auction JSON payloads
  • Cover semantic validation failures, orchestration failures, and a stable no-network success path
  • Note /auction now returns 502 Bad Gateway when every configured provider is skipped, disabled, or fails to launch instead of returning 200 with an empty seatbid.

Closes #452

Testing

  • cargo test -p trusted-server-adapter-fastly route_tests -- --nocapture
  • cargo test -p trusted-server-core auction:: -- --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:14
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

The route coverage is useful, but one new test now codifies a configuration typo as a request-time 502 instead of a startup/configuration failure.

Blocking

🔧 wrench

  • Missing auction provider should fail startup, not become request-time 502: auction.providers = ["missing-provider"] currently builds the route stack and only fails when /auction is called. Invalid enabled providers should fail during startup/registration so deployments do not start with a mistyped provider list.

CI Status

  • cargo fmt / clippy: PASS
  • rust tests: PASS
  • js tests: PASS
  • integration tests: PASS

Comment thread crates/trusted-server-adapter-fastly/src/route_tests.rs Outdated
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

The endpoint change correctly fixes a real bug — malformed /auction JSON now returns 400 instead of 502 — and the three accompanying tests for that path are solid. The contentious piece (already flagged by @prk-Jr) is the fourth test, which codifies a configuration typo (providers = ["missing-provider"]) as a request-time 502. That conflicts with the project rule in CLAUDE.md and locks in disputed behavior with an asserting test ahead of the #669 follow-up. Inline notes cover the blocking item, two thinking notes, two refactor suggestions, and one seedling.

Blocking

🔧 wrench

  • Test codifies a config typo as request-time 502, conflicting with the project rule (crates/trusted-server-adapter-fastly/src/route_tests.rs:389-400). See inline comment for the two suggested resolutions (validate at startup, or drop just this test pending #669).

Non-blocking

🤔 thinking

  • Orchestrator guard behavior change is broader than the typo case (crates/trusted-server-core/src/auction/orchestrator.rs:365-369). Inline.
  • Test name "no_providers" is ambiguous (crates/trusted-server-adapter-fastly/src/route_tests.rs:377). Inline.

♻️ refactor

  • Error message lacks diagnostic info (crates/trusted-server-core/src/auction/orchestrator.rs:367). Inline alongside the thinking note.
  • TOML duplication between test settings helpers (crates/trusted-server-adapter-fastly/src/route_tests.rs:166-196). Inline.

🌱 seedling

  • Add a unit-level test for the new orchestrator guard. The existing TODO at orchestrator.rs:765-778 already notes the gap. A fake AuctionProvider whose request_bids returns Err would cover the "registered-but-failed-to-launch" path that the route-level test doesn't reach. Could land with the #669 follow-up.

CI Status

  • cargo fmt: PASS
  • cargo clippy: PASS
  • cargo test (Rust): PASS
  • vitest (JS): PASS
  • format-typescript / format-docs: PASS
  • integration tests + browser integration tests: PASS
  • CodeQL / Analyze: PASS

Comment thread crates/trusted-server-adapter-fastly/src/route_tests.rs Outdated
Comment thread crates/trusted-server-core/src/auction/orchestrator.rs Outdated
Comment thread crates/trusted-server-adapter-fastly/src/route_tests.rs Outdated
Comment thread crates/trusted-server-adapter-fastly/src/route_tests.rs
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 tests for auction endpoint request handling

3 participants