pjfanning opened a new pull request, #3524:
URL: https://github.com/apache/pekko/pull/3524
### Motivation
`TcpFraming.ReadStreamId` selects `maximumLargeFrameSize` only for
`ArteryTransport.LargeStreamId` and `maximumFrameSize` otherwise (#3492),
but no test
exercised that selection. The existing bound tests (`"reject a frame that
exceeds the
maximum frame size"`, `"accept a frame at exactly the maximum frame size"`)
construct
`TcpFraming` with only `maximumFrameSize` set — `maximumLargeFrameSize`
defaults to
`Int.MaxValue` — so they cannot distinguish the correct per-stream selection
from a bug
that applied one bound to every stream regardless of `streamId`.
### Modification
Add a `perStreamBoundedFramingFlow` fixture with distinct `maximumFrameSize`
and
`maximumLargeFrameSize`, and two tests:
- a frame over the ordinary maximum but within the large maximum is
**accepted** on
`ArteryTransport.LargeStreamId`
- the same frame is **rejected** on `ArteryTransport.OrdinaryStreamId`
Both frames carry their full declared payload rather than just the header,
so an
incorrectly-accepted frame can't be mistaken for a truncation failure — I
hit exactly
that mistake while writing the reject test (an under-length frame fails from
truncation
regardless of the bound logic) and caught it by deliberately breaking the
production
selection and confirming the test still passed for the wrong reason, then
fixed the test
to include the payload.
### Result
The per-stream bound selection in `ReadStreamId` is covered directly, rather
than only
by the code being read during review.
### Tests
- `sbt "remote/testOnly org.apache.pekko.remote.artery.tcp.TcpFramingSpec"`
— 16 passed
- Checked the new tests discriminate: with `ReadStreamId`'s selection
temporarily
replaced by an unconditional `maximumLargeFrameSize`, the ordinary-stream
test failed
(`Future.failed not completed with a throwable`) while the large-stream
test still
passed; reverted after confirming
- `sbt "remote/scalafmtCheckAll"` — clean
### References
None - test-coverage gap noticed while comparing `TcpFraming.scala`
(hardened in #3492)
against an unrelated fix for the same class of issue in a downstream project.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
