Patchwork [1,of,4,V2] run-tests: line do match on windows when only path sep do not match

login
register
mail settings
Submitter Simon Heimberg
Date July 18, 2013, 1:34 a.m.
Message ID <d7b95f96248521d1c21c.1374111295@lapsi.heimberg.home>
Download mbox | patch
Permalink /patch/1928/
State Rejected
Headers show

Comments

Simon Heimberg - July 18, 2013, 1:34 a.m.
# HG changeset patch
# User Simon Heimberg <simohe@besonet.ch>
# Date 1374102389 -7200
# Node ID d7b95f96248521d1c21cb7456e07ffb3366cc179
# Parent  43291885b7d299616045efc493adaa9038925545
run-tests: line do match on windows when only path sep do not match

Instead of failing the test when \ instead of / is returned, only print a
warning (suggesting to append glob to output).
Like this writing test output is less delicate. Failures at buildbot just
because of paths not matching on windows do not occur anymore. But we do not
loose any information. We can fix the expected output by reading the test
result.

Output example:
...
Info, missing glob in test test-example.t (after line 5): moving x/y/z
.....
Matt Mackall - July 19, 2013, 1:06 a.m.
On Thu, 2013-07-18 at 03:34 +0200, Simon Heimberg wrote:
> # HG changeset patch
> # User Simon Heimberg <simohe@besonet.ch>
> # Date 1374102389 -7200
> # Node ID d7b95f96248521d1c21cb7456e07ffb3366cc179
> # Parent  43291885b7d299616045efc493adaa9038925545
> run-tests: line do match on windows when only path sep do not match
> 
> Instead of failing the test when \ instead of / is returned, only print a
> warning (suggesting to append glob to output).

This patch is simply wrong. Tests should fail when the output is
incorrect. The ONLY argument I will even entertain here is 'we are
giving up on matching path separators on Windows, and thus there's no
correctness issue here now', but that probably needs to be a discussion
outside of this. Otherwise this patch is a step backwards, and
intentional steps backwards are not allowed.

Perhaps there's a future patch in this series that undoes this. If so,
these patches need reordering.

But I still think the best way forward is to further improve the tests
in check-code. This way Unix devs can get their tests right before
submission without ever running on Windows. Most of the common separator
cases are in fact already tested and the four remaining extra glob
messages are caused by false positives.

Patch

diff -r 43291885b7d2 -r d7b95f962485 tests/run-tests.py
--- a/tests/run-tests.py	Don Jul 18 01:03:00 2013 +0200
+++ b/tests/run-tests.py	Don Jul 18 01:06:29 2013 +0200
@@ -632,6 +632,8 @@ 
         if (el.endswith(" (re)\n") and rematch(el[:-6], l) or
             el.endswith(" (glob)\n") and globmatch(el[:-8], l)):
             return True
+        elif os.altsep and l.replace('\\', '/') == el:
+            return 'slash'
     return False
 
 def tsttest(test, wd, options, replacements, env):
@@ -787,8 +789,12 @@ 
             if pos in expected and expected[pos]:
                 el = expected[pos].pop(0)
 
-            if linematch(el, lout):
+            r = linematch(el, lout)
+            if r:
                 postout.append("  " + el)
+                if r == 'slash':
+                    log('\nInfo, missing glob in test %s (after line %d): %s'
+                        % (test, pos, el))
             else:
                 if needescape(lout):
                     lout = stringescape(lout.rstrip('\n')) + " (esc)\n"
diff -r 43291885b7d2 -r d7b95f962485 tests/test-run-tests.py
--- /dev/null	Don Jan 01 00:00:00 1970 +0000
+++ b/tests/test-run-tests.py	Don Jul 18 01:06:29 2013 +0200
@@ -0,0 +1,87 @@ 
+r"""test line matching with some failing examples and some which warn
+
+run-test.t only checks positive matches and can not see warnings
+(both by design)
+
+does it generally work?
+        >>> lm('h*e (glob)\n', 'here\n')
+        True
+        >>> try: lm('a\n','a')
+        ... except AssertionError, ex: print ex
+        missing newline
+        >>> try: lm('single backslash\n', 'single \backslash\n')
+        ... except AssertionError, ex: print ex
+        single backslash or unknown char
+"""
+
+
+import doctest, os, re
+run_tests = __import__('run-tests')
+
+linematch = run_tests.linematch
+
+def lm(expected, output):
+    "check if output matches expected"
+    assert expected.endswith('\n') and output.endswith('\n'), 'missing newline'
+    assert not re.search(r'[^ a-z\\/\n()*?]', expected + output), \
+           'single backslash or unknown char'
+    match = linematch(expected, output)
+    if not match:
+        return False
+    elif isinstance(match, str):
+        return 'True + warn(%s)' % match
+    else:
+        return True
+
+def wintests():
+    r"""test matching like running on windows
+
+    enable windows matching on any os
+        >>> _osaltsep = os.altsep
+        >>> os.altsep = True
+
+    valid match on windows
+        >>> lm('g/a*/d (glob)\n', 'g\\abc/d\n')
+        True
+
+    direct match, warn
+        >>> lm('g/b (glob)\n', 'g/b\n')
+        <BLANKLINE>
+        Info, unnecessary glob: g/b (glob)
+        True
+
+    slash matches backslash, but warn
+        >>> lm('/g/c/d/fg\n', '\\g\\c\\d/fg\n')
+        'True + warn(slash)'
+
+    restore os.altsep
+        >>> os.altsep = _osaltsep
+    """
+    pass
+
+def otherostests():
+    r"""test matching like running on non-windows os
+
+    disable windows matching on any os
+        >>> _osaltsep = os.altsep
+        >>> os.altsep = False
+
+    backslash does not match now
+        >>> lm('h/a* (glob)\n', 'h\\ab\n')
+        False
+
+    direct match only warns on windows
+        >>> lm('h/b (glob)\n', 'h/b\n')
+        True
+
+    slash does not match backslash
+        >>> lm('h/c/df/g/\n', '\\h/c\\df/g\\\n')
+        False
+
+    restore os.altsep
+        >>> os.altsep = _osaltsep
+    """
+    pass
+
+if __name__ == '__main__':
+    doctest.testmod()