Hi Jing, Thanks for your submission.
On Tue, 2016-09-13 at 09:17 +0800, jwang wrote: > From: zjh <[email protected]> There's no commit message here, which makes this harder to review as I have to try and work out what it's for and why we want it. A useful commit message would tell me why we are introducing a baserunner framework. This may also make a useful comment in the code itself. Also, the patch prefix for the series should be "lib/oeqa", i.e "[PATCH 3/4] lib/oeqa: use baserunner in oetest" > Signed-off-by: zjh <[email protected]> > --- > meta/lib/base/baserunner.py | 60 > +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 60 insertions(+) > create mode 100755 meta/lib/base/baserunner.py > > diff --git a/meta/lib/base/baserunner.py > b/meta/lib/base/baserunner.py > new file mode 100755 > index 0000000..56b838e > --- /dev/null > +++ b/meta/lib/base/baserunner.py > @@ -0,0 +1,60 @@ > +#!/usr/bin/env python We're defaulting to Python3 in OE-Core nowadays, do you really want Python2 here (which is the default Python on most Linux distros) > +# Copyright (C) 2013 Intel Corporation > +# > +# Released under the MIT license (see COPYING.MIT) > + > +# Base unittest module used by testrunner > +# This provides the common test runner functionalities including > manifest input, > +# xunit output, timeout, tag filtering. > + > +"""Base testrunner""" > + > +from __future__ import absolute_import > +import os > +import sys > +import time > +import unittest > +import shutil > + > +class TestContext(object): > + '''test context which inject into testcase''' > + def __init__(self): > + self.target = None > + > +class FakeOptions(object): > + '''This class just use for configure's defualt arg. > + Usually, we use this object in a non comandline > environment.''' > + timeout = 0 > + def __getattr__(self, name): > + return None What's the purpose of overloading __getattr__() here? > + > +class TestRunnerBase(object): > + '''test runner base ''' This comment isn't very useful, same for other similar comments in this series. > + def __init__(self, context=None): > + self.tclist = [] > + self.runner = None > + self.context = context if context else TestContext() This would probably be better as: self.context = context or TestContext() or better yet, pass TestContext() as the default param for context, rather than None? > + self.test_result = None > + self.run_time = None > + > + > + def configure(self, options=FakeOptions()): > + '''configure before testing''' > + pass > + > + def result(self): > + '''output test result ''' > + pass > + > + def loadtest(self, names=None): > + '''load test suite''' > + pass > + > + def runtest(self, testsuite): > + '''run test suite''' > + pass > + > + def start(self, testsuite): > + '''start testing''' > + pass > + > -- > 2.1.4 > -- _______________________________________________ Openembedded-core mailing list [email protected] http://lists.openembedded.org/mailman/listinfo/openembedded-core
