Subject: [PATCH v4 8/7] wildmatch test: skip file creation tests on Windows proper

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 5 Jan 2018 22:12:22 +0000

Skip the newly added file creation tests on Windows proper, these
already work under Cygwin, but as that involves a significant
emulation layer the results are different under Windows proper with
MinGW.

Ideally we'd get exhaustive coverage for this area on all platforms,
but having any increase in test coverage anywhere is a net
improvement.

Particularly in this case where there's no reason to suspect (aside
from perhaps odd edge case like \foo meaning C:\foo) that the actual
pattern matching engine will behave differently on Windows. The tests
can't be run due to limitations elsewhere.

The thread starting at
https://public-inbox.org/git/?q=nycvar.QRO.7.76.6.1801051622010.1337%40wbunaarf-fpuvaqryva.tvgsbejvaqbjf.bet
has more details about specific issues under Windows.

Reported-by: Johannes Schindelin <[email-masked]>
Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
---

On Fri, Jan 05 2018, Johannes Schindelin jotted:

> Hi Ævar,
>
> On Fri, 5 Jan 2018, Ævar Arnfjörð Bjarmason wrote:
>
>> On Fri, Jan 05 2018, Johannes Schindelin jotted:
>> 
>> > [...]
>> >
>> > In short: the Unix shell script t3070 manages to write what it thinks is a
>> > file called 'foo*', but Git only sees 'foo<some-undisplayable-character>'.
>> >
>> > I tried to address this problem with this patch:
>> 
>> ...I don't see any particular value in trying to do these full roundtrip
>> tests on platforms like Windows. Perhaps we should just do these on a
>> whitelist of POSIX systems for now, and leave expanding that list to
>> some future step.
>
> I don't think so. Windows is already handled as a second-class citizen, as
> if nobody developed on it. As a consequence, only very few of the
> gazillions of Windows developers... develop Git. We could worsify the
> situation, of course, but why? Shouldn't we at least pretend to try the
> opposite?

I don't think we should never test for this on MinGW, but given the
increase in test coverage, and not making perfect the enemy of the
good I think (as explained in the commit message above) that we're
better off *starting* with just disabling these tests under MinGW, and
then fixing that platform later.

> [...]
> That's all good and dandy, but what about regressions? I know how much I
> will curse in your vague direction when I encounter the next
> wildmatch-related bug in, say, half a year and have to wade through the
> jungle of unintuitive tests in t3070.

If we have a new wildmatch-related bug we'll be a lot better off with
exhaustive test coverage, even if we can only run those tests on
*nix-like platforms.

> Can't we do a lot better than this? Shouldn't it be a lot more obvious
> what the heck went wrong when running t3070 with -i -v -x?

I had something closer to that in v1 in
[email-masked], but trying again I didn't find
a good way to compromise between -x readability and the entire patch
basically not being the copy/pasted code all over again, and I think
e.g. doing string interpolation into the test code would be even
nastier.

 t/t3070-wildmatch.sh | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
index f606f91acb..50a53e7a62 100755
--- a/t/t3070-wildmatch.sh
+++ b/t/t3070-wildmatch.sh
@@ -7,6 +7,14 @@ test_description='wildmatch tests'
 create_test_file() {
 	file=$1
 
+	# These tests limp along under Cygwin, but the failures on
+	# native Windows are still too many. Skip file tests there
+	# until they're sorted out.
+	if test_have_prereq MINGW
+	then
+		return 1
+	fi
+
 	case $file in
 	# `touch .` will succeed but obviously not do what we intend
 	# here.
@@ -28,7 +36,7 @@ create_test_file() {
 	*/)
 		return 1
 		;;
-	# On Windows, \ in paths is silently converted to /, which
+	# On Cygwin, \ in paths is silently converted to /, which
 	# would result in the "touch" below working, but the test
 	# itself failing. See 6fd1106aa4 ("t3700: Skip a test with
 	# backslashes in pathspec", 2009-03-13) for prior art and
-- 
2.15.1.424.g9478a66081