Patchwork [1,of,5] run-tests: when building json, use result.failures instead of result.faildata

login
register
mail settings
Submitter Pierre-Yves David
Date May 8, 2015, 6:53 p.m.
Message ID <dccc3de8c055f386a1db.1431111234@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/8979/
State Accepted
Headers show

Comments

Pierre-Yves David - May 8, 2015, 6:53 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1431066024 25200
#      Thu May 07 23:20:24 2015 -0700
# Node ID dccc3de8c055f386a1dbfe72e86091251dbd0b50
# Parent  c25b2adb3664cd3c488e2c53aab0c64100d40af7
run-tests: when building json, use result.failures instead of result.faildata

It is unclear to me why 'faildata' was used. Lets use the same kind of attribute
as for the other groups.
Martin von Zweigbergk - May 8, 2015, 11:23 p.m.
On Fri, May 8, 2015 at 11:54 AM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1431066024 25200
> #      Thu May 07 23:20:24 2015 -0700
> # Node ID dccc3de8c055f386a1dbfe72e86091251dbd0b50
> # Parent  c25b2adb3664cd3c488e2c53aab0c64100d40af7
> run-tests: when building json, use result.failures instead of
> result.faildata
>
> It is unclear to me why 'faildata' was used. Lets use the same kind of
> attribute
> as for the other groups.
>

I think this needs to be described better. They don't seem to be
equivalent. For example, "faildata" is not populated with failures that
were accepted in interactive mode. addFailure(), which adds to "failures",
is called from two places and I know too little to understand what kinds of
failures would be added in each place and whether the same errors would be
added to "faildata"


