net/http: standardize HTTP/1 and HTTP/2 test paths #56032
Assignees
Labels
Comments
|
Change https://go.dev/cl/438137 mentions this issue: |
romaindoumenc
pushed a commit
to TroutSoftware/go
that referenced
this issue
Nov 3, 2022
Replace the ad-hoc approach to running tests in HTTP/1 and HTTP/2 modes with a 'run' function that executes a test in various modes. By default, these modes are HTTP/1 and HTTP/2, but tests can opt-in to HTTPS/1 as well. The 'run' function also takes care of post-test cleanup (running the afterTest function). The 'run' function runs tests in parallel by default. Tests which can't run in parallel (generally because they use global test hooks) pass a testNotParallel option to disable parallelism. Update clientServerTest to use t.Cleanup to clean up after itself, rather than leaving this up to tests to handle. Drop an unnecessary mutex in SetReadLoopBeforeNextReadHook. Test hooks can't be set in parallel, and we want the race detector to notify us if two simultaneous tests try to set a hook. Fixes golang#56032 Change-Id: I16be64913c426fc93d84abc6ad85dbd3bc191224 Reviewed-on: https://go-review.googlesource.com/c/go/+/438137 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: David Chase <drchase@google.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
The handling of HTTP/1 and HTTP/2 in
net/httptests is ad hoc and inconsistent. We've got a number of pre-HTTP/2 tests that are fairly tied to the protocol version (writing HTTP/1 request bytes directly to a socket, for example), but there are tests which could trivially run on both protocol versions but don't. The tests which do run on both versions do so by manually setting up paths for both.We should clean all this up: Run tests on all appropriate versions, using a standardized helper. This will also make it easier to test HTTP/3 if/when we support it. It'd also let us run other variations easily, such as HTTPS/1.
(I've already got CL 438137 to do this, filing an issue for bookkeeping/future reference.)
The text was updated successfully, but these errors were encountered: