[PATCH 1/4] meta: introduce a small baserunner framework

[PATCH 1/4] meta: introduce a small baserunner framework

[jwang]

From: zjh <junhuix.zhang@intel.com>

Signed-off-by: zjh <junhuix.zhang@intel.com>
---
 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
+# 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
+
+class TestRunnerBase(object):
+    '''test runner base '''
+    def __init__(self, context=None):
+        self.tclist = []
+        self.runner = None
+        self.context = context if context else TestContext()
+        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

[PATCH 2/4] meta: implement key baserunner features

[jwang]

From: zjh <junhuix.zhang@intel.com>

Baserunner contains three features:
1. load cases from a manifest file
2. load cases from a package such as "oeqa.runtime"
3. create runner engine based on pyunit textrunner

Signed-off-by: zjh <junhuix.zhang@intel.com>
---
 meta/lib/base/baserunner.py | 44 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/meta/lib/base/baserunner.py b/meta/lib/base/baserunner.py
index 56b838e..d59872f 100755
--- a/meta/lib/base/baserunner.py
+++ b/meta/lib/base/baserunner.py
@@ -31,30 +31,62 @@ class FakeOptions(object):
 class TestRunnerBase(object):
     '''test runner base '''
     def __init__(self, context=None):
-        self.tclist = []
+        self.testslist = []
         self.runner = None
         self.context = context if context else TestContext()
+        self.test_options = None
         self.test_result = None
         self.run_time = None
 
+    def __del__(self):
+        """
+        Because unittest.TestCase is a class object, it will exist as long as the python virtual machine process.
+        So tc can't be released if we don't release them explicitly.
+        """
+        if hasattr(unittest.TestCase, "tc"):
+            delattr(unittest.TestCase, "tc")
+
+    @staticmethod
+    def get_tc_from_manifest(fname):
+        '''get tc list from manifest format '''
+        with open(fname, "r") as f:
+            tclist = [n.strip() for n in f.readlines() \
+                                if n.strip() and not n.strip().startswith('#')]
+        return tclist
 
     def configure(self, options=FakeOptions()):
         '''configure before testing'''
-        pass
+        self.test_options = options
+        self.runner = unittest.TextTestRunner(stream=sys.stderr, \
+                                                  verbosity=2)
 
     def result(self):
         '''output test result '''
-        pass
+        return self.test_result
 
     def loadtest(self, names=None):
         '''load test suite'''
-        pass
+        if names is None:
+            names = self.testslist
+        testloader = unittest.TestLoader()
+        tclist = []
+        for name in names:
+            tset = testloader.loadTestsFromName(name)
+            if tset.countTestCases() > 0:
+                tclist.append(tset)
+            elif tset._tests == []:
+                tclist.append(testloader.discover(name, "[!_]*.py", os.path.curdir))
+        return testloader.suiteClass(tclist)
 
     def runtest(self, testsuite):
         '''run test suite'''
-        pass
+        starttime = time.time()
+        self.test_result = self.runner.run(testsuite)
+        self.run_time = time.time() - starttime
 
     def start(self, testsuite):
         '''start testing'''
-        pass
+        setattr(unittest.TestCase, "tc", self.context)
+        self.runtest(testsuite)
+        self.result()
 
-- 
2.1.4

Re: [PATCH 2/4] meta: implement key baserunner features

[Joshua Lock]

On Tue, 2016-09-13 at 09:17 +0800, jwang wrote:
> From: zjh <junhuix.zhang@intel.com>
> 
> Baserunner contains three features:
> 1. load cases from a manifest file
> 2. load cases from a package such as "oeqa.runtime"
> 3. create runner engine based on pyunit textrunner


I think this and 1/4 should probably be squashed together?

> 
> Signed-off-by: zjh <junhuix.zhang@intel.com>
> ---
>  meta/lib/base/baserunner.py | 44
> ++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/meta/lib/base/baserunner.py
> b/meta/lib/base/baserunner.py
> index 56b838e..d59872f 100755
> --- a/meta/lib/base/baserunner.py
> +++ b/meta/lib/base/baserunner.py
> @@ -31,30 +31,62 @@ class FakeOptions(object):
>  class TestRunnerBase(object):
>      '''test runner base '''
>      def __init__(self, context=None):
> -        self.tclist = []
> +        self.testslist = []
>          self.runner = None
>          self.context = context if context else TestContext()
> +        self.test_options = None
>          self.test_result = None
>          self.run_time = None
>  
> +    def __del__(self):
> +        """
> +        Because unittest.TestCase is a class object, it will exist
> as long as the python virtual machine process.
> +        So tc can't be released if we don't release them explicitly.
> +        """
> +        if hasattr(unittest.TestCase, "tc"):
> +            delattr(unittest.TestCase, "tc")
> +
> +    @staticmethod
> +    def get_tc_from_manifest(fname):
> +        '''get tc list from manifest format '''
> +        with open(fname, "r") as f:
> +            tclist = [n.strip() for n in f.readlines() \
> +                                if n.strip() and not
> n.strip().startswith('#')]
> +        return tclist

It might be nice to handle open() failing here? If open() fails we're
trying to return an undefined instance.

>  
>      def configure(self, options=FakeOptions()):
>          '''configure before testing'''
> -        pass
> +        self.test_options = options
> +        self.runner = unittest.TextTestRunner(stream=sys.stderr, \

There's no need for a backslash here, we can rely on Python's implied
continuation.

> +                                                  verbosity=2)
>  
>      def result(self):
>          '''output test result '''
> -        pass
> +        return self.test_result
>  
>      def loadtest(self, names=None):
>          '''load test suite'''
> -        pass
> +        if names is None:

It's much more idiomatic to write these like:

	if not names:

> +            names = self.testslist
> +        testloader = unittest.TestLoader()
> +        tclist = []
> +        for name in names:
> +            tset = testloader.loadTestsFromName(name)
> +            if tset.countTestCases() > 0:
> +                tclist.append(tset)
> +            elif tset._tests == []:

variable names prefixed with an underscore are, by convention,
internal/private to the object.

Is there a case where countTestCases() might not be > 0 and _tests[] !=
[] ? i.e. can we just use an else here?

> +                tclist.append(testloader.discover(name, "[!_]*.py",
> os.path.curdir))
> +        return testloader.suiteClass(tclist)
>  
>      def runtest(self, testsuite):
>          '''run test suite'''
> -        pass
> +        starttime = time.time()
> +        self.test_result = self.runner.run(testsuite)
> +        self.run_time = time.time() - starttime
>  
>      def start(self, testsuite):
>          '''start testing'''
> -        pass
> +        setattr(unittest.TestCase, "tc", self.context)
> +        self.runtest(testsuite)
> +        self.result()
>  
> -- 
> 2.1.4
> 

[PATCH 3/4] meta: use baserunner in oetest

[jwang]

From: zjh <junhuix.zhang@intel.com>

enable two features from baserunner:
1. loadtest
2. read test cases from manifest file

Signed-off-by: zjh <junhuix.zhang@intel.com>
---
 meta/lib/oeqa/oetest.py | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/meta/lib/oeqa/oetest.py b/meta/lib/oeqa/oetest.py
index 4a740fb..9211cec 100644
--- a/meta/lib/oeqa/oetest.py
+++ b/meta/lib/oeqa/oetest.py
@@ -28,6 +28,8 @@ except ImportError:
     pass
 from oeqa.utils.decorators import LogResults, gettag, getResults
 from oeqa.utils import avoid_paths_in_environ
+from base.baserunner import TestRunnerBase
+
 
 logger = logging.getLogger("BitBake")
 
@@ -203,8 +205,10 @@ def custom_verbose(msg, *args, **kwargs):
             logger.info(_buffer_logger.rstrip("\n"), *args, **kwargs)
         _buffer_logger = ""
 
-class TestContext(object):
+class TestContext(TestRunnerBase):
+    _configure_in_init = True
     def __init__(self, d, exported=False):
+        super(TestContext, self).__init__(self)
         self.d = d
 
         self.testsuites = self._get_test_suites()
@@ -223,6 +227,9 @@ class TestContext(object):
         self.imagefeatures = d.getVar("IMAGE_FEATURES", True).split()
         self.distrofeatures = d.getVar("DISTRO_FEATURES", True).split()
 
+        if self._configure_in_init:
+            self.configure()
+
     # get testcase list from specified file
     # if path is a relative path, then relative to build/conf/
     def _read_testlist(self, fpath, builddir):
@@ -329,9 +336,7 @@ class TestContext(object):
     def loadTests(self):
         setattr(oeTest, "tc", self)
 
-        testloader = unittest.TestLoader()
-        testloader.sortTestMethodsUsing = None
-        suites = [testloader.loadTestsFromName(name) for name in self.testslist]
+        suites = self.loadtest(self.testslist)
         suites = filterByTagExp(suites, getattr(self, "tagexp", None))
 
         # Determine dependencies between suites by looking for @skipUnlessPassed
@@ -376,9 +381,9 @@ class TestContext(object):
         def cmpfunc(a, b):
             return cmp((a.depth, a.index), (b.depth, b.index))
 
-        suites.sort(key=functools.cmp_to_key(cmpfunc))
+        suites._tests.sort(key=functools.cmp_to_key(cmpfunc))
 
