Patchwork [085,of,179,tests-refactor] run-tests: move SKIPPED_STATUS into Test class

login
register
mail settings
Submitter Gregory Szorc
Date May 2, 2014, 6:38 p.m.
Message ID <8f290defff48c8ae7624.1399055922@vm-ubuntu-main.gateway.sonic.net>
Download mbox | patch
Permalink /patch/4580/
State Accepted
Commit de6ea36ca9f78fe6667fda473bde615f41397d5f
Headers show

Comments

Gregory Szorc - May 2, 2014, 6:38 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1398014196 25200
#      Sun Apr 20 10:16:36 2014 -0700
# Branch stable
# Node ID 8f290defff48c8ae762404609c67c70a018cd69f
# Parent  f6820235f603f171f82ba5e1641805ae5b423674
run-tests: move SKIPPED_STATUS into Test class
Pierre-Yves David - May 10, 2014, 7:40 p.m.
Greg and I meet in real live and we discussed its patches series.

The general direction seems good.

I've been through the first 90 patches that are code movement only. I 
check that they were d code movement as they claimed and that there was 
no unrelated changes (at least no changes unrelated to code movement).

This was not a deep review of each patch seen I would like this series 
to get in before chrismass but the overall goal of the series looks 
good, the end result look not worse than the initial state. And this was 
just half of the series. The other half make run-tests use the python 
unittest machinery. This will help use integrating the test suite with 
other tools.

So, the series look overall good and test pass on all patches[1].

However, some of this series relies on __del__ + "del t" for clean up. 
This is very 3v1l and fragile and note something I'm willing to queue. 
I'll be happy to queue this series as soon as the involved changeset 
have been cleansed.

473d97194610 run-tests: move createenv() into Test
c02389c8e34f run-tests: remove threadtmp as part of Test.__del__
ac3a389bcb5c run-tests: kill daemons during Test.__del__
b02c1068f9e1 run-tests: move success() into Test
9290fd204a07 run-tests: refactor runone() into gettest() and scheduletests()
641cb3d9f1cf run-tests: move scheduletests() into TestRunner
Gregory Szorc - May 13, 2014, 1:24 a.m.
On 5/10/2014 12:40 PM, Pierre-Yves David wrote:
> Greg and I meet in real live and we discussed its patches series.
> 
> The general direction seems good.
> 
> I've been through the first 90 patches that are code movement only. I
> check that they were d code movement as they claimed and that there was
> no unrelated changes (at least no changes unrelated to code movement).
> 
> This was not a deep review of each patch seen I would like this series
> to get in before chrismass but the overall goal of the series looks
> good, the end result look not worse than the initial state. And this was
> just half of the series. The other half make run-tests use the python
> unittest machinery. This will help use integrating the test suite with
> other tools.
> 
> So, the series look overall good and test pass on all patches[1].
> 
> However, some of this series relies on __del__ + "del t" for clean up.
> This is very 3v1l and fragile and note something I'm willing to queue.
> I'll be happy to queue this series as soon as the involved changeset
> have been cleansed.
> 
> 473d97194610 run-tests: move createenv() into Test
> c02389c8e34f run-tests: remove threadtmp as part of Test.__del__
> ac3a389bcb5c run-tests: kill daemons during Test.__del__
> b02c1068f9e1 run-tests: move success() into Test
> 9290fd204a07 run-tests: refactor runone() into gettest() and
> scheduletests()
> 641cb3d9f1cf run-tests: move scheduletests() into TestRunner
> 

I've renamed __del__ to cleanup() and pushed the latest patches to
http://hg.stage.mozaws.net/hg/. run-tests-refactor-sane bookmark is what
you want.
Pierre-Yves David - May 13, 2014, 7:07 a.m.
On 05/12/2014 06:24 PM, Gregory Szorc wrote:
> On 5/10/2014 12:40 PM, Pierre-Yves David wrote:
>> Greg and I meet in real live and we discussed its patches series.
>>
>> The general direction seems good.
>>
>> I've been through the first 90 patches that are code movement only. I
>> check that they were d code movement as they claimed and that there was
>> no unrelated changes (at least no changes unrelated to code movement).
>>
>> This was not a deep review of each patch seen I would like this series
>> to get in before chrismass but the overall goal of the series looks
>> good, the end result look not worse than the initial state. And this was
>> just half of the series. The other half make run-tests use the python
>> unittest machinery. This will help use integrating the test suite with
>> other tools.
>>
>> So, the series look overall good and test pass on all patches[1].
>>
>> However, some of this series relies on __del__ + "del t" for clean up.
>> This is very 3v1l and fragile and note something I'm willing to queue.
>> I'll be happy to queue this series as soon as the involved changeset
>> have been cleansed.
>>
>> 473d97194610 run-tests: move createenv() into Test
>> c02389c8e34f run-tests: remove threadtmp as part of Test.__del__
>> ac3a389bcb5c run-tests: kill daemons during Test.__del__
>> b02c1068f9e1 run-tests: move success() into Test
>> 9290fd204a07 run-tests: refactor runone() into gettest() and
>> scheduletests()
>> 641cb3d9f1cf run-tests: move scheduletests() into TestRunner
>>
>
> I've renamed __del__ to cleanup() and pushed the latest patches to
> http://hg.stage.mozaws.net/hg/. run-tests-refactor-sane bookmark is what
> you want.

Cool, I'll melt power7 core during the night and hopefully push them 
tomorrow.

