Junio C Hamano wrote:
Mark Mentovai <mark@xxxxxxxxxxxx> writes:
+test_file_mode_common() {
Unlike the two callers, this lacks SP before "()".
I would have expected that "no such file" would be expressed with
mode 000000 (taken from "git diff --raw" for a removal/creation
patch), but an empty string works just as well. The same comment
about requiring 0-prefix before 644 and 755 applies here, but as
long as it is done consistently, I wouldn't complain too loudly ;-)
No, it's fine. I'll switch to the 6-digit form.
+ test -n "$1" && test_grep "^10$1 " "$2" || test_must_be_empty "$2"
This requires attention anyway. Reading it back, it's buggy: if there's a
mode in $1, but the file $2 is erroneously empty, test_grep will evaluate
false and evaluation will advance to test_must_be_empty, which will cause
the whole expression to evaluate true. This construct only works properly
as an if-then-else when the "then" clause can't fail. I will revise this.
+test_expect_success 'git apply restores file modes (change_x_to_notx)' '
+ test_config core.fileMode false &&
Wouldn't this and the subsequent tests want to begin with
git reset --hard <commit> &&
to a known good state? We expect that after successfully running
this test piece, for example, the path is removed after the last
patch that removes change-x-to-notx is applied.
Yes, good point. And with just slightly more effort, I can use the same
strategy to eliminate the ordering dependency between the the tests in the
1/2 patch.
Other than that, we seem to have a very good coverage of the
combinations now. Thanks for a thorough work.
Thanks for your thoughtful reviews. v3 incoming shortly.
