Skip to content

fix(test): expect legacy-abort deprecation warning in request_signal_streaming#35191

Closed
divybot wants to merge 1 commit into
mainfrom
fix/serve-request-signal-warning
Closed

fix(test): expect legacy-abort deprecation warning in request_signal_streaming#35191
divybot wants to merge 1 commit into
mainfrom
fix/serve-request-signal-warning

Conversation

@divybot

@divybot divybot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Problem

test specs ... serve::request_signal_streaming is currently failing on every main commit since #34397 landed. Example: it fails on the latest main (69de4f41e4) and on the 6 commits before it, back to #34397 itself.

This is a semantic merge conflict between two PRs that landed in parallel:

The test exercises exactly that scenario (it reads req.signal.aborted and adds an abort listener, then returns a successful streaming response), so the new warning is now printed to stderr and the captured output no longer matches main.out. Neither PR's CI saw the other's change.

Fix

The warning is correct and intended for this case, so update the test's expected output to include it. The warning is emitted deterministically as the first line (verified across repeated runs).

+Deno.serve: request.signal aborts on successful responses (legacy behavior, ...). ...
 body: chunk1;chunk2;chunk3;chunk4;chunk5;
 aborted during stream: false
 aborted after completion: true

Verification

cargo test --test specs -- serve::request_signal_streaming
test specs::serve::request_signal_streaming ... ok

This unblocks test specs (…) debug for all open PRs (it currently fails for every PR that merges with main).

…streaming

The `serve::request_signal_streaming` spec test (added in #35049) and the
legacy-abort deprecation warning (added in #34397) landed in parallel and
collided on main: the test exercises legacy abort with a handler that accesses
`request.signal` and returns successfully, which is exactly the case #34397
now warns about. Neither PR saw the other, so main has been failing
`test specs ... request_signal_streaming` on every commit since #34397.

The warning is correct and expected for this scenario, so update the test's
expected output to include it. Verified the warning is emitted deterministically
as the first line.
littledivy pushed a commit that referenced this pull request Jun 13, 2026
…streaming

This test (added in #35049) and the legacy-abort deprecation warning (added in
#34397) landed in parallel and collided on main: the test exercises legacy
abort with a handler that accesses request.signal and returns successfully,
which is exactly what #34397 warns about, but its main.out wasn't updated.
main has been failing this spec test on every commit since #34397, which blocks
CI for this PR (and every other) once merged with main.

The warning is correct for this scenario, so expect it in the test output.
Folded in here (rather than the standalone #35191) to make this PR's CI
self-sufficient; happy to split back out if preferred.
@divybot

divybot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor Author

Folding this into #35133 (the publish PR) so that PR's CI is self-sufficient and doesn't have to wait on this one to merge first. The fix is identical — updating request_signal_streaming's expected output to include the legacy-abort deprecation warning. If you'd rather keep it standalone, I can reopen and drop it from #35133.

@divybot divybot closed this Jun 13, 2026
littledivy pushed a commit that referenced this pull request Jun 13, 2026
…ming

Merging main brings in #34397, which emits a legacy-abort deprecation
warning when a Deno.serve handler uses legacy request.signal abort on a
successful response. The request_signal_streaming spec exercises exactly
that path, so its expected output must include the warning line. Same fix
as the closed #35191 (folded into #35133); included here so this PR's CI
is self-sufficient. Unrelated to the blob-worker change.
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.

1 participant