Re: [PATCH v2 7/7] wildmatch test: create & test files on disk in addition to in-memory

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Jan 03 2018, Adam Dinwoodie jotted:

> On Wednesday 03 January 2018 at 02:31 pm +0100, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Wed, Jan 03 2018, Adam Dinwoodie jotted:
>>
>> > On Monday 25 December 2017 at 12:28 am +0000, Ævar Arnfjörð Bjarmason wrote:
>> >> There has never been any full roundtrip testing of what git-ls-files
>> >> and other functions that use wildmatch() actually do, rather we've
>> >> been satisfied with just testing the underlying C function.
>> >>
>> >> Due to git-ls-files and friends having their own codepaths before they
>> >> call wildmatch() there's sometimes differences in the behavior between
>> >> the two, and even when we test for those (as with
>> >> 9e4e8a64c2 ("pathspec: die on empty strings as pathspec", 2017-06-06))
>> >> there was no one place where you can review how these two modes
>> >> differ.
>> >>
>> >> Now there is. We now attempt to create a file called $haystack and
>> >> match $needle against it for each pair of $needle and $haystack that
>> >> we were passing to test-wildmatch.
>> >>
>> >> If we can't create the file we skip the test. This ensures that we can
>> >> run this on all platforms and not maintain some infinitely growing
>> >> whitelist of e.g. platforms that don't support certain characters in
>> >> filenames.
>> >>
>> >> As a result of doing this we can now see the cases where these two
>> >> ways of testing wildmatch differ:
>> >>
>> >>  * Creating a file called 'a[]b' and running ls-files 'a[]b' will show
>> >>    that file, but wildmatch("a[]b", "a[]b") will not match
>> >>
>> >>  * wildmatch() won't match a file called \ against \, but ls-files
>> >>    will.
>> >>
>> >>  * `git --glob-pathspecs ls-files 'foo**'` will match a file
>> >>    'foo/bba/arr', but wildmatch won't, however pathmatch will.
>> >>
>> >>    This seems like a bug to me, the two are otherwise equivalent as
>> >>    these tests show.
>> >>
>> >> This also reveals the case discussed in 9e4e8a64c2 above, where '' is
>> >> now an error as far as ls-files is concerned, but wildmatch() itself
>> >> happily accepts it.
>> >>
>> >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> >
>> > I'm seeing this test script failing on the pu branch as a result of this
>> > commit when building on Cygwin.  Specifically, the test fails at
>> > 9d45e1ca4 ("Merge branch 'bw/oidmap-autoinit' into pu", 2017-12-28), and
>> > bisecting points the blame at 2ee0c785a ("wildmatch test: create & test
>> > files on disk in addition to in-memory", 2017-12-25).
>> >
>> > I've copied the verbose error output for the first error below, and
>> > uploaded the full output, including verbose and trace output for the
>> > unexpectedly failing tests, at [0].  (With 42 failures among 1512 tests,
>> > there's a lot of it, so I didn't want to include it in an email.)
>>
>> Does the fixup above in <878tdm8k2d.fsf@evledraar.gmail.com> work for
>> you, i.e. changing $10 in the script to ${10}?
>
> This fixes some but not all of the failures: I'm now down from 42 to 24
> failures.
>
> Updated verbose test output is at
> https://gist.github.com/me-and/04443bcb00e12436f0eacce079b56d02

Thanks lot, looking through our own commit logs I believe the rest
should be fixed by this (prior art in 6fd1106aa4), it would be great if
you could test it, I don't have access to a Windows machine:

    diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
    index f985139b6f..5838fcb77d 100755
    --- a/t/t3070-wildmatch.sh
    +++ b/t/t3070-wildmatch.sh
    @@ -23,6 +23,15 @@ create_test_file() {
            *//*)
                    return 1
                    ;;
    +       # On Windows, \ in paths is silently converted to /, which
    +       # would result in the "touch" below working, but the test
    +       # itself failing.
    +       *\\*)
    +               if ! test_have_prereq BSLASHPSPEC
    +               then
    +                       return 1
    +               fi
    +               ;;
            # When testing the difference between foo/bar and foo/bar/ we
            # can't test the latter.
            */)