Patchwork run-tests: test result shows when a failed test could not start a server

login
register
mail settings
Submitter Simon Heimberg
Date Jan. 16, 2014, 8:58 p.m.
Message ID <afdcb3c8d7aae39b02ea.1389905920@lapsi.heimberg.home>
Download mbox | patch
Permalink /patch/3361/
State Accepted
Headers show

Comments

Simon Heimberg - Jan. 16, 2014, 8:58 p.m.
# HG changeset patch
# User Simon Heimberg <simohe@besonet.ch>
# Date 1385413246 -3600
# Node ID afdcb3c8d7aae39b02ea208c180e36a2a6602bf0
# Parent  e2068d5d4289fe61f2e5739d62d92fa3c0c2f285
run-tests: test result shows when a failed test could not start a server

Failing to start a server happens regularly, at least on windows buildbot.
Such a failure often has nothing to do with the test, but with the environment.
But half the test output can change because some data is missing. Therefore this
is worth an extended error message.

Detect the server failure in the diff output because it is most reliable
there. Checking the output only does not show if the server failure was
expected.

 Old failure message when server start failed:
Failed test-serve.t: output changed

 New message:
Failed test-serve.t: serve failed and output changed
Simon Heimberg - Jan. 21, 2014, 11:37 a.m.
Simon Heimberg <simohe <at> besonet.ch> writes:
> 
> # HG changeset patch
> # User Simon Heimberg <simohe <at> besonet.ch>
> # Date 1385413246 -3600
> # Node ID afdcb3c8d7aae39b02ea208c180e36a2a6602bf0
> # Parent  e2068d5d4289fe61f2e5739d62d92fa3c0c2f285
> run-tests: test result shows when a failed test could not start a server
> 
> Failing to start a server happens regularly, at least on windows buildbot.
> Such a failure often has nothing to do with the test, but with the 
environment.
> But half the test output can change because some data is missing. 
Therefore this
> is worth an extended error message.
> 
> Detect the server failure in the diff output because it is most reliable
> there. Checking the output only does not show if the server failure was
> expected.
> 
>  Old failure message when server start failed:
> Failed test-serve.t: output changed
> 
>  New message:
> Failed test-serve.t: serve failed and output changed
> 
> diff -r e2068d5d4289 -r afdcb3c8d7aa tests/run-tests.py
> --- a/tests/run-tests.py	Son Nov 17 20:15:28 2013 +0100
> +++ b/tests/run-tests.py	Mon Nov 25 22:00:46 2013 +0100
<snip>

This patch would help to read the test results of buildbot more easily (for 
windows tests). The problem of a failing server start would directly be 
visible in the warning log [1] and in the rss feed [2]. So more important 
failures would not be hidden in the mass anymore.


[1] warnings: 
http://hgbuildbot.kublai.com/builders/Windows%202008%20R2%20hg%20tests/build
s/76/steps/run-tests.py%20%28python2.6%29/logs/warnings%20%2821%29
[2] rss: http://hgbuildbot.kublai.com/rss
Augie Fackler - Feb. 28, 2014, 7:57 p.m.
On Jan 16, 2014, at 3:58 PM, Simon Heimberg <simohe@besonet.ch> wrote:

> # HG changeset patch
> # User Simon Heimberg <simohe@besonet.ch>
> # Date 1385413246 -3600
> # Node ID afdcb3c8d7aae39b02ea208c180e36a2a6602bf0
> # Parent  e2068d5d4289fe61f2e5739d62d92fa3c0c2f285
> run-tests: test result shows when a failed test could not start a server

This looks mostly reasonable to me. Does anyone else have feelings?

> 
> Failing to start a server happens regularly, at least on windows buildbot.
> Such a failure often has nothing to do with the test, but with the environment.
> But half the test output can change because some data is missing. Therefore this
> is worth an extended error message.
> 
> Detect the server failure in the diff output because it is most reliable
> there. Checking the output only does not show if the server failure was
> expected.
> 
> Old failure message when server start failed:
> Failed test-serve.t: output changed
> 
> New message:
> Failed test-serve.t: serve failed and output changed
> 
> diff -r e2068d5d4289 -r afdcb3c8d7aa tests/run-tests.py
> --- a/tests/run-tests.py	Son Nov 17 20:15:28 2013 +0100
> +++ b/tests/run-tests.py	Mon Nov 25 22:00:46 2013 +0100
> @@ -300,8 +300,14 @@
> 
> def showdiff(expected, output, ref, err):
>    print
> +    servefail = False
>    for line in difflib.unified_diff(expected, output, ref, err):
>        sys.stdout.write(line)
> +        if not servefail and line.startswith(
> +                             '+  abort: child process failed to start'):
> +            servefail = True
> +    return {'servefail': servefail}
> +
> 
> verbose = False
> def vlog(*msg):
> @@ -1022,17 +1028,21 @@
>    elif ret == 'timeout':
>        result = fail("timed out", ret)
>    elif out != refout:
> +        info = {}
>        if not options.nodiff:
>            iolock.acquire()
>            if options.view:
>                os.system("%s %s %s" % (options.view, ref, err))
>            else:
> -                showdiff(refout, out, ref, err)
> +                info = showdiff(refout, out, ref, err)
>            iolock.release()
> +        msg = ""
> +        if info.get('servefail'): msg += "serve failed and "
>        if ret:
> -            result = fail("output changed and " + describe(ret), ret)
> +            msg += "output changed and " + describe(ret)
>        else:
> -            result = fail("output changed", ret)
> +            msg += "output changed"
> +        result = fail(msg, ret)
>    elif ret:
>        result = fail(describe(ret), ret)
>    else:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Simon Heimberg - March 1, 2014, 10:07 a.m.
On 28.02.2014 20:57, Augie Fackler wrote:
> On Jan 16, 2014, at 3:58 PM, Simon Heimberg <simohe@besonet.ch> wrote:
>
>> # HG changeset patch
>> # User Simon Heimberg <simohe@besonet.ch>
>> # Date 1385413246 -3600
>> # Node ID afdcb3c8d7aae39b02ea208c180e36a2a6602bf0
>> # Parent  e2068d5d4289fe61f2e5739d62d92fa3c0c2f285
>> run-tests: test result shows when a failed test could not start a server
> This looks mostly reasonable to me. Does anyone else have feelings?
Not sure if we still need this when
[PATCH stable] win32: spawndetached returns pid of detached process and 
not of cmd.exe [1]
is accepted (plus buildbot is rebooted). Server start on windows should 
not fail after this.

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