-        self.suite = testloader.suiteClass(suites)
+        self.suite = suites
 
         return self.suite
 
@@ -387,11 +392,10 @@ class TestContext(object):
         if hasattr(self, "tagexp") and self.tagexp:
             logger.info("Filter test cases by tags: %s" % self.tagexp)
         logger.info("Found %s tests" % self.suite.countTestCases())
-        runner = unittest.TextTestRunner(verbosity=2)
         if 'bb' in sys.modules:
-            runner.stream.write = custom_verbose
-
-        return runner.run(self.suite)
+            self.runner.stream.write = custom_verbose
+        self.start(self.suite)
+        return self.result()
 
 class RuntimeTestContext(TestContext):
     def __init__(self, d, target, exported=False):
-- 
2.1.4

Re: [PATCH 3/4] meta: use baserunner in oetest

[Joshua Lock]

On Tue, 2016-09-13 at 09:17 +0800, jwang wrote:
> From: zjh <junhuix.zhang@intel.com>
> 
> enable two features from baserunner:
> 1. loadtest
> 2. read test cases from manifest file
> 
> Signed-off-by: zjh <junhuix.zhang@intel.com>
> ---
>  meta/lib/oeqa/oetest.py | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/meta/lib/oeqa/oetest.py b/meta/lib/oeqa/oetest.py
> index 4a740fb..9211cec 100644
> --- a/meta/lib/oeqa/oetest.py
> +++ b/meta/lib/oeqa/oetest.py
> @@ -28,6 +28,8 @@ except ImportError:
>      pass
>  from oeqa.utils.decorators import LogResults, gettag, getResults
>  from oeqa.utils import avoid_paths_in_environ
> +from base.baserunner import TestRunnerBase
> +
>  
>  logger = logging.getLogger("BitBake")
>  
> @@ -203,8 +205,10 @@ def custom_verbose(msg, *args, **kwargs):
>              logger.info(_buffer_logger.rstrip("\n"), *args,
> **kwargs)
>          _buffer_logger = ""
>  
> -class TestContext(object):
> +class TestContext(TestRunnerBase):
> +    _configure_in_init = True

This is a funny variable, when would it not be True and why? Could we
add a comment here about what the variable is for?

Should it be an argument passed at object construction, as it's denoted
as internal by the _ prefix?

>      def __init__(self, d, exported=False):
> +        super(TestContext, self).__init__(self)
>          self.d = d
>  
>          self.testsuites = self._get_test_suites()
> @@ -223,6 +227,9 @@ class TestContext(object):
>          self.imagefeatures = d.getVar("IMAGE_FEATURES",
> True).split()
>          self.distrofeatures = d.getVar("DISTRO_FEATURES",
> True).split()
>  
> +        if self._configure_in_init:
> +            self.configure()

If we don't configure during __init__() when do we do it? Why might we
not do it during __init__() ?

> +
>      # get testcase list from specified file
>      # if path is a relative path, then relative to build/conf/
>      def _read_testlist(self, fpath, builddir):
> @@ -329,9 +336,7 @@ class TestContext(object):
>      def loadTests(self):
>          setattr(oeTest, "tc", self)
>  
> -        testloader = unittest.TestLoader()
> -        testloader.sortTestMethodsUsing = None
> -        suites = [testloader.loadTestsFromName(name) for name in
> self.testslist]
> +        suites = self.loadtest(self.testslist)
>          suites = filterByTagExp(suites, getattr(self, "tagexp",
> None))
>  
>          # Determine dependencies between suites by looking for
> @skipUnlessPassed
> @@ -376,9 +381,9 @@ class TestContext(object):
>          def cmpfunc(a, b):
>              return cmp((a.depth, a.index), (b.depth, b.index))
>  
> -        suites.sort(key=functools.cmp_to_key(cmpfunc))
> +        suites._tests.sort(key=functools.cmp_to_key(cmpfunc))

We appear to be using a non-public instance variable, is there
something else we could be using here to get the list of tests?

>  
> -        self.suite = testloader.suiteClass(suites)
> +        self.suite = suites
>  
>          return self.suite
>  
> @@ -387,11 +392,10 @@ class TestContext(object):
>          if hasattr(self, "tagexp") and self.tagexp:
>              logger.info("Filter test cases by tags: %s" %
> self.tagexp)
>          logger.info("Found %s tests" % self.suite.countTestCases())
> -        runner = unittest.TextTestRunner(verbosity=2)
>          if 'bb' in sys.modules:
> -            runner.stream.write = custom_verbose
> -
> -        return runner.run(self.suite)
> +            self.runner.stream.write = custom_verbose
> +        self.start(self.suite)
> +        return self.result()
>  
>  class RuntimeTestContext(TestContext):
>      def __init__(self, d, target, exported=False):
> -- 
> 2.1.4
> 

