Re: [PATCH] t1500-rev-parse: rewrite each test to run in isolation

From: Jeff King <peff@peff.net>
Date: Apr 13, 2016, 18:29 UTC
Message-ID: <20160413182928.GA31787@sigill.intra.peff.net>
In-Reply-To: <CAPig+cT8oD1jJdDrk+YPoquGfTh6m3m-ha0J+er42jOoxnVxzg@mail.gmail.com>

On Wed, Apr 13, 2016 at 12:54:16AM -0400, Eric Sunshine wrote:

Show 7 quoted lines

> > -# label is-bare is-inside-git is-inside-work prefix git-dir absolute-git-dir
> > +test_expect_success '.git/: is-inside-git-dir' '
> > +       (cd .git && test true = "$(git rev-parse --is-inside-git-dir)")
> 
> Simpler:
> 
>     test true = "$(git -C .git rev-parse --is-inside-git-dir)"

While we are modernizing, I wonder if it is a good idea to drop the use of "test" for string comparisons here. We usually save output to a file and use test_cmp, for two reasons:

  1. It tests the exit code of the command substitution.

  2. It's easier to debug when it goes wrong (the test produces useful
     output, and the state is left in the filesystem).

It is more verbose to do so, but we could easily wrap that in:

  test_stdout "true" git -C .git rev-parse --is-inside-git-dir

or something. The other problem is that it uses an extra process to do the test_cmp, which might make the tests slower (especially on Windows). We could do something like:

  test_stdout () {
          expect=$1; shift
	  if ! actual=$("$@")
	  then
		echo >&2 "test_stdout: command failed: $*"
		return 1
	  fi
	  if test "$expect" != "$actual"
	  then
	          echo >&2 "test_stdout: unexpected output for $*"
		  echo >&2 "test_stdout: $expect != $actual"
		  return 1
	  fi
	  return 0
  }

which is more thorough without incurring extra processes (we lose the nice diff output of test_cmp, but since this interface is only useful for short outputs anyway, I don't think that's a big deal.

-Peff