Thanks goes to Sean Farley that greatly improved `hg update` from an 
obsolete changesets with a bookmarks. (For those curious, it get you to 
the successors and move the bookmark there.)
Pierre-Yves David - May 13, 2014, 8:25 p.m.
On 05/13/2014 12:07 AM, Pierre-Yves David wrote:
>
>
> On 05/12/2014 06:24 PM, Gregory Szorc wrote:
>> On 5/10/2014 12:40 PM, Pierre-Yves David wrote:
>>> Greg and I meet in real live and we discussed its patches series.
>>>
>>> The general direction seems good.
>>>
>>> I've been through the first 90 patches that are code movement only. I
>>> check that they were d code movement as they claimed and that there was
>>> no unrelated changes (at least no changes unrelated to code movement).
>>>
>>> This was not a deep review of each patch seen I would like this series
>>> to get in before chrismass but the overall goal of the series looks
>>> good, the end result look not worse than the initial state. And this was
>>> just half of the series. The other half make run-tests use the python
>>> unittest machinery. This will help use integrating the test suite with
>>> other tools.
>>>
>>> So, the series look overall good and test pass on all patches[1].
>>>
>>> However, some of this series relies on __del__ + "del t" for clean up.
>>> This is very 3v1l and fragile and note something I'm willing to queue.
>>> I'll be happy to queue this series as soon as the involved changeset
>>> have been cleansed.
>>>
>>> 473d97194610 run-tests: move createenv() into Test
>>> c02389c8e34f run-tests: remove threadtmp as part of Test.__del__
>>> ac3a389bcb5c run-tests: kill daemons during Test.__del__
>>> b02c1068f9e1 run-tests: move success() into Test
>>> 9290fd204a07 run-tests: refactor runone() into gettest() and
>>> scheduletests()
>>> 641cb3d9f1cf run-tests: move scheduletests() into TestRunner
>>>
>>
>> I've renamed __del__ to cleanup() and pushed the latest patches to
>> http://hg.stage.mozaws.net/hg/. run-tests-refactor-sane bookmark is what
>> you want.
>
> Cool, I'll melt power7 core during the night and hopefully push them
> tomorrow.

Test run is happy, tthe first 90 have been pushed to the clowncotper.
Gregory Szorc - May 13, 2014, 11:33 p.m.
On 5/13/2014 1:25 PM, Pierre-Yves David wrote:
>>> I've renamed __del__ to cleanup() and pushed the latest patches to
>>> http://hg.stage.mozaws.net/hg/. run-tests-refactor-sane bookmark is what
>>> you want.
>>
>> Cool, I'll melt power7 core during the night and hopefully push them
>> tomorrow.
> 
> Test run is happy, tthe first 90 have been pushed to the clowncotper.

\o/

We discussed IRL the idea of folding all the patches together such that
any refactoring issues wouldn't result in permanent bisection traps. I
take it you didn't do that?

Patch

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -87,18 +87,16 @@  def Popen4(cmd, wd, timeout, env=None):
                 time.sleep(.1)
             p.timeout = True
             if p.returncode is None:
                 terminate(p)
         threading.Thread(target=t).start()
 
     return p
 
-# reserved exit code to skip test (used by hghave)
-SKIPPED_STATUS = 80
 SKIPPED_PREFIX = 'skipped: '
 FAILED_PREFIX  = 'hghave check failed: '
 PYTHON = sys.executable.replace('\\', '/')
 IMPL_PATH = 'PYTHONPATH'
 if 'java' in sys.platform:
     IMPL_PATH = 'JYTHONPATH'
 
 defaults = {
@@ -347,16 +345,19 @@  def killdaemons(pidfile):
 
 class Test(object):
     """Encapsulates a single, runnable test.
 
     Test instances can be run multiple times via run(). However, multiple
     runs cannot be run concurrently.
     """
 
+    # Status code reserved for skipped tests (used by hghave).
+    SKIPPED_STATUS = 80
+
     def __init__(self, runner, test, count, refpath):
         path = os.path.join(runner.testdir, test)
         errpath = os.path.join(runner.testdir, '%s.err' % test)
 
         self._runner = runner
         self._testdir = runner.testdir
         self._test = test
         self._path = path
@@ -443,17 +444,17 @@  class Test(object):
 
         def describe(ret):
             if ret < 0:
                 return 'killed by signal: %d' % -ret
             return 'returned error code %d' % ret
 
         skipped = False
 
-        if ret == SKIPPED_STATUS:
+        if ret == self.SKIPPED_STATUS:
             if out is None: # Debug mode, nothing to parse.
                 missing = ['unknown']
                 failed = None
             else:
                 missing, failed = TTest.parsehghaveoutput(out)
 
             if not missing:
                 missing = ['irrelevant']
@@ -640,17 +641,17 @@  class TTest(Test):
 
         cmd = '%s "%s"' % (self._options.shell, fname)
         vlog("# Running", cmd)
 
         exitcode, output = run(cmd, testtmp, self._options, replacements, env,
                                self._runner.abort)
         # Do not merge output if skipped. Return hghave message instead.
         # Similarly, with --debug, output is None.
-        if exitcode == SKIPPED_STATUS or output is None:
+        if exitcode == self.SKIPPED_STATUS or output is None:
             return exitcode, output
 
         return self._processoutput(exitcode, output, salt, after, expected)
 
     def _hghave(self, reqs, testtmp):
         # TODO do something smarter when all other uses of hghave are gone.
         tdir = self._testdir.replace('\\', '/')
         proc = Popen4('%s -c "%s/hghave %s"' %