Subject: Re: [PATCH] More test cases for sanitized path names

From: Junio C Hamano <[email-masked]>
Date: Thu, 31 Jan 2008 23:17:11 -0800

Robin Rosenberg <[email-masked]> writes:

> +test_expect_failure 'add a directory outside the work tree' '
> +	d1="$(cd .. ; pwd)" &&
> +	git add "$d1"
> +	echo $?
> +'

This test will always fail as the final exit status is that of
"echo", which will exit with success and you are expecting a
failure.

> +test_expect_failure 'add a file outside the work tree, nasty case 1' '(
> +	f="$(pwd)x" &&
> +	touch "$f" &&
> +	git add "$f"
> +)'

You are in the directory "t/trash", and try to add t/trashx, so
this should fail and you would want to make sure it fails.

But this has a few problems:

 * First, the obvious one.  You are creating a garbage file
   outside of t/trash directory.  Don't.  If you need to, dig a
   test directory one level lower inside t/trash and play around
   there.

 * In general you should stay away from test_expect_failure.  If
   any of the command in && chain fails, it fails the whole
   thing, but you cannot tell if the sequence failed at the
   command you expected to fail or something else that is much
   earlier.  For example, it may be that somebody created t/trashx
   file in the source tree that is read-only, and the comand
   that failed in the sequence could be 'touch' before the
   command you are testing.

   Instead, write it like (after fixing it not to go outside
   t/trash):

	test_expect_success 'add a path outside repo (1)' '

		file=path_to_outside_repo &&
                touch "$file" &&
		! git add "$f"

	'

I'd like to make the _first_ patch after 1.5.4 to be a fix-up
for tests that misuse test_expect_failure.  After that, we can
use test_expect_failure to mark tests that ought to pass but
don't because of bugs in the commands.  That way, people who are
absolutely bored can grep for test_expect_failure to see what
existing issues to tackle ;-).