| 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
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
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
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)