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

fredagen den 1 februari 2008 skrev Junio C Hamano:

Robin Rosenberg [off-list ref] writes:

quoted

quoted

quoted

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

Oops. Remove the echo $?. It still fails, i.e. git add succeeds when
it shouldn't. I was double checking it just before sending the patch.

Ah, you found breakages.

I could not quite tell what you meant by these tests with
test_expect_failure, either they were misuse or "currently fails
but they shouldn't".  Coming up with a small reproducible
failure case is 50% of solving the problem.  That's very
appreciated.

In any case, I think the large-ish test framework update patch I
sent tonight should go in very early post 1.5.4 cycle, so plesae
use the new-and-improved test_expect_failure to mark these
reproducible failure cases.  Also, there is no need for hurry
for you to just send test cases without fixes.  When I say I do
not take patches early, I do mean it --- I do not even take _my_
own patches like the one you are following up on.  I've sent
quite a many of them, and I think some are on 'pu' or 'offcuts',
but most are only in the list archive.  If nobody cares deeply
enough about them to test and resend with Tested-by: , I am not
planning to go back to pick them up.

I had those test cases on my machine from my earlier work on absolute
path names so I just ran them with your code and extracted those that
failed and put them into your testsuite instead. That's why I sent them
so early. Read them as comments on your patches, "oh you need to cover
this too". It was simply very convenient for me, and hopefull for you,
to supply them in  patch form.

The reasone my code for absolute path names wasn't re-submitted was
because I had some test cases that didn't pass. I don't really care how
the problem is solved.

quoted

Can we move the default trash one level down for all tests? That
would give us one free level to play with.

I'd rather not.  Most tests do not have to step outside and it
is very handy to debug any breakage they find by always being
able to go there with "cd t/trash".

The change would be to "cd t/trash/repo" instead. Not much different.

The updated get_pathspec() issues a warning message and returns
the result that omits paths outside of the work tree.  It does
not die (and it is intentional, by the way).  The callers that
expect to always receive the same number of paths in the return
value as argv+i they pass to get_pathspec() should be updated to
notice that they got less than they passed in, if they care
about this error condition, and --error-unmatch codepath is one
of them.  I did not touch that in the weatherbaloon patch.

Git add in the current version on an absolute path outside the repo
fails, so I think the updated one should. It really doesn't make sense.

ls-files outside the repo doesn't make sense either, but then the 
current version exits with 0 so I can't make the same reference to 
existing behaviour there. It just might break someone's script.

-- robin