[PATCH 4/4] meta: modify runexported script to inherit the features from baserunner

[jwang]

From: zjh <junhuix.zhang@intel.com>

Signed-off-by: zjh <junhuix.zhang@intel.com>
---
 meta/classes/testexport.bbclass |  2 ++
 meta/lib/base/baserunner.py     | 21 ++++++++++++++++++++-
 meta/lib/oeqa/runexported.py    | 12 +++++++++++-
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/meta/classes/testexport.bbclass b/meta/classes/testexport.bbclass
index 15fa470..c65ad55 100644
--- a/meta/classes/testexport.bbclass
+++ b/meta/classes/testexport.bbclass
@@ -128,6 +128,8 @@ def exportTests(d,tc):
         for f in files:
             shutil.copy2(os.path.join(root, f), os.path.join(exportpath, "oeqa/runtime/files"))
 
+    # copy base*
+    shutil.copytree(os.path.join(meta_layer, "lib", "base"), os.path.join(exportpath, "base"))
     # Create tar file for common parts of testexport
     create_tarball(d, "testexport.tar.gz", d.getVar("TEST_EXPORT_DIR", True))
 
diff --git a/meta/lib/base/baserunner.py b/meta/lib/base/baserunner.py
index d59872f..9b38f1b 100755
--- a/meta/lib/base/baserunner.py
+++ b/meta/lib/base/baserunner.py
@@ -57,9 +57,28 @@ class TestRunnerBase(object):
     def configure(self, options=FakeOptions()):
         '''configure before testing'''
         self.test_options = options
-        self.runner = unittest.TextTestRunner(stream=sys.stderr, \
+        if options.xunit:
+            try:
+                from xmlrunner import XMLTestRunner
+            except ImportError:
+                raise Exception("unittest-xml-reporting not installed")
+            self.runner = XMLTestRunner(stream=sys.stderr, \
+                                        verbosity=2, output=options.xunit)
+        else:
+            self.runner = unittest.TextTestRunner(stream=sys.stderr, \
                                                   verbosity=2)
 
+        if options.manifest:
+            fbname, fext = os.path.splitext(os.path.basename(options.manifest))
+            assert fbname == "manifest" or fext == ".manifest", \
+                  "Please specify file name like xxx.manifest or manifest.xxx"
+            self.tclist = self.get_tc_from_manifest(options.manifest)
+
+        if options.tests:
+            tcs = [t[0:-3] if t.endswith(".py") else t[0:-1] \
+                               if t.endswith("/") else t for t in options.tests]
+            self.tclist.extend([tc.replace("/", ".") for tc in tcs])
+
     def result(self):
         '''output test result '''
         return self.test_result
diff --git a/meta/lib/oeqa/runexported.py b/meta/lib/oeqa/runexported.py
index 5886739..18e9543 100755
--- a/meta/lib/oeqa/runexported.py
+++ b/meta/lib/oeqa/runexported.py
@@ -69,6 +69,9 @@ class MyDataDict(dict):
     def getVar(self, key, unused = None):
         return self.get(key, "")
 
+class RunExportTestContext(ExportTestContext):
+    _configure_in_init = False
+
 def main():
 
     parser = argparse.ArgumentParser()
@@ -81,6 +84,12 @@ def main():
             specified in the json if that directory actually exists or it will error out.")
     parser.add_argument("-l", "--log-dir", dest="log_dir", help="This sets the path for TEST_LOG_DIR. If not specified \
             the current dir is used. This is used for usually creating a ssh log file and a scp test file.")
+    parser.add_argument("-f", "--manifest", dest="manifest",
+                        help="The test list file"),
+    parser.add_argument("-x", "--xunit", dest="xunit",
+                        help="Output result path of in xUnit XML format"),
+    parser.add_argument("-e", "--tests", dest="tests", action="append",
+                        help="Run tests by dot separated module path"),
     parser.add_argument("json", help="The json file exported by the build system", default="testdata.json", nargs='?')
 
     args = parser.parse_args()
@@ -114,7 +123,8 @@ def main():
         setattr(target, key, loaded["target"][key])
 
     target.exportStart()
-    tc = ExportTestContext(d, target, True)
+    tc = RunExportTestContext(d, target, True)
+    tc.configure(args)
     tc.loadTests()
     tc.runTests()
 
-- 
2.1.4

Re: [PATCH 1/4] meta: introduce a small baserunner framework

[Joshua Lock]

Hi Jing, 

Thanks for your submission.

On Tue, 2016-09-13 at 09:17 +0800, jwang wrote:
> From: zjh <junhuix.zhang@intel.com>

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 <junhuix.zhang@intel.com>
> ---
>  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
>