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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> > -# 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]