[v3,0/5] tests: migrate to "test-tool pkt-line"

Message

Ævar Arnfjörð Bjarmason July 14, 2021, 12:54 a.m. UTC

Change code that uses [de]packetize() to use "test-tool
pkt-line". That we had these two concurrently in the tests is mostly
just a historical anomaly/inconsistency that we can fix.

Hopefully addresses Jeff King's feedback on v2: https://lore.kernel.org/git/[e-mail]/#t

I:

 * Changed the t5411/once-0010-report-status-v1.sh pattern to move
   away from the verbose "printf"..

 * Got rid of the stray strbuf_read()

 * Used a {} block in t/t5562-http-backend-content-length.sh

I kept the test_cmp in favor of the old logic of grepping out specific
parts, now with an updated commit message addressing that in 3/5, and
that we consistently include "0000" in all cases, even when we can
omit it.

Ævar Arnfjörð Bjarmason (5):
  serve tests: add missing "extra delim" test
  serve tests: use test_cmp in "protocol violations" test
  tests: replace [de]packetize() shell+perl test-tool pkt-line
  tests: replace remaining packetize() with "test-tool pkt-line"
  test-lib-functions.sh: remove unused [de]packetize() functions

 t/helper/test-pkt-line.c               | 12 +++++
 t/t5410-receive-pack-alternates.sh     | 42 +++++++++++++-----
 t/t5411/once-0010-report-status-v1.sh  | 20 ++++-----
 t/t5500-fetch-pack.sh                  | 15 ++++---
 t/t5530-upload-pack-error.sh           | 24 +++++-----
 t/t5562-http-backend-content-length.sh | 16 ++++---
 t/t5570-git-daemon.sh                  | 20 ++++++---
 t/t5704-protocol-violations.sh         | 61 ++++++++++++++++++--------
 t/test-lib-functions.sh                | 42 ------------------
 9 files changed, 138 insertions(+), 114 deletions(-)

Range-diff against v2:
1:  67aa814115 = 1:  67aa814115 serve tests: add missing "extra delim" test
2:  64dfd14865 = 2:  64dfd14865 serve tests: use test_cmp in "protocol violations" test
3:  c33f344ab2 ! 3:  92bfd8a87b tests: replace [de]packetize() shell+perl test-tool pkt-line
    @@ Commit message
         more legible code, and for anyone who cares about efficiency it'll be
         faster.
     
    +    The conversion away from extract_haves() in
    +    t5410-receive-pack-alternates.sh and the "just pick out" logic in
    +    t5570-git-daemon.sh isn't strictly required for this migration, but in
    +    this case it's easy enough to test_cmp the whole output, so let's just
    +    do that.
    +
    +    Similarly, there are cases here of changing "printf 0000" to a version
    +    that'll emit a trailing newline after "0000", or where we can do away
    +    with the "0000" trailer. Let's just always include, and include it in
    +    the same way.
    +
         We can't convert all the users of packetize(), it has a feature the
         test-tool is missing. This'll be addressed in the subsequent commit.
     
    @@ t/t5410-receive-pack-alternates.sh: test_expect_success 'with core.alternateRefs
     
      ## t/t5411/once-0010-report-status-v1.sh ##
     @@ t/t5411/once-0010-report-status-v1.sh: test_expect_success "proc-receive: report status v1" '
    + 			printf "%s %s refs/heads/main\0report-status object-format=$GIT_DEFAULT_HASH\n" \
      				$A $B | packetize
      		fi &&
    - 		printf "%s %s refs/for/main/topic1\n" \
    +-		printf "%s %s refs/for/main/topic1\n" \
     -			$ZERO_OID $A | packetize &&
    -+			$ZERO_OID $A | test-tool pkt-line pack &&
    - 		printf "%s %s refs/heads/foo\n" \
    +-		printf "%s %s refs/heads/foo\n" \
     -			$ZERO_OID $A | packetize &&
    -+			$ZERO_OID $A | test-tool pkt-line pack &&
    - 		printf "%s %s refs/for/next/topic\n" \
    +-		printf "%s %s refs/for/next/topic\n" \
     -			$ZERO_OID $A | packetize &&
    -+			$ZERO_OID $A | test-tool pkt-line pack &&
    - 		printf "%s %s refs/for/main/topic2\n" \
    +-		printf "%s %s refs/for/main/topic2\n" \
     -			$ZERO_OID $A | packetize &&
    -+			$ZERO_OID $A | test-tool pkt-line pack &&
    - 		printf 0000 &&
    +-		printf 0000 &&
    ++		test-tool pkt-line pack <<-EOF &&
    ++		$ZERO_OID $A refs/for/main/topic1
    ++		$ZERO_OID $A refs/heads/foo
    ++		$ZERO_OID $A refs/for/next/topic
    ++		$ZERO_OID $A refs/for/main/topic2
    ++		0000
    ++		EOF
      		printf "" | git -C "$upstream" pack-objects --stdout
      	} | git receive-pack "$upstream" --stateless-rpc \
    + 	>out 2>&1 &&
     
      ## t/t5500-fetch-pack.sh ##
     @@ t/t5500-fetch-pack.sh: test_expect_success 'shallow since with commit graph and already-seen commit' '
4:  a44e1790f2 ! 4:  9842c85d1f tests: replace remaining packetize() with "test-tool pkt-line"
    @@ t/helper/test-pkt-line.c: static void pack(int argc, const char **argv)
     +static void pack_raw_stdin(void)
     +{
     +	struct strbuf sb = STRBUF_INIT;
    -+	strbuf_read(&sb, 0, 0);
    ++
     +	if (strbuf_read(&sb, 0, 0) < 0)
     +		die_errno("failed to read from stdin");
     +	packet_write(1, sb.buf, sb.len);
    @@ t/t5411/once-0010-report-status-v1.sh: test_expect_success "proc-receive: report
     -				$A $B | packetize
     +				$A $B | test-tool pkt-line pack-raw-stdin
      		fi &&
    - 		printf "%s %s refs/for/main/topic1\n" \
    - 			$ZERO_OID $A | test-tool pkt-line pack &&
    + 		test-tool pkt-line pack <<-EOF &&
    + 		$ZERO_OID $A refs/for/main/topic1
     
      ## t/t5562-http-backend-content-length.sh ##
     @@ t/t5562-http-backend-content-length.sh: test_expect_success 'setup' '
    @@ t/t5562-http-backend-content-length.sh: test_expect_success 'setup' '
     
      ## t/t5570-git-daemon.sh ##
     @@ t/t5570-git-daemon.sh: test_expect_success 'hostname cannot break out of directory' '
    - '
      
      test_expect_success FAKENC 'hostname interpolation works after LF-stripping' '
    --	{
    + 	{
     -		printf "git-upload-pack /interp.git\n\0host=localhost" | packetize
     -		printf "0000"
    --	} >input &&
    -+	printf "git-upload-pack /interp.git\n\0host=localhost" >has-null &&
    -+	test-tool pkt-line pack-raw-stdin >input <has-null &&
    -+	test-tool pkt-line pack >>input <<-\EOF &&
    -+	0000
    -+	EOF
    ++		printf "git-upload-pack /interp.git\n\0host=localhost" |
    ++		test-tool pkt-line pack-raw-stdin &&
    ++		test-tool pkt-line pack <<-\EOF
    ++		0000
    ++		EOF
    + 	} >input &&
     +
      	fake_nc "$GIT_DAEMON_HOST_PORT" <input >output &&
      	test-tool pkt-line unpack <output >actual &&
5:  cc91d15ef7 = 5:  7ca83c5a55 test-lib-functions.sh: remove unused [de]packetize() functions