Jeff King <p...@peff.net> writes:
> I was also puzzled why the test fails for you; it does not for me.
> Running the test script as root does make it fail. There are some
> earlier tests which are skipped in this case, which run "git reset
> --hard" with xfoo1 in the index, which cleans it up.
>
>> + echo foo >xfoo &&
>> + chmod 755 xfoo &&
>> + git add --chmod=-x xfoo &&
>> + case "$(git ls-files --stage xfoo)" in
>> + 100644" "*xfoo) echo pass;;
>> + *) echo fail; git ls-files --stage xfoo; (exit 1);;
>
> Here you just pick another name, "xfoo", which does happen to work. But
> it seems like that has the same potential for flakiness if earlier tests
> get adjusted or skipped, since they also use that name.
>
> How about just:
>
> rm -f xfoo1
>
> at the top of the test, which explicitly documents the state we are
> looking for?
That's much more sensible.
> I also wondered if this test, which calls "chmod 755 xfoo1", should be
> marked with the POSIXPERM prerequisite. But I guess since its goal is to
> strip the executable bit, it "works" even on systems where that chmod is
> a noop (the "git add --chmod" doesn't do anything, but one way or the
> other we end up at the end state we expect).
We could make sure --chmod=[-+]x works both ways, which would be
more robust on either type of underlying platform. Something along
the lines of
echo foo >xfoo1 &&
git add --chmod=+x xfoo1 &&
test_mode_in_index 100755 xfoo1 &&
git add --chmod=-x xfoo1 &&
test_mode_in_index 100644 xfoo1
with an obvious addition of a test_mode_in_index helper function as
the same "case $(ls-files -s) in ... esac" pattern appears number of
times.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
