git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] t: run t5551 tests with both HTTP and HTTP/2

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 11, 2022, 15:06 UTC
Message-ID
<221111.86wn81mssk.gmgdl@evledraar.gmail.com>
In-Reply-To
<Y25hDr7aHvKnxso3@coredump.intra.peff.net>
On Fri, Nov 11 2022, Jeff King wrote:
Show 51 quoted lines
> On Thu, Nov 10, 2022 at 09:29:15PM -0500, Jeff King wrote:
>
>> We don't get the _whole_ test suite running with http2, but hopefully it
>> gives us a fairly representative sample. And it does find this bug.
>> 
>> I can try to work the above into patch form, but I may not get to it for
>> a day or two.
>
> So here's a patch which finds the bug you're fixing. It's set up to be
> prepared before your patch, which helps confirm the bug (and that we're
> correctly using http/2 in the tests!). You'll want to unmark the !HTTP2
> prereqs as part of your patch.
>
> Alternatively, it could come after your patch to confirm the fix, in
> which case it would omit those !HTTP2 prereqs from the get-go.
>
> Let me know if you'd like to pick it up as part of your series.
> Otherwise, I can submit it separately in the on-top form (but my
> preference is for you to take it, since it makes testing the fix
> easier).
>
> -- >8 --
> Subject: t: run t5551 tests with both HTTP and HTTP/2
>
> We have occasionally seen bugs that affect Git running only against an
> HTTP/2 web server, not an HTTP one. For instance, b66c77a64e (http:
> match headers case-insensitively when redacting, 2021-09-22). But since
> we have no test coverage using HTTP/2, we only uncover these bugs in the
> wild.
>
> That commit gives a recipe for converting our Apache setup to support
> HTTP/2, but:
>
>   - it's not necessarily portable
>
>   - we don't want to just test HTTP/2; we really want to do a variety of
>     basic tests for _both_ protocols
>
> This patch handles both problems by running a duplicate of t5551
> (labeled as t5559 here) with an alternate-universe setup that enables
> HTTP/2. So we'll continue to run t5551 as before, but run the same
> battery of tests again with HTTP/2. If HTTP/2 isn't supported on a given
> platform, then t5559 should bail during the webserver setup, and
> gracefully skip all tests (unless GIT_TEST_HTTPD has been changed from
> "auto" to "yes", where the point is to complain when webserver setup
> fails).
>
> In theory other http-related test scripts could benefit from the same
> duplication, but doing t5551 should give us a reasonable check of basic
> functionality, and would have caught both bugs we've seen in the wild
> with HTTP/2.

Looking at the diff below, would it be much more work to just support a GIT_TEST_HTTP_VERSION=2?

Then:
Show 10 quoted lines
> diff --git a/t/t5559-http-fetch-smart-http2.sh b/t/t5559-http-fetch-smart-http2.sh
> new file mode 100755
> index 0000000000..9eece71c2c
> --- /dev/null
> +++ b/t/t5559-http-fetch-smart-http2.sh
> @@ -0,0 +1,4 @@
> +#!/bin/sh
> +
> +HTTP_PROTO=HTTP/2
> +. ./t5551-http-fetch-smart.sh

This would just skip_all if it was already set to 2, as we'd know we were testing it in other tests, and lib-httpd.sh would do GIT_TEST_HTTP_VERSION=1 by default.

There's the "!HTTP2" additions, which you mention you'll be "fixing momentarily", but this is a stand-alone patch, so maybe I'm missing that context...

Previous: Jeff KingNext: Jeff King
Message 9 of 24 in “http: redact curl h2h3 headers in info”
  1. http: redact curl h2h3 headers in infoGlen Choo via GitGitGadget, Nov 9, 2022
  2. Taylor BlauNov 10, 2022
  3. Glen ChooNov 10, 2022
  4. Jeff KingNov 10, 2022
  5. Glen ChooNov 10, 2022
  6. Jeff KingNov 11, 2022
  7. Taylor BlauNov 11, 2022
  8. t: run t5551 tests with both HTTP and HTTP/2Jeff King, Nov 11, 2022
  9. Ævar Arnfjörð BjarmasonNov 11, 2022
  10. Jeff KingNov 11, 2022
  11. Jeff KingNov 11, 2022
  12. Emily ShafferNov 10, 2022
  13. Glen ChooNov 10, 2022
  14. Taylor BlauNov 11, 2022
  15. http: redact curl h2h3 headers in infoGlen Choo via GitGitGadget, Nov 10, 2022
  16. Taylor BlauNov 11, 2022
  17. Jeff KingNov 11, 2022
  18. Taylor BlauNov 11, 2022
  19. Glen ChooNov 11, 2022
  20. 0/2 http: redact curl h2h3 headers in infoGlen Choo via GitGitGadget, Nov 11, 2022
  21. 1/2 t: run t5551 tests with both HTTP and HTTP/2Jeff King via GitGitGadget, Nov 11, 2022
  22. 2/2 http: redact curl h2h3 headers in infoGlen Choo via GitGitGadget, Nov 11, 2022
  23. Jeff KingNov 14, 2022
  24. Taylor BlauNov 14, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.