Skip to content

Commit

Permalink
feat(http/retry): add more replay body test coverage (#3583)
Browse files Browse the repository at this point in the history
see linkerd/linkerd2#8733.

we are currently in the process of upgrading our hyper, http, and http-body dependencies to their 1.0 major releases.

this branch introduces additional test coverage, and further refines existing test coverage, concerning how a `ReplayBody<B>` behaves when it reaches the "end of stream" of its internal `B`-typed body.

additional assertions are added to show that bodies with trailers may be replayed an arbitrary number of times, and that capacity errors occur precisely at their expected boundary. additional assertions are added to confirm that `ReplayBody::is_capped()` reports these conditions properly.

this branch also notably outlines the unit test suite into a separate file, due to its size. as a result, reviewers are encouraged to walk through this branch on a commit-by-commit basis when reading these changes.

i noticed some relatively minor issues with `is_end_stream()` and `size_hint()` while i was reviewing this middleware, in preparation to port it to http-body 1.0. i have left `TODO` comments noting where today's behavior is slightly askew, but _intentionally avoided_ fixing them here. my goal on that front is to highlight those wrinkles so that later fixes to these edges are more easily reviewable.

---

* refactor(http/retry): outline `ReplayBody<B>` unit tests

there are more than 500 lines of unit tests. let's move them into a
submodule, for convenience.

Signed-off-by: katelyn martin <[email protected]>

* nit(http/retry): reorganize replay tests

this is a small cosmetic change reording some test helpers.

there is a common convention of affixing a banner comment above groups
of `impl T {}` blocks, which is useful when top-level blocks are folded
in an editor.

similarly, there is a convention of defining structures at the top of a
file.

this commit reorganizes the replay body tests to follow each of these
conventions.

Signed-off-by: katelyn martin <[email protected]>

* nit(http/retry): test replays trailers twice

just to be extra sure!

Signed-off-by: katelyn martin <[email protected]>

* nit(http/retry): rename `trailers_only()` test

this is part of a family of other tests called `replays_one_chunk()`,
`replays_several_chunks()`, and `replays_trailers()`. let's name this
something that lines up with this convention.

Signed-off-by: katelyn martin <[email protected]>

* feat(http/retry): add a `TestBody` type

we have a unit test, called `eos_only_when_fully_replayed` that confirms
`Body::end_of_stream()` reports the stream ending properly.

soon, we aim to introduce additional test coverage that exercises this
when a body has trailers, as well. this will be useful for assurance
related to upgrading to http-body v1.x. see linkerd/linkerd2#8733 for
more information.

unfortunately, hyper 0.14's channel-backed body does not report itself
as having reached the end of the stream. this is an unfortunate quality
that prevents us from using `Test::new()`.

this commit adds a `TestBody` type that we can use in place of
`BoxBody::from_static(..)`, which boxes a static string, but does not
send trailers.

Signed-off-by: katelyn martin <[email protected]>

* feat(http/retry): add `is_end_stream()` coverage for trailers

this commit introduces additional test coverage that exercises
`is_end_stream()` when a replay body is wrapping a body with trailers.
this will be useful for assurance related to upgrading to http-body
v1.x. see linkerd/linkerd2#8733 for more information.

Signed-off-by: katelyn martin <[email protected]>

* feat(http/retry): add `is_capped()` test coverage

Signed-off-by: katelyn martin <[email protected]>

* feat(http/retry): further refine capacity test coverage

we want to show that exceeding the capacity is the point at which
replays will fail.

this commit defines some constants to further communicate and encode
this relationship between the bytes sent, and the capacity of the replay
body.

further, it shortens the second frame sent so that we ensure precisely
when a body becomes capped.

Signed-off-by: katelyn martin <[email protected]>

* feat(http/retry): add `size_hint()` test coverage

Signed-off-by: katelyn martin <[email protected]>

* chore(http/retry): add todo comments concerning eos

for now, a replaying body that will not yield trailers must be polled to
the `None` before reporting itself as reaching the end of the stream.

this isn't hugely important, but does affect some test control flow.

leave two todo comments so that if/when upgrading to hyper 1.0, it is
clear that these are not load-bearing or otherwise expected behavior,
should this behavior be rectified.

Signed-off-by: katelyn martin <[email protected]>

---------

Signed-off-by: katelyn martin <[email protected]>
  • Loading branch information
cratelyn authored Jan 30, 2025
1 parent 8d37620 commit afda8a7
Show file tree
Hide file tree
Showing 2 changed files with 812 additions and 572 deletions.
Loading

0 comments on commit afda8a7

Please sign in to comment.