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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 6, 2018, 12:51 UTC
Message-ID: <nycvar.QRO.7.76.6.1801061337020.1337@wbunaarf-fpuvaqryva.tvgsbejvaqbjf.bet>
In-Reply-To: <xmqqzi5raogu.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Fri, 5 Jan 2018, Junio C Hamano wrote:

Show 25 quoted lines

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
> 
> > 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.
> > ...
> > 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
> > +
> 
> That looks to be a nuclear option.

Indeed:

	# still have 84 known breakage(s)
	# failed 52 among remaining 1428 test(s)
	1..1512

Let's just switch it off completely because 52 of those 1512 test cases break only on Windows? Pretty heavy-handed.

But do read on.

> For now it may be suffice, but somehow it feels that it goes directly
> against Dscho's wish to treat (or pretend to treat) Windows as
> first-class citizens and/or just like other platforms.

It not only goes against my wish to treat Windows as a normal citizen instead of like Rey's parents, it also goes against my wish to have a focused and meaningful test suite. Nobody likes to run tests that take too long. And look at this:

	...
	ok 1511 - ipathmatch: match 'Z' '[Z-y]'
	ok 1512 - ipathmatch(ls): match '[Z-y]' 'Z'
	# still have 84 known breakage(s)
	# failed 52 among remaining 1428 test(s)
	1..1512

	real    5m51.432s
	user    0m33.986s
	sys     2m13.162s

Yep. It takes *over eight minutes*.

And this is a *fast* machine.

Why? Because of the completely overboard testing of all kinds of *potential* breakages *at some point* in the future.

I understand that Ævar wants to increase test coverage. I am sympathetic to this cause.

But I completely disagree that increasing the test coverage beyond reason is a good thing. Tests *can* take too long, and they do, in practice, and the results are always problematic: every developer who has to deal with test suites that take too long... does not run them. As simple as that. And then you have *zero* test coverage. Pretty stupid, eh? And contrary to your intentions, too.

So yes, I think that it has been proven beyond any doubt that t3070 takes *waaaaaaaaaay* too long on Windows, for dubitable benefit.

I could see a reasonable compromise where

- an extensive test matrix is hidden behind an EXPENSIVE_WILDMATCH prereq,

- the test matrix would be written in a much more understandable way, i.e.
  using English words rather than "1"s. If need be, there could be blocks
  ("test this with case-sensitive matching", "skip case-sensitive matching",
  you get the idea),

- these magic skippings of certain test cases (where the (non-traced)
  `create_test_file()` function returns 1 for certain things) still would
  need to go away, in favor probably of prereqs and/or blocks where flags
  are set accordingly in a preamble,

- by default, i.e. without the EXPENSIVE_WILDMATCH prereq, it should test
  a *tiny* subset that is indicative of the most likely bugs.

As it is, I like 8/7 in the present form for all the wrong reasons: it protects me from the damage a full t3070 would do to me.

Ciao, Dscho