Patchwork [02,of,11,STABLE] tests: add "(glob)" to paths in test-run-tests.t for Windows

login
register
mail settings
Submitter Mads Kiilerich
Date Oct. 20, 2014, 1:54 p.m.
Message ID <54451411.2050305@kiilerich.com>
Download mbox | patch
Permalink /patch/6427/
State Not Applicable
Headers show

Comments

Mads Kiilerich - Oct. 20, 2014, 1:54 p.m.
On 10/20/2014 03:26 PM, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1413810488 -32400
> #      Mon Oct 20 22:08:08 2014 +0900
> # Branch stable
> # Node ID 8ffac97b85a5b2888f67e9b9a4d0cada7171fc76
> # Parent  ef382dbe8c20ee19e984ed26c8c46f93e50bf229
> tests: add "(glob)" to paths in test-run-tests.t for Windows

So much tedious work. How about something like

              r.append((re.escape(self._testtmp), '$TESTTMP'))

?

/Mads
Katsunori FUJIWARA - Oct. 23, 2014, 5:53 p.m.
At Mon, 20 Oct 2014 15:54:25 +0200,
Mads Kiilerich wrote:
> 
> On 10/20/2014 03:26 PM, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1413810488 -32400
> > #      Mon Oct 20 22:08:08 2014 +0900
> > # Branch stable
> > # Node ID 8ffac97b85a5b2888f67e9b9a4d0cada7171fc76
> > # Parent  ef382dbe8c20ee19e984ed26c8c46f93e50bf229
> > tests: add "(glob)" to paths in test-run-tests.t for Windows
> 
> So much tedious work. How about something like
> 
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -600,7 +600,8 @@ class Test(unittest.TestCase):
>               r.append(
>                   (''.join(c.isalpha() and '[%s%s]' % (c.lower(), 
> c.upper()) or
>                       c in '/\\' and r'[/\\]' or c.isdigit() and c or 
> '\\' + c
> -                    for c in self._testtmp), '$TESTTMP'))
> +                    for c in self._testtmp) + '([-._/a-zA-Z0-9]*)',
> +                 lambda m: '$TESTTMP' + m.group(1).replace('\\', '/')))
>           else:
>               r.append((re.escape(self._testtmp), '$TESTTMP'))
> 
> ?

I also agree to the plan to avoid adding tedious "(glob)".

But at the same time, automatically replacing from '\\' to "/"
prevents tests from examining whether "$TESTTMP" related path contains
"\" correctly or not: for example, tests around 'ui.slash'.

IMHO, something for such tests like " (exact)" directive or so should
be needed.

Then, this kind of changes on "run-tests.py" seems to be too critical
for code freeze period, doesn't it ? :-)

So, what about steps below ?

  1. apply this patch as it is, to pass the test on Windows while code
     freeze period

  2. introduce something to examine output without any conversion
     (like "(exact)"), after 3.2 release

  3. introduce something to make "(glob)" meaningless
     (and test well that "(exact)" works still correctly)

  4. move existing "(glob)" away !
     (this may need adding the checker for meaningless "(glob)" in tests)


> /Mads
> 


----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Mads Kiilerich - Oct. 23, 2014, 10:52 p.m.
On 10/23/2014 07:53 PM, FUJIWARA Katsunori wrote:
> At Mon, 20 Oct 2014 15:54:25 +0200,
> Mads Kiilerich wrote:
>> On 10/20/2014 03:26 PM, FUJIWARA Katsunori wrote:
>>> # HG changeset patch
>>> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
>>> # Date 1413810488 -32400
>>> #      Mon Oct 20 22:08:08 2014 +0900
>>> # Branch stable
>>> # Node ID 8ffac97b85a5b2888f67e9b9a4d0cada7171fc76
>>> # Parent  ef382dbe8c20ee19e984ed26c8c46f93e50bf229
>>> tests: add "(glob)" to paths in test-run-tests.t for Windows
>> So much tedious work. How about something like
>>
>> --- a/tests/run-tests.py
>> +++ b/tests/run-tests.py
>> @@ -600,7 +600,8 @@ class Test(unittest.TestCase):
>>                r.append(
>>                    (''.join(c.isalpha() and '[%s%s]' % (c.lower(),
>> c.upper()) or
>>                        c in '/\\' and r'[/\\]' or c.isdigit() and c or
>> '\\' + c
>> -                    for c in self._testtmp), '$TESTTMP'))
>> +                    for c in self._testtmp) + '([-._/a-zA-Z0-9]*)',
>> +                 lambda m: '$TESTTMP' + m.group(1).replace('\\', '/')))
>>            else:
>>                r.append((re.escape(self._testtmp), '$TESTTMP'))
>>
>> ?
> I also agree to the plan to avoid adding tedious "(glob)".
>
> But at the same time, automatically replacing from '\\' to "/"
> prevents tests from examining whether "$TESTTMP" related path contains
> "\" correctly or not: for example, tests around 'ui.slash'.

ui.slash a weird, incomplete and inconsistent thing. But AFAIK&IIRC it 
mainly applies to how repository paths are shown - and that is by 
definition paths that are relative to the repo root, never absolute 
paths that start with $TESTTMP, and my snippet should thus not make any 
difference for testing ui.slash. Anyway: the few places where we do 
care, we could pipe through sed and strip everything before the last N 
slashes.

(It could be argued that we in all places where we show file system 
paths always should use the native slash. I would rather argue that we 
in all places should use /, also on windows. AFAIK that should work 
everywhere in the drive letter land where Mercurial lives - Mercurial 
can not handle \\ paths anyway.)

> IMHO, something for such tests like " (exact)" directive or so should
> be needed.

I guess that could be tricky to shoehorn into the existing processing. 
But it could be useful.

>    4. move existing "(glob)" away !
>       (this may need adding the checker for meaningless "(glob)" in tests)

How about just making run-tests add a (glob) if that could make the test 
pass?

It would perhaps also be nice to have a (slash) marker instead of 
overloading (glob).

/Mads

Patch

--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -600,7 +600,8 @@  class Test(unittest.TestCase):
              r.append(
                  (''.join(c.isalpha() and '[%s%s]' % (c.lower(), 
c.upper()) or
                      c in '/\\' and r'[/\\]' or c.isdigit() and c or 
'\\' + c
-                    for c in self._testtmp), '$TESTTMP'))
+                    for c in self._testtmp) + '([-._/a-zA-Z0-9]*)',
+                 lambda m: '$TESTTMP' + m.group(1).replace('\\', '/')))
          else: