On Mon, May 18, 2015 at 11:30 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> Stefan Beller <sbeller@xxxxxxxxxx> writes: >> >>> When looking through existing tests to point out good style I came across >>> t1020, which has a test commented out and the comment wasn't helping me >>> either of what the test should accomplish in the future. The code of the >>> test is the same as the test before except setting GIT_DIR=. explicitly, >>> so it did not ring a bell for me as well. >> >> I think this one should be clear, especially if you did notice the >> one that sets GIT_DIR=. explicitly. It is saying that "git show -s >> HEAD" inside the bare repository should be intelligent enough to >> realize that it is inside bare repository (hence HEAD cannot be a >> file in the working tree); the user's asking for "HEAD" therefore >> must mean "the tip commit", and never "(by default the tip commit) >> filtered to the pathspec HEAD". > > I forgot to conclude that sentence: "... and it should be able to do > so without the help of an explicit GIT_DIR=." Not sure I follow you there. So currently there are 2 tests having the same name, and doing exactly the same thing, apart from setting the GIT_DIR and one of them being commented out. So I don't understand, what should be tested *additionally* in the second test, where GIT_DIR is not set. (The naming doesn't hint at testing explicit GIT_DIR, so we test the 'no file/rev ambiguity check inside a bare repo' again.) Maybe this diff is better for review: diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh index 2edb4f2..4470ede 100755 --- a/t/t1020-subdirectory.sh +++ b/t/t1020-subdirectory.sh @@ -163,24 +163,15 @@ test_expect_success 'no file/rev ambiguity check inside .git' ' ' test_expect_success 'no file/rev ambiguity check inside a bare repo' ' + test_when_finished "rm -fr foo.git" && git clone -s --bare .git foo.git && ( cd foo.git && GIT_DIR=. git show -s HEAD ) ' -# This still does not work as it should... -: test_expect_success 'no file/rev ambiguity check inside a bare repo' ' - git clone -s --bare .git foo.git && - ( - cd foo.git && - git show -s HEAD - ) -' - test_expect_success SYMLINKS 'detection should not be fooled by a symlink' ' - rm -fr foo.git && git clone -s .git another && ln -s another yetanother && ( -- 2.4.0.194.gc518059 > >> If it does not still work, shouldn't it be marked as >> test_expect_failure instead of being commented out? When commenting in, it doesn't work because git clone -s --bare .git foo.git fails, as foo.git is already there. That problem removed it succeeds. -- 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