| Message ID | Y/dEYYWKy/o96vBG@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Jeff King <peff@peff.net> writes: > So all I wanted to do was this one-liner: > > diff --git a/t/t5559-http-fetch-smart-http2.sh b/t/t5559-http-fetch-smart-http2.sh > index 9eece71c2c..54aa9d3bff 100755 > --- a/t/t5559-http-fetch-smart-http2.sh > +++ b/t/t5559-http-fetch-smart-http2.sh > @@ -1,4 +1,5 @@ > #!/bin/sh > > HTTP_PROTO=HTTP/2 > +LIB_HTTPD_SSL=1 > . ./t5551-http-fetch-smart.sh > > but somehow I'm 16 patches deep. Let me back up. > > I got bit once again by the "oops, HTTP/2 tests in t5559 are sometimes > flaky" bug. One thing that came up in earlier discussion is that HTTP/2 > over TLS should be much more reliable, because it doesn't have to go > through the funny HTTP-upgrade path. > > Hence the patch above, which is also patch 16 here. And it does make the > consistent failure of t5551.30 go away. And it even makes --stress work > longer before a racy failure, though it still fails for me pretty > consistently within a few dozen runs. > > But in doing so, I found out all sorts of neat things, like: > > - when I tested with HTTP/2 and TLS before, I was accidentally not > using HTTP/2! > > - we even have a test that should detect which version is used, but > it's a silent noop unless you set GIT_TEST_PROTOCOL_VERSION=0, which > clearly nobody does > > - it turns out there are a bunch of tests which are skipped (some of > which even fail!) unless you set that variable > > So this series fixes the broken tests, adapts them to work with both v0 > and v2 Git protocol, makes them work with HTTP/2 when needed, sprinkles > in a couple other fixes, and then finally does that one-liner. > > I'm actually not sure if the final patch is a good idea or not, but > certainly all of the fixes leading up to it are worth doing. Thanks; this must have been a lot of work. From the "test what the end users use, or at least something close to it" standpoint, 16/16 certainly is the right thing to do, I would think.
On Thu, Feb 23, 2023 at 03:37:05PM -0800, Junio C Hamano wrote: > > I'm actually not sure if the final patch is a good idea or not, but > > certainly all of the fixes leading up to it are worth doing. > > Thanks; this must have been a lot of work. From the "test what the > end users use, or at least something close to it" standpoint, 16/16 > certainly is the right thing to do, I would think. Yeah. My main concern is that we are now using SSL by default in the test suite (or at least trying to; I _think_ we should fail gracefully, but since it works on my system, I don't have any data beyond the fact that CI seems OK with it). I think it's one of those things where we try it and see if anybody screams. -Peff
Jeff King <peff@peff.net> writes: > On Thu, Feb 23, 2023 at 03:37:05PM -0800, Junio C Hamano wrote: > >> > I'm actually not sure if the final patch is a good idea or not, but >> > certainly all of the fixes leading up to it are worth doing. >> >> Thanks; this must have been a lot of work. From the "test what the >> end users use, or at least something close to it" standpoint, 16/16 >> certainly is the right thing to do, I would think. > > Yeah. My main concern is that we are now using SSL by default in the > test suite (or at least trying to; I _think_ we should fail gracefully, > but since it works on my system, I don't have any data beyond the fact > that CI seems OK with it). I think it's one of those things where we try > it and see if anybody screams. True. If a platform has TLS/SSL implementation of acceptable quality and if we fail to build with it, that is something we want to learn about and help them make it work anyway, I suspect. Thanks.
diff --git a/t/t5559-http-fetch-smart-http2.sh b/t/t5559-http-fetch-smart-http2.sh index 9eece71c2c..54aa9d3bff 100755 --- a/t/t5559-http-fetch-smart-http2.sh +++ b/t/t5559-http-fetch-smart-http2.sh @@ -1,4 +1,5 @@ #!/bin/sh HTTP_PROTO=HTTP/2 +LIB_HTTPD_SSL=1 . ./t5551-http-fetch-smart.sh
So all I wanted to do was this one-liner: but somehow I'm 16 patches deep. Let me back up. I got bit once again by the "oops, HTTP/2 tests in t5559 are sometimes flaky" bug. One thing that came up in earlier discussion is that HTTP/2 over TLS should be much more reliable, because it doesn't have to go through the funny HTTP-upgrade path. Hence the patch above, which is also patch 16 here. And it does make the consistent failure of t5551.30 go away. And it even makes --stress work longer before a racy failure, though it still fails for me pretty consistently within a few dozen runs. But in doing so, I found out all sorts of neat things, like: - when I tested with HTTP/2 and TLS before, I was accidentally not using HTTP/2! - we even have a test that should detect which version is used, but it's a silent noop unless you set GIT_TEST_PROTOCOL_VERSION=0, which clearly nobody does - it turns out there are a bunch of tests which are skipped (some of which even fail!) unless you set that variable So this series fixes the broken tests, adapts them to work with both v0 and v2 Git protocol, makes them work with HTTP/2 when needed, sprinkles in a couple other fixes, and then finally does that one-liner. I'm actually not sure if the final patch is a good idea or not, but certainly all of the fixes leading up to it are worth doing. [01/16]: t5541: run "used receive-pack service" test earlier [02/16]: t5541: stop marking "used receive-pack service" test as v0 only [03/16]: t5541: simplify and move "no empty path components" test [04/16]: t5551: drop redundant grep for Accept-Language [05/16]: t5551: lower-case headers in expected curl trace [06/16]: t5551: handle HTTP/2 when checking curl trace [07/16]: t5551: stop forcing clone to run with v0 protocol [08/16]: t5551: handle v2 protocol when checking curl trace [09/16]: t5551: handle v2 protocol in upload-pack service test [10/16]: t5551: simplify expected cookie file [11/16]: t5551: handle v2 protocol in cookie test [12/16]: t5551: drop curl trace lines without headers [13/16]: t/lib-httpd: respect $HTTPD_PROTO in expect_askpass() [14/16]: t/lib-httpd: enable HTTP/2 "h2" protocol, not just h2c [15/16]: t5559: fix test failures with LIB_HTTPD_SSL [16/16]: t5559: make SSL/TLS the default t/lib-httpd.sh | 6 +- t/lib-httpd/apache.conf | 2 +- t/t5541-http-push-smart.sh | 57 +++------- t/t5551-http-fetch-smart.sh | 170 ++++++++++++++++++------------ t/t5559-http-fetch-smart-http2.sh | 1 + 5 files changed, 122 insertions(+), 114 deletions(-) -Peff