mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <>
To: Jeff King <>
Cc: Glen Choo <>, Taylor Blau <>,
	Glen Choo via GitGitGadget <>,
Subject: Re: [PATCH] t: run t5551 tests with both HTTP and HTTP/2
Date: Fri, 11 Nov 2022 16:06:20 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Fri, Nov 11 2022, Jeff King wrote:

> 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


> diff --git a/t/ b/t/
> new file mode 100755
> index 0000000000..9eece71c2c
> --- /dev/null
> +++ b/t/
> @@ -0,0 +1,4 @@
> +#!/bin/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 would do

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

  reply	other threads:[~2022-11-11 15:13 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-09  0:52 [PATCH] http: redact curl h2h3 headers in info Glen Choo via GitGitGadget
2022-11-10  2:52 ` Taylor Blau
2022-11-10 17:48   ` Glen Choo
2022-11-10 21:50     ` Jeff King
2022-11-10 22:53       ` Glen Choo
2022-11-11  2:29         ` Jeff King
2022-11-11  2:31           ` Taylor Blau
2022-11-11 14:49           ` [PATCH] t: run t5551 tests with both HTTP and HTTP/2 Jeff King
2022-11-11 15:06             ` Ævar Arnfjörð Bjarmason [this message]
2022-11-11 15:19               ` Jeff King
2022-11-11 15:20             ` Jeff King
2022-11-10 21:57 ` [PATCH] http: redact curl h2h3 headers in info Emily Shaffer
2022-11-10 22:14   ` Glen Choo
2022-11-11  2:35     ` Taylor Blau
2022-11-10 22:57 ` [PATCH v2] " Glen Choo via GitGitGadget
2022-11-11  2:36   ` Taylor Blau
2022-11-11  2:38   ` Jeff King
2022-11-11  2:39     ` Taylor Blau
2022-11-11 17:55     ` Glen Choo
2022-11-11 22:35   ` [PATCH v3 0/2] " Glen Choo via GitGitGadget
2022-11-11 22:35     ` [PATCH v3 1/2] t: run t5551 tests with both HTTP and HTTP/2 Jeff King via GitGitGadget
2022-11-11 22:35     ` [PATCH v3 2/2] http: redact curl h2h3 headers in info Glen Choo via GitGitGadget
2022-11-14 22:33     ` [PATCH v3 0/2] " Jeff King
2022-11-14 22:43       ` Taylor Blau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).