Re: [PATCH] Fix t3701 if core.filemode disabled
From: Jeff King <hidden>
Date: 2016-06-15 22:44:37
On Tue, May 20, 2008 at 11:59:32PM +0200, Alex Riesen wrote:
Sure. I have explicitely test for core.filemode=false, because some older setups (where git init did not set it) don't have the setting and git config core.filemode reports nothing.
I don't think that is relevant here; this is a test script, and as such is testing the results of the current version of git-init (from when we git-init the trash directory). That being said, I think the test you have is perfectly fine, and the patch is correct. And I am OK with the patch as-is, though I have a few style nits:
quoted hunk ↗ jump to hunk
@@ -65,6 +65,7 @@ test_expect_success 'revert works (commit)' ' git add -i </dev/null >output && grep "unchanged *+3/-0 file" output ' +if test "$(git config core.filemode)" != false ; then test_expect_success 'patch does not affect mode' ' git reset --hard &&@@ -84,5 +85,6 @@ test_expect_success 'stage mode but not hunk' ' git diff file | grep "+content" ' +fi
1. It should be $(git config --bool core.filemode). As it happens,
git-init always uses the word "false" so this works OK, but it is
probably better to model good behavior and to be more robust.
2. It's a little hard to see which tests are affected. I would have done
something more like:
if test "$(git config --bool core.filemode)" = true
test_filemode=
else
test_filemode=:
fi
$test_filemode test_expect_success ...
But maybe that is just overengineering.
3. Usually when we skip tests we do something like
say 'skipping filemode tests (filesystem does not properly support modes')
-Peff