git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Anthony Ramine" <n.oxyde@gmail.com>,
	"Johannes Sixt" <j6t@kdbg.org>,
	"Adam Dinwoodie" <adam@dinwoodie.org>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v4 8/7] wildmatch test: skip file creation tests on Windows proper
Date: Fri,  5 Jan 2018 22:12:22 +0000	[thread overview]
Message-ID: <20180105221222.28867-1-avarab@gmail.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1801052133380.1337@wbunaarf-fpuvaqryva.tvgsbejvaqbjf.bet>

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 <Johannes.Schindelin@gmx.de>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

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
20171223213012.1962-7-avarab@gmail.com, 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


  reply	other threads:[~2018-01-05 22:12 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-23 21:30 [PATCH 0/6] increase wildmatch test coverage Ævar Arnfjörð Bjarmason
2017-12-23 21:30 ` [PATCH 1/6] wildmatch test: indent with tabs, not spaces Ævar Arnfjörð Bjarmason
2017-12-23 21:30 ` [PATCH 2/6] wildmatch test: use more standard shell style Ævar Arnfjörð Bjarmason
2017-12-23 21:30 ` [PATCH 3/6] wildmatch test: use a paranoia pattern from nul_match() Ævar Arnfjörð Bjarmason
2017-12-23 21:30 ` [PATCH 4/6] wildmatch test: remove dead fnmatch() test code Ævar Arnfjörð Bjarmason
2017-12-23 21:30 ` [PATCH 5/6] wildmatch test: perform all tests under all wildmatch() modes Ævar Arnfjörð Bjarmason
2017-12-23 21:30 ` [PATCH 6/6] wildmatch test: create & test files on disk in addition to in-memory Ævar Arnfjörð Bjarmason
2017-12-24  9:24   ` Johannes Sixt
2017-12-24 11:06     ` Ævar Arnfjörð Bjarmason
2017-12-24 11:51       ` Johannes Sixt
2017-12-25  0:28 ` [PATCH v2 0/7] increase wildmatch test coverage Ævar Arnfjörð Bjarmason
2017-12-28 22:48   ` Junio C Hamano
2017-12-28 23:49     ` Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 " Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 1/7] wildmatch test: indent with tabs, not spaces Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 2/7] wildmatch test: use more standard shell style Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 3/7] wildmatch test: don't try to vertically align our output Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 4/7] wildmatch test: use a paranoia pattern from nul_match() Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 5/7] wildmatch test: remove dead fnmatch() test code Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 6/7] wildmatch test: perform all tests under all wildmatch() modes Ævar Arnfjörð Bjarmason
2017-12-28 23:28   ` [PATCH v3 7/7] wildmatch test: create & test files on disk in addition to in-memory Ævar Arnfjörð Bjarmason
2017-12-29  0:16     ` Ævar Arnfjörð Bjarmason
2017-12-25  0:28 ` [PATCH v2 1/7] wildmatch test: indent with tabs, not spaces Ævar Arnfjörð Bjarmason
2017-12-25  0:28 ` [PATCH v2 2/7] wildmatch test: use more standard shell style Ævar Arnfjörð Bjarmason
2017-12-25  0:28 ` [PATCH v2 3/7] wildmatch test: don't try to vertically align our output Ævar Arnfjörð Bjarmason
2017-12-25  0:28 ` [PATCH v2 4/7] wildmatch test: use a paranoia pattern from nul_match() Ævar Arnfjörð Bjarmason
2017-12-25  0:28 ` [PATCH v2 5/7] wildmatch test: remove dead fnmatch() test code Ævar Arnfjörð Bjarmason
2017-12-25  0:28 ` [PATCH v2 6/7] wildmatch test: perform all tests under all wildmatch() modes Ævar Arnfjörð Bjarmason
2017-12-28 20:28   ` Junio C Hamano
2017-12-25  0:28 ` [PATCH v2 7/7] wildmatch test: create & test files on disk in addition to in-memory Ævar Arnfjörð Bjarmason
2017-12-25  9:26   ` Johannes Sixt
2017-12-27 19:07   ` Junio C Hamano
2018-01-03 13:02   ` Adam Dinwoodie
2018-01-03 13:31     ` Ævar Arnfjörð Bjarmason
2018-01-03 14:41       ` Adam Dinwoodie
2018-01-03 19:14         ` Ævar Arnfjörð Bjarmason
2018-01-04 11:50           ` Adam Dinwoodie
2018-01-04 19:26             ` [PATCH v4 0/7] increase wildmatch test coverage Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 00/10] " Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 01/10] wildmatch test: indent with tabs, not spaces Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 02/10] wildmatch test: use more standard shell style Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 03/10] wildmatch test: don't try to vertically align our output Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 04/10] wildmatch test: use a paranoia pattern from nul_match() Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 05/10] wildmatch test: remove dead fnmatch() test code Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 06/10] wildmatch test: use test_must_fail, not ! for test-wildmatch Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 07/10] wildmatch test: perform all tests under all wildmatch() modes Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 08/10] wildmatch test: create & test files on disk in addition to in-memory Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 09/10] test-lib: add an EXPENSIVE_ON_WINDOWS prerequisite Ævar Arnfjörð Bjarmason
2018-01-30 21:21               ` [PATCH v5 10/10] wildmatch test: mark test as EXPENSIVE_ON_WINDOWS Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 1/7] wildmatch test: indent with tabs, not spaces Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 2/7] wildmatch test: use more standard shell style Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 3/7] wildmatch test: don't try to vertically align our output Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 4/7] wildmatch test: use a paranoia pattern from nul_match() Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 5/7] wildmatch test: remove dead fnmatch() test code Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 6/7] wildmatch test: perform all tests under all wildmatch() modes Ævar Arnfjörð Bjarmason
2018-01-04 19:26             ` [PATCH v4 7/7] wildmatch test: create & test files on disk in addition to in-memory Ævar Arnfjörð Bjarmason
2018-01-05 16:41               ` Johannes Schindelin
2018-01-05 19:08                 ` Ævar Arnfjörð Bjarmason
2018-01-05 20:48                   ` Johannes Schindelin
2018-01-05 22:12                     ` Ævar Arnfjörð Bjarmason [this message]
2018-01-05 23:13                       ` [PATCH v4 8/7] wildmatch test: skip file creation tests on Windows proper Junio C Hamano
2018-01-06 12:51                         ` Johannes Schindelin
2018-01-06 13:32                           ` Ævar Arnfjörð Bjarmason
2018-01-06 20:46                             ` Johannes Schindelin
2018-01-08 12:46                             ` Johannes Schindelin
2018-01-08 18:49                             ` Junio C Hamano
2018-01-07  2:51                           ` Duy Nguyen
2018-01-08 12:25                             ` Johannes Schindelin
2018-01-10  9:07                               ` Duy Nguyen
2018-01-10 10:38                                 ` Adam Dinwoodie
2018-01-10 10:52                                   ` Duy Nguyen
2018-01-10 20:24                                 ` Johannes Schindelin
2018-01-11  9:25                                   ` Duy Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180105221222.28867-1-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=adam@dinwoodie.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=n.oxyde@gmail.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).