Giuseppe Bilotta <giuseppe.bilotta@xxxxxxxxx> writes:
> diff --git a/t/t4107-apply-ignore-whitespace.sh b/t/t4107-apply-ignore-whitespace.sh
> new file mode 100755
> index 0000000..8e3fce3
> --- /dev/null
> +++ b/t/t4107-apply-ignore-whitespace.sh
> @@ -0,0 +1,149 @@
> ...
> +cat > patch3.patch <<\EOF
> +diff --git a/main.c b/main.c
> +--- a/main.c
> ++++ b/main.c
> +@@ -10,1 +10,1 @@
> + print_int(func(i));
> +EOF
This part is triply troublesome in that:
(1) the payload ends with trailing whitespace which can be eaten by MUAs
(and I almost always use "git am" with --whitespace=fix);
(2) even if we apply your patch correctly, it is very unobvious that you
are testing the case where the line has an unwanted trailing
whitespace; and
(3) a hunk without any added/deleted lines is an obviously artificial
test input that would not appear in real life, something one would
never think of doing unless one knows how "git apply" internally
works. It makes the test too knowledgable about the implementation.
You can fix the first two issues by doing:
sed -e 's/Z/ /g' >patch3.patch <<\EOF
...
+Z print_int(func(i));Z
EOF
to make invisible SP stand out more for the benefit of people reading the
test script (I know you did not have leading SP before HT in yours, but
the above illustrates the visibility issues). For other tests with test
vector patches, visibility of whitespace is not much an issue, but this
script is _all about_ whitespace, so anything that clarifies what is going
on better would help.
> +test_expect_success "S = patch1" \
> + 'git-apply patch1.patch'
> +
> +test_expect_failure "F = patch2" \
> + 'git-apply patch2.patch'
Please say
test_expect_success "whitespace corrupted patch does not apply" '
test_must_fail git apply patch2.patch
'
instead.
"test_expect_failure" is a declaration that the command being tested (in
this case "git apply") is faulty. It also is a request for somebody
interested to later fix it (again, in this case "git apply") to make this
test pass.
But you do _not_ want this to pass without an explicit ignore option, so
test_expect_failure is inappropriate here.
--
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