Patchwork [2,of,4] run-tests: allow to put conditions and comments in expected output

login
register
mail settings
Submitter Simon Heimberg
Date Feb. 4, 2014, 12:14 a.m.
Message ID <94c2bb6ad0656684bdc9.1391472858@lapsi.heimberg.home>
Download mbox | patch
Permalink /patch/3455/
State Changes Requested
Headers show

Comments

Simon Heimberg - Feb. 4, 2014, 12:14 a.m.
# HG changeset patch
# User Simon Heimberg <simohe@besonet.ch>
# Date 1391470011 -3600
#      Tue Feb 04 00:26:51 2014 +0100
# Branch stable
# Node ID 94c2bb6ad0656684bdc90bb52a1a205a5ad151e3
# Parent  ab6f4d067f28326dfdd5c352491c92148f0ee160
run-tests: allow to put conditions and comments in expected output

Writing the condition in the output will reduce the copied output lines in
some tests (see next patch).
To see output containing comments or conditions, see near the changes in
test-run-tests.t.

Limitation: The return code must follow the last output line directly. So this
does not work:
  $ echo output; false
#if true
  output
#else
  something else
#endif
  [1]

It must be written like this:
  $ echo output; false
#if true
  output
  [1]
#else
  something else
  [1]
#endif
Mads Kiilerich - Feb. 4, 2014, 3:42 p.m.
On 02/04/2014 01:14 AM, Simon Heimberg wrote:
> # HG changeset patch
> # User Simon Heimberg <simohe@besonet.ch>
> # Date 1391470011 -3600
> #      Tue Feb 04 00:26:51 2014 +0100
> # Branch stable
> # Node ID 94c2bb6ad0656684bdc90bb52a1a205a5ad151e3
> # Parent  ab6f4d067f28326dfdd5c352491c92148f0ee160
> run-tests: allow to put conditions and comments in expected output
...
> It must be written like this:
>    $ echo output; false
> #if true
>    output
>    [1]
> #else
>    something else
>    [1]
> #endif

What will happen when using -i ?

Currently the command and its output is obvious, and -i should just do 
the right thing in the "code" path that is tested.

How will it work with this new conditional style?

If "not so well": Is the increased complexity and maintenance overhead 
worth it?

For the file://drive/path handling: Wouldn't it be simpler to add a 
replacement hack to run-tests?

/Mads
Simon Heimberg - Feb. 10, 2014, 3:46 p.m.
Shortly: Right, this series is not yet ready. At least a small change is 
necessary.
Details below

Am 04.02.2014 16:42, schrieb Mads Kiilerich:
> On 02/04/2014 01:14 AM, Simon Heimberg wrote:
>> # HG changeset patch
>> # User Simon Heimberg <simohe@besonet.ch>
>> # Date 1391470011 -3600
>> #      Tue Feb 04 00:26:51 2014 +0100
>> # Branch stable
>> # Node ID 94c2bb6ad0656684bdc90bb52a1a205a5ad151e3
>> # Parent  ab6f4d067f28326dfdd5c352491c92148f0ee160
>> run-tests: allow to put conditions and comments in expected output
> ...
>> It must be written like this:
>>    $ echo output; false
>> #if true
>>    output
>>    [1]
>> #else
>>    something else
>>    [1]
>> #endif
>
> What will happen when using -i ?
>
> Currently the command and its output is obvious, and -i should just do 
> the right thing in the "code" path that is tested.
>
> How will it work with this new conditional style?

"run-tests.py -i " still behaves nicely when lines have changed. (And of 
course also on any changes on commands outside of a conditional bloc run 
nicely.)

There are some problems when lines are inserted or deleted. They are the 
same kind as when matching (glob/re) is used. See at end of [1] for a 
diff output difficult to read because glob is not matching on the 
expected line. Matching and mixed conditions currently depend on fix 
line numbers. This could probably be improved generally.

example:
   $ printf $DEMO
   l1
   #if true
   l2
   l3
   #else
   L2
   L3
   #endif
   l4
   $ echo xx
   xx

When there is a line before l1, the conditions are moved up by 1 like this:
example:
   $ printf $DEMO
   l0_new
   #if true
   l1
   l2
   #else
   L2
   L3
   #endif
   l3
   $ echo xx
   l4
   xx


Of course l4 may not be moved after the next command. This can be fixed 
by moving one line. Then l4 is directly after l3. The test would run 
smoothly after "-i" and accepting, as long as the condition is the same. 
So the test needs manual editing. Inserting some empty lines or deleting 
some lines will do the trick to let "-i" do the right thing.
(Manual editing is also necessary also when this test would be written 
with the command repeated in the conditional part. The suite will never 
be able to rewrite a non active case.)


Will this behaviour be good enough?


[1] 
http://www.selenic.com/pipermail/mercurial-devel/2014-January/055771.html

