Re: [PATCH] Fix failing test t3700-add.sh
flat view
From: Junio C Hamano <hidden>
Date: 2016-07-29 16:49:42
Jeff King [off-list ref] 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.quoted
+ 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.