Patchwork [STABLE] runtests: skip json for blacklisted tests

login
register
mail settings
Submitter Durham Goode
Date Jan. 21, 2016, 6:40 p.m.
Message ID <488c9fe7971e7df1304c.1453401643@dev8486.prn1.facebook.com>
Download mbox | patch
Permalink /patch/12861/
State Rejected
Headers show

Comments

Durham Goode - Jan. 21, 2016, 6:40 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1453400477 28800
#      Thu Jan 21 10:21:17 2016 -0800
# Branch stable
# Node ID 488c9fe7971e7df1304c13504407efc4621f642d
# Parent  f96bd7d331c205068691de5ee201d2f9cc405e44
runtests: skip json for blacklisted tests

run-tests.py currently throws a KeyError if a blacklist and json output are
specified, since it tries to lookup a non-existent test name in the times table.
This patch fixes it.
Laurent Charignon - Jan. 21, 2016, 7:02 p.m.
I already sent a patch doing almost the same thing (issue5050).

Your patch completely removes the test from the output which is not what my patch is doing, I am not sure which idea makes more sense here.

We should definitely add a test though :)


On 1/21/16, 10:40 AM, "Mercurial-devel on behalf of Durham Goode" <mercurial-devel-bounces@selenic.com on behalf of durham@fb.com> wrote:

># HG changeset patch

># User Durham Goode <durham@fb.com>

># Date 1453400477 28800

>#      Thu Jan 21 10:21:17 2016 -0800

># Branch stable

># Node ID 488c9fe7971e7df1304c13504407efc4621f642d

># Parent  f96bd7d331c205068691de5ee201d2f9cc405e44

>runtests: skip json for blacklisted tests

>

>run-tests.py currently throws a KeyError if a blacklist and json output are

>specified, since it tries to lookup a non-existent test name in the times table.

>This patch fixes it.

>

>diff --git a/tests/run-tests.py b/tests/run-tests.py

>--- a/tests/run-tests.py

>+++ b/tests/run-tests.py

>@@ -1727,6 +1727,9 @@ class TextTestRunner(unittest.TextTestRu

>                               ('skip', result.skipped)]

>                     for res, testcases in groups:

>                         for tc, __ in testcases:

>+                            # Skip tests that were blacklisted

>+                            if not tc.name in timesd:

>+                                continue

>                             tres = {'result': res,

>                                     'time': ('%0.3f' % timesd[tc.name][2]),

>                                     'cuser': ('%0.3f' % timesd[tc.name][0]),

>_______________________________________________

>Mercurial-devel mailing list

>Mercurial-devel@selenic.com

>https://selenic.com/mailman/listinfo/mercurial-devel
timeless - Jan. 21, 2016, 7:17 p.m.
On Thu, Jan 21, 2016 at 2:02 PM, Laurent Charignon <lcharignon@fb.com> wrote:
> I already sent a patch doing almost the same thing (issue5050).
>
> Your patch completely removes the test from the output which is not what my patch is doing, I am not sure which idea makes more sense here.

I'd rather be told if i'm skipping a test.
but, i'd like to know if i'm skipping a test that exists vs. skipping
a test that doesn't exist. they're different. also, it's sorta
important for me to know "i'm skipping a test i explicitly asked for"
v. "i'm skipping a test that I didn't explicitly exclude".

./run-tests.py --blacklist stuff
./run-tests.py --blacklist stuff test-not-there.t
./run-tests.py --blacklist stuff test-will-be-skipped.t
are all different

> We should definitely add a test though :)
yes

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -1727,6 +1727,9 @@  class TextTestRunner(unittest.TextTestRu
                               ('skip', result.skipped)]
                     for res, testcases in groups:
                         for tc, __ in testcases:
+                            # Skip tests that were blacklisted
+                            if not tc.name in timesd:
+                                continue
                             tres = {'result': res,
                                     'time': ('%0.3f' % timesd[tc.name][2]),
                                     'cuser': ('%0.3f' % timesd[tc.name][0]),