Re: [PATCH v3 1/3] t5323: test cases for git-pack-redundant

From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Jiang Xin <worldhello.net@gmail.com>
Cc: Sun Chao <sunchao9@huawei.com>, Git List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Jiang Xin <zhiyou.jx@alibaba-inc.com>
Subject: Re: [PATCH v3 1/3] t5323: test cases for git-pack-redundant
Date: Wed, 9 Jan 2019 17:47:31 +0100	[thread overview]
Message-ID: <20190109164731.GJ4673@szeder.dev> (raw)
In-Reply-To: <20190109125628.GG4673@szeder.dev>

On Wed, Jan 09, 2019 at 01:56:28PM +0100, SZEDER Gábor wrote:
> On Wed, Jan 02, 2019 at 12:34:54PM +0800, Jiang Xin wrote:
> > +cat >expected <<EOF
> > +P1:$P1
> > +P4:$P4
> > +P5:$P5
> > +P6:$P6
> > +EOF

Creating the expected results could be moved into the
test_expect_success block as well.

> > +
> > +test_expect_success 'git pack-redundant --all' '
> > +	git pack-redundant --all | \
> 
> Don't run a git command (especially the particular command the test
> script focuses on) upstream of a pipe, because it hides the command's
> exit code.  Use an intermediate file instead.
> 
> > +		sed -e "s#^.*/pack-\(.*\)\.\(idx\|pack\)#\1#g" | \
> 
> This sed command doesn't seem to work on macOS (on Travis CI), and
> causes the test to fail with:
> 
>   ++git pack-redundant --all
>   ++sed -e 's#^.*/pack-\(.*\)\.\(idx\|pack\)#\1#g'
>   ++sort -u
>   ++read p
>   ++sort
>   ++eval echo '${P.git/objects/pack/pack-0cf5cb6afaa1bae36b8e61ca398dbe29a15bc74e.idx}'
>   ./test-lib.sh: line 697: ${P.git/objects/pack/pack-0cf5cb6afaa1bae36b8e61ca398dbe29a15bc74e.idx}: bad substitution
>   ++test_cmp expected actual
>   ++diff -u expected actual
>   --- expected    2019-01-09 01:53:45.000000000 +0000
>   +++ actual      2019-01-09 01:53:45.000000000 +0000
>   @@ -1,4 +0,0 @@
>   -P1:24ee080366509364d04a138cd4e168dc4ff33354
>   -P4:139d8b0cfe7e8970a8f3533835f90278d88de474
>   -P5:23e0f02d822fa4bfe5ee63337ba5632cd7be208e
>   -P6:deeb289f1749972f1cd57c3b9f359ece2361f60a
>   error: last command exited with $?=1
>   not ok 2 - git pack-redundant --all
> 
> I'm not sure what's wrong with it, though.

So, it appears that 'sed' in macOS doesn't understand the
'$idx\|pack$' part of that regex.  Turning that command into

  sed -e "s#^.git/objects/pack/pack-\($OID_REGEX\)\..*#\1#" out | \

makes it work even on macOS, but note that those 40 hexdigits are not
actual OIDs but file content checksums, so using $OID_REGEX is not the
right thing to do here (though I'm not sure what is supposed to be
used instead, as $_x40 hardcodes the number of hexdigits).

Alas, the test as a whole still fails with the following on macOS:

  ++diff -u expected actual
  --- expected    2019-01-09 15:54:49.000000000 +0000
  +++ actual      2019-01-09 15:54:49.000000000 +0000
  @@ -1,4 +1,4 @@
   P1:24ee080366509364d04a138cd4e168dc4ff33354
  -P4:139d8b0cfe7e8970a8f3533835f90278d88de474
  +P3:0cf5cb6afaa1bae36b8e61ca398dbe29a15bc74e
   P5:23e0f02d822fa4bfe5ee63337ba5632cd7be208e
  -P6:deeb289f1749972f1cd57c3b9f359ece2361f60a
  +P7:4ecc1eb138516a26654cd4e3570b322c0820f170
  error: last command exited with $?=1