>
> If "not so well": Is the increased complexity and maintenance overhead 
> worth it?
>
> For the file://drive/path handling: Wouldn't it be simpler to add a 
> replacement hack to run-tests?

I do not know what a replacement hack is, sorry.

There are for sure different methods for matching. I suggested some in a 
mail [2].
My (new) favorite of them is to match the 3rd "/" by a "*":
    large: largefile 7f7097b041ccf68cc5561e9600da4655d21c6d18 not 
available from file:/*/$TESTTMP/mirror (glob)
instead of
    large: largefile 7f7097b041ccf68cc5561e9600da4655d21c6d18 not 
available from file://$TESTTMP/mirror (glob)
The match is not exact, it would match any number of any character 
between / and /. But this will probably never happen.
I will send this first for fixing the test failures on windows. This 
case inspired the idea for this patch series, but it is not the main use 
case. Less duplicate code as shown in patch 3 is more important.


[2] 
http://www.selenic.com/pipermail/mercurial-devel/2014-January/055770.html

>
> /Mads
Mads Kiilerich - Feb. 22, 2014, 7:22 p.m.
On 02/10/2014 04:46 PM, Simon Heimberg wrote:
>> For the file://drive/path handling: Wouldn't it be simpler to add a 
>> replacement hack to run-tests?
>
> I do not know what a replacement hack is, sorry.

...
     replacements = [
         (r':%s\b' % port, ':$HGPORT'),
         (r':%s\b' % (port + 1), ':$HGPORT1'),
         (r':%s\b' % (port + 2), ':$HGPORT2'),
         ]
     if os.name == 'nt':
         replacements.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 testtmp), '$TESTTMP'))
     else:
         replacements.append((re.escape(testtmp), '$TESTTMP'))
...

/Mads

Patch

diff -r ab6f4d067f28 -r 94c2bb6ad065 tests/run-tests.py
--- a/tests/run-tests.py	Mon Feb 03 22:20:52 2014 +0100
+++ b/tests/run-tests.py	Tue Feb 04 00:26:51 2014 +0100
@@ -655,14 +655,11 @@ 
 
     # After we run the shell script, we re-unify the script output
     # with non-active parts of the source, with synchronization by our
-    # SALT line number markers. The after table contains the
-    # non-active components, ordered by line number
-    after = {}
+    # SALT line number markers. The alloutput table contains the
+    # non-active components and the expected output, ordered by line number
+    alloutput = {}
     pos = prepos = -1
 
-    # Expected shellscript output
-    expected = {}
-
     # We keep track of whether or not we're in a Python block so we
     # can generate the surrounding doctest magic
     inpython = False
@@ -685,10 +682,7 @@ 
         return ret == 0
 
     def add_output(pos, text, isexpected=False):
-        if isexpected:
-            expected.setdefault(pos, []).append(text)
-        else:
-            after.setdefault(pos, []).append(text)
+        alloutput.setdefault(pos, []).append((isexpected, text))
 
     f = open(test)
     t = f.readlines()
@@ -795,9 +789,12 @@ 
                 lout += ' (no-eol)\n'
 
             # find the expected output at the current position
-            el = None
-            if pos in expected and expected[pos]:
-                el = expected[pos].pop(0)
+            l =  alloutput.get(pos, None)
+            while l:
+                isexpected, el = l.pop(0)
+                if isexpected:
+                    break
+                postout.append(el) # add non-active component
 
             r = linematch(el, lout)
             if isinstance(r, str):
@@ -823,13 +820,17 @@ 
             ret = int(lcmd.split()[1])
             if ret != 0:
                 postout.append("  [%s]\n" % ret)
-            if pos in after:
+            for isexpected, el in alloutput.get(pos, []):
                 # merge in non-active test bits
-                postout += after.pop(pos)
+                if not isexpected:
+                    postout.append(el)
+            alloutput[pos] = [] # clear, doctest comes here twice with same pos
             pos = int(lcmd.split()[0])
 
-    if pos in after:
-        postout += after.pop(pos)
+    # add remaining non-active components
+    for isexpected, el in alloutput.get(pos, []):
+        if not isexpected:
+            postout.append(el)
 
     return exitcode, postout
 
diff -r ab6f4d067f28 -r 94c2bb6ad065 tests/test-run-tests.t
--- a/tests/test-run-tests.t	Mon Feb 03 22:20:52 2014 +0100
+++ b/tests/test-run-tests.t	Tue Feb 04 00:26:51 2014 +0100
@@ -31,6 +31,7 @@ 
   ...     print c
   x
   y
+comment in the output
   z
   >>> print
   
@@ -115,6 +116,14 @@ 
   tested
 #endif
 
+  $ printf a\\nB\\nc
+  a
+#if true
+  B
+#endif
+and some comment
+  c (no-eol)
+
 Exit code:
 
   $ (exit 1)