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:
> @@ -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
--
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
