Re: [PATCH 1/2] mingw: verify that paths are not mistaken for remote nicknames

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: May 29, 2017, 21:59 UTC
Message-ID: <bc145b9d-96cc-325a-8df3-af87d23c8727@ramsayjones.plus.com>
In-Reply-To: <34029b29-9f65-78c0-c2ed-649b179a79ac@kdbg.org>

On 29/05/17 22:02, Johannes Sixt wrote:

Show 32 quoted lines

> Am 29.05.2017 um 22:40 schrieb Ævar Arnfjörð Bjarmason:
>> On Mon, May 29, 2017 at 10:25 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>>> diff --git a/t/t5580-clone-push-unc.sh b/t/t5580-clone-push-unc.sh
>>> index b195f71ea9..fd719a209e 100755
>>> --- a/t/t5580-clone-push-unc.sh
>>> +++ b/t/t5580-clone-push-unc.sh
>>> @@ -1,13 +1,19 @@
>>>   #!/bin/sh
>>>
>>> -test_description='various UNC path tests (Windows-only)'
>>> +test_description='various Windows-only path tests'
>>>   . ./test-lib.sh
>>>
>>>   if ! test_have_prereq MINGW; then
>>> -       skip_all='skipping UNC path tests, requires Windows'
>>> +       skip_all='skipping Windows-only path tests'
>>>          test_done
>>>   fi
>>>
>>> +test_expect_failure 'remote nick cannot contain backslashes' '
>>> +       BACKSLASHED="$(pwd | tr / \\\\)" &&
>>> +       git ls-remote "$BACKSLASHED" >out 2>err &&
>>> +       ! grep "unable to access" err
>>> +'
>>
>> Doesn't this need test_i18ngrep?:
> 
> Good catch! It would be this one in warn_on_inaccessible:
> 
>>      wrapper.c:581:  warning_errno(_("unable to access '%s'"), path);
> 
> But actually, I'm more worried about the unholy mix of one-test-first-then-skip_all-later that occurs in this test script (I do not mean the skip_all that is visible in the context, there are others later). I think there was some buzz recently that prove only understands a summary line that reads "1..0", but here we would see "1..1". What to do? Reorganize the test script? Dscho, any ideas?

See commit c7018be509 ("test: allow skipping the remainder", 18-05-2017) which is currently merged to the 'next' branch (merge 03b8a61e47 of the 'jc/skip-test-in-the-middle' branch).

(see also http://testanything.org)

If you look at http://testanything.org//tap-specification.html, it shows that you are allowed to annotate a plan of '1..0' with a SKIP directive to explain why no tests in a file were run. However, a plan with '1..n' (for any n > 0) must not have any annotation. (Back in 2012, when I wrote commit bf4b721932, I found much better documentation than the above!)

So, after commit c7018be509, you can now use the 'skip_all' facility after having run some tests; it now converts that into an 'SKIP comment' just before the test plan, effectively skipping the remainder of the tests in the file. (since we are using a 'trailing plan', and have thus not declared how many tests we had intended to run, we can output an accurate plan).

ATB, Ramsay Jones