>
> diff --git a/tests/run-tests.py b/tests/run-tests.py
> --- a/tests/run-tests.py
> +++ b/tests/run-tests.py
> @@ -1535,16 +1535,16 @@ class TextTestRunner(unittest.TextTestRu
>                                    'time': ('%0.3f' % timesd[tc.name][0]),
>                                    'cuser': ('%0.3f' % timesd[tc.name
> ][1]),
>                                    'csys': ('%0.3f' % timesd[tc.name][2])}
>                      outcome[tc.name] = testresult
>
> -                for tc, err in sorted(result.faildata.iteritems()):
> +                for tc, error in result.failures:
>                      testresult = {'result': 'failure',
> -                                  'time': ('%0.3f' % timesd[tc][0]),
> -                                  'cuser': ('%0.3f' % timesd[tc][1]),
> -                                  'csys': ('%0.3f' % timesd[tc][2])}
> -                    outcome[tc] = testresult
> +                                  'time': ('%0.3f' % timesd[tc.name][0]),
> +                                  'cuser': ('%0.3f' % timesd[tc.name
> ][1]),
> +                                  'csys': ('%0.3f' % timesd[tc.name][2])}
> +                    outcome[tc.name] = testresult
>
>                  for tc, reason in result.skipped:
>                      testresult = {'result': 'skip',
>                                    'time': ('%0.3f' % timesd[tc.name][0]),
>                                    'cuser': ('%0.3f' % timesd[tc.name
> ][1]),
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Gregory Szorc - May 8, 2015, 11:30 p.m.
On Fri, May 8, 2015 at 11:53 AM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1431066024 25200
> #      Thu May 07 23:20:24 2015 -0700
> # Node ID dccc3de8c055f386a1dbfe72e86091251dbd0b50
> # Parent  c25b2adb3664cd3c488e2c53aab0c64100d40af7
> run-tests: when building json, use result.failures instead of
> result.faildata
>
>
If you are going to spend a lot of time on run-tests.py, I suggest you wait
a few days for Python 2.4/2.5 to be deprecated. There is tons of
legacy/polyfill code in run-tests.py that exists solely to support
antiquated versions of the unittest package. The file should become much
simpler once it is refactored to use modern unittest primitives.
Pierre-Yves David - May 9, 2015, 12:01 a.m.
On 05/08/2015 04:30 PM, Gregory Szorc wrote:
> On Fri, May 8, 2015 at 11:53 AM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david@fb.com
>     <mailto:pierre-yves.david@fb.com>>
>     # Date 1431066024 25200
>     #      Thu May 07 23:20:24 2015 -0700
>     # Node ID dccc3de8c055f386a1dbfe72e86091251dbd0b50
>     # Parent  c25b2adb3664cd3c488e2c53aab0c64100d40af7
>     run-tests: when building json, use result.failures instead of
>     result.faildata
>
>
> If you are going to spend a lot of time on run-tests.py, I suggest you
> wait a few days for Python 2.4/2.5 to be deprecated. There is tons of
> legacy/polyfill code in run-tests.py that exists solely to support
> antiquated versions of the unittest package. The file should become much
> simpler once it is refactored to use modern unittest primitives.

I'm pretty much done spending time in it. the second part of this series 
add recording of start and end of each test. And helped me speeding him 
my test run.

Does dropping 2.4/2.5 changes this failures/faildata things?
Pierre-Yves David - May 9, 2015, 2:46 a.m.
On 05/08/2015 04:23 PM, Martin von Zweigbergk wrote:
>
>
> On Fri, May 8, 2015 at 11:54 AM Pierre-Yves David
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david@fb.com
>     <mailto:pierre-yves.david@fb.com>>
>     # Date 1431066024 25200
>     #      Thu May 07 23:20:24 2015 -0700
>     # Node ID dccc3de8c055f386a1dbfe72e86091251dbd0b50
>     # Parent  c25b2adb3664cd3c488e2c53aab0c64100d40af7
>     run-tests: when building json, use result.failures instead of
>     result.faildata
>
>     It is unclear to me why 'faildata' was used. Lets use the same kind
>     of attribute
>     as for the other groups.
>
>
> I think this needs to be described better. They don't seem to be
> equivalent. For example, "faildata" is not populated with failures that
> were accepted in interactive mode. addFailure(), which adds to
> "failures", is called from two places and I know too little to
> understand what kinds of failures would be added in each place and
> whether the same errors would be added to "faildata"

I've added a test to check for the --interactive case and everything 
seems to be fine with python-27 (I could not get python2.4 to run test 
at all).

I think we are fine here. If we actually fine regression in the wild, 
this will be a good time to add a test case.
Gregory Szorc - May 9, 2015, 4:37 p.m.
On Fri, May 8, 2015 at 5:01 PM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 05/08/2015 04:30 PM, Gregory Szorc wrote:
>
>> On Fri, May 8, 2015 at 11:53 AM, Pierre-Yves David
>> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
>> wrote:
>>
>>     # HG changeset patch
>>     # User Pierre-Yves David <pierre-yves.david@fb.com
>>     <mailto:pierre-yves.david@fb.com>>
>>     # Date 1431066024 25200
>>     #      Thu May 07 23:20:24 2015 -0700
>>     # Node ID dccc3de8c055f386a1dbfe72e86091251dbd0b50
>>     # Parent  c25b2adb3664cd3c488e2c53aab0c64100d40af7
>>     run-tests: when building json, use result.failures instead of
>>     result.faildata
>>
>>
>> If you are going to spend a lot of time on run-tests.py, I suggest you
>> wait a few days for Python 2.4/2.5 to be deprecated. There is tons of
>> legacy/polyfill code in run-tests.py that exists solely to support
>> antiquated versions of the unittest package. The file should become much
>> simpler once it is refactored to use modern unittest primitives.
>>
>
> I'm pretty much done spending time in it. the second part of this series
> add recording of start and end of each test. And helped me speeding him my
> test run.
>
> Does dropping 2.4/2.5 changes this failures/faildata things?


IIRC the modern unittest API offers better mechanisms for dealing with
reporting of test results.

Also, our current implementation of result reporting that writes output,
obtains a lock, etc grossly violates the intended unittest API. Cleaning it
up is very far down on my priorities list, however.
Gregory Szorc - May 9, 2015, 4:40 p.m.
On Fri, May 8, 2015 at 11:53 AM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1431066024 25200
> #      Thu May 07 23:20:24 2015 -0700
> # Node ID dccc3de8c055f386a1dbfe72e86091251dbd0b50
> # Parent  c25b2adb3664cd3c488e2c53aab0c64100d40af7
> run-tests: when building json, use result.failures instead of
> result.faildata
>
>
This series LGTM. I'd queue it if I could.
Martin von Zweigbergk - May 9, 2015, 5:39 p.m.
I will not be logged on to my computer today, and with a review from
someone who actually seems to know a bit about the code (Greg), I'd say you
should feel free to queue it yourself, Pierre-Yves. You said you have done
added test that I don't have anyway.

On Sat, May 9, 2015, 09:41 Gregory Szorc <gregory.szorc@gmail.com> wrote:

> On Fri, May 8, 2015 at 11:53 AM, Pierre-Yves David <
> pierre-yves.david@ens-lyon.org> wrote:
>
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>> # Date 1431066024 25200
>> #      Thu May 07 23:20:24 2015 -0700
>> # Node ID dccc3de8c055f386a1dbfe72e86091251dbd0b50
>> # Parent  c25b2adb3664cd3c488e2c53aab0c64100d40af7
>> run-tests: when building json, use result.failures instead of
>> result.faildata
>>
>>
> This series LGTM. I'd queue it if I could.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Pierre-Yves David - May 9, 2015, 7:47 p.m.
On 05/09/2015 10:39 AM, Martin von Zweigbergk wrote:
> I will not be logged on to my computer today, and with a review from
> someone who actually seems to know a bit about the code (Greg), I'd say
> you should feel free to queue it yourself, Pierre-Yves. You said you
> have done added test that I don't have anyway.

The series (with the extra test) is pushed to the clowncopter.
Augie Fackler - May 12, 2015, 5:57 p.m.
On Sat, May 09, 2015 at 09:37:44AM -0700, Gregory Szorc wrote:
> On Fri, May 8, 2015 at 5:01 PM, Pierre-Yves David <
> pierre-yves.david@ens-lyon.org> wrote:
>
> >
> >
> > On 05/08/2015 04:30 PM, Gregory Szorc wrote:
> >
> >> On Fri, May 8, 2015 at 11:53 AM, Pierre-Yves David
> >> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> >> wrote:
> >>
> >>     # HG changeset patch
> >>     # User Pierre-Yves David <pierre-yves.david@fb.com
> >>     <mailto:pierre-yves.david@fb.com>>
> >>     # Date 1431066024 25200
> >>     #      Thu May 07 23:20:24 2015 -0700
> >>     # Node ID dccc3de8c055f386a1dbfe72e86091251dbd0b50
> >>     # Parent  c25b2adb3664cd3c488e2c53aab0c64100d40af7
> >>     run-tests: when building json, use result.failures instead of
> >>     result.faildata
> >>
> >>
> >> If you are going to spend a lot of time on run-tests.py, I suggest you
> >> wait a few days for Python 2.4/2.5 to be deprecated. There is tons of
> >> legacy/polyfill code in run-tests.py that exists solely to support
> >> antiquated versions of the unittest package. The file should become much
> >> simpler once it is refactored to use modern unittest primitives.
> >>
> >
> > I'm pretty much done spending time in it. the second part of this series
> > add recording of start and end of each test. And helped me speeding him my
> > test run.
> >
> > Does dropping 2.4/2.5 changes this failures/faildata things?
>
>
> IIRC the modern unittest API offers better mechanisms for dealing with
> reporting of test results.
>
> Also, our current implementation of result reporting that writes output,
> obtains a lock, etc grossly violates the intended unittest API. Cleaning it
> up is very far down on my priorities list, however.

I beg you both to please hold off on invasive run-tests stuff until
after my 3.5-compat series for run-tests can go in, which is as soon
as we drop 2.5.


> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -1535,16 +1535,16 @@  class TextTestRunner(unittest.TextTestRu
                                   'time': ('%0.3f' % timesd[tc.name][0]),
                                   'cuser': ('%0.3f' % timesd[tc.name][1]),
                                   'csys': ('%0.3f' % timesd[tc.name][2])}
                     outcome[tc.name] = testresult
 
-                for tc, err in sorted(result.faildata.iteritems()):
+                for tc, error in result.failures:
                     testresult = {'result': 'failure',
-                                  'time': ('%0.3f' % timesd[tc][0]),
-                                  'cuser': ('%0.3f' % timesd[tc][1]),
-                                  'csys': ('%0.3f' % timesd[tc][2])}
-                    outcome[tc] = testresult
+                                  'time': ('%0.3f' % timesd[tc.name][0]),
+                                  'cuser': ('%0.3f' % timesd[tc.name][1]),
+                                  'csys': ('%0.3f' % timesd[tc.name][2])}
+                    outcome[tc.name] = testresult
 
                 for tc, reason in result.skipped:
                     testresult = {'result': 'skip',
                                   'time': ('%0.3f' % timesd[tc.name][0]),
                                   'cuser': ('%0.3f' % timesd[tc.name][1]),