>> Failing to start a server happens regularly, at least on windows buildbot.
>> Such a failure often has nothing to do with the test, but with the environment.
>> But half the test output can change because some data is missing. Therefore this
>> is worth an extended error message.
>>
>>
<snip>
Simon Heimberg - March 5, 2014, 9:45 p.m.
On 2014-03-01 11:07, Simon Heimberg wrote:
> On 28.02.2014 20:57, Augie Fackler wrote:
>> On Jan 16, 2014, at 3:58 PM, Simon Heimberg <simohe@besonet.ch> wrote:
>>
>>> # HG changeset patch
>>> # User Simon Heimberg <simohe@besonet.ch>
>>> # Date 1385413246 -3600
>>> # Node ID afdcb3c8d7aae39b02ea208c180e36a2a6602bf0
>>> # Parent  e2068d5d4289fe61f2e5739d62d92fa3c0c2f285
>>> run-tests: test result shows when a failed test could not start a server
>> This looks mostly reasonable to me. Does anyone else have feelings?
> Not sure if we still need this when
> [PATCH stable] win32: spawndetached returns pid of detached process and
> not of cmd.exe [1]
> is accepted (plus buildbot is rebooted). Server start on windows should
> not fail after this.
>
> [1]
> http://www.selenic.com/pipermail/mercurial-devel/2014-February/056358.html
>

Well, not sure any more about my statement.
In windows build 131 [2] test-wireproto.t starts to have a serve 
failure. And I do not have an idea why this has happened. The previous 
run did not report any problem (and kill should do this now). No run of 
(windows) stable did run between the two.

So probably this patch would still be helpful.

[2] 
http://hgbuildbot.kublai.com/builders/Windows%202008%20R2%20hg%20tests/builds/131 


>>> Failing to start a server happens regularly, at least on windows
>>> buildbot.
>>> Such a failure often has nothing to do with the test, but with the
>>> environment.
>>> But half the test output can change because some data is missing.
>>> Therefore this
>>> is worth an extended error message.
>>>
>>>
> <snip>
Pierre-Yves David - April 14, 2014, 4:34 a.m.
On 01/16/2014 03:58 PM, Simon Heimberg wrote:
> # HG changeset patch
> # User Simon Heimberg <simohe@besonet.ch>
> # Date 1385413246 -3600
> # Node ID afdcb3c8d7aae39b02ea208c180e36a2a6602bf0
> # Parent  e2068d5d4289fe61f2e5739d62d92fa3c0c2f285
> run-tests: test result shows when a failed test could not start a server

I'm please to announce that this patch have been successfully 
clowncopterized. Thanks!

This is very common and silly failure this patch is much welcome.

Patch

diff -r e2068d5d4289 -r afdcb3c8d7aa tests/run-tests.py
--- a/tests/run-tests.py	Son Nov 17 20:15:28 2013 +0100
+++ b/tests/run-tests.py	Mon Nov 25 22:00:46 2013 +0100
@@ -300,8 +300,14 @@ 
 
 def showdiff(expected, output, ref, err):
     print
+    servefail = False
     for line in difflib.unified_diff(expected, output, ref, err):
         sys.stdout.write(line)
+        if not servefail and line.startswith(
+                             '+  abort: child process failed to start'):
+            servefail = True
+    return {'servefail': servefail}
+
 
 verbose = False
 def vlog(*msg):
@@ -1022,17 +1028,21 @@ 
     elif ret == 'timeout':
         result = fail("timed out", ret)
     elif out != refout:
+        info = {}
         if not options.nodiff:
             iolock.acquire()
             if options.view:
                 os.system("%s %s %s" % (options.view, ref, err))
             else:
-                showdiff(refout, out, ref, err)
+                info = showdiff(refout, out, ref, err)
             iolock.release()
+        msg = ""
+        if info.get('servefail'): msg += "serve failed and "
         if ret:
-            result = fail("output changed and " + describe(ret), ret)
+            msg += "output changed and " + describe(ret)
         else:
-            result = fail("output changed", ret)
+            msg += "output changed"
+        result = fail(msg, ret)
     elif ret:
         result = fail(describe(ret), ret)
     else: