diff mbox series

[1/7] support/testing: create default test case for python packages

Message ID 20181016004230.10393-2-ricardo.martincoski@gmail.com
State Changes Requested
Headers show
Series default runtime test case for python packages | expand

Commit Message

Ricardo Martincoski Oct. 16, 2018, 12:42 a.m. UTC
Test cases for python packages are very similar among each other: run a
simple script in the target that minimally tests the package.
So move some common logic to the TestPythonBase. Also move the test
script to be run on the target from inline in the test case to a
separate file.

After this change:
TestPythonBase adds in build time one or more sample scripts to be run
on the target. The test case for the python package must explicitly list
them in the "sample_scripts" property. The test case then logins to the
target, checks the scripts are really in the rootfs (it calls "md5sum"
instead of "ls" or "test" in an attempt to make the logfile more
friendly, since someone analysing a failure can easily check the
expected script was executed) and then calls the python interpreter
passing the sample script as parameter.
An optional property "timeout" exists for the case the sample script
needs more time to run than the default timeout from the test infra
(currently 5 seconds).
Instead of providing a defconfig fragment containing the python version,
the test case for a python package must provide only its own config set,
and inherit directly from TestPython2 or TestPython3 (that in turn
inherit from TestPythonBase).

A simple test case for a package that only supports Python 2 will look
like this:
|import tests.package.test_python
|
|
|class TestPythonPy2<Package>(tests.package.test_python.TestPython2):
|    config_package = \
|        """
|        BR2_PACKAGE_PYTHON_<PACKAGE>=y
|        """
|    sample_scripts = ["tests/package/sample_python_<package>.py"]
|    timeout = 15

When the python package supports both Python 2 and Python 3 a helper
class can be used to hold the common properties:
|class TestPython<Package>():
|    config_package = \
...
|class TestPythonPy2<Package>(TestPython<Package>, tests.package.test_python.TestPython2):
...
|class TestPythonPy3<Package>(TestPython<Package>, tests.package.test_python.TestPython3):

One catch when inheriting from another class that contains a method
test_run is that if "from tests.package.test_python import TestPython2"
is used, nose2 would list the test case TestPython2 twice because it
looks for symbols in the module namespace to discover test cases to run.
So any test cases that inherits from TestPython2 or TestPython3 should
instead import the module and use the full name
tests.package.test_python.TestPython2.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Yegor Yefremov <yegorslists@googlemail.com>
---
 .../package/copy-sample-script-to-target.sh   |  7 +++
 support/testing/tests/package/test_python.py  | 43 +++++++++++++++++++
 2 files changed, 50 insertions(+)
 create mode 100755 support/testing/tests/package/copy-sample-script-to-target.sh

Comments

Thomas Petazzoni Oct. 22, 2018, 7:55 a.m. UTC | #1
Hello Ricardo,

Overall, I find the patch series good (thanks for working on this!), but
there is one thing that I dislike a bit see below.

On Mon, 15 Oct 2018 21:42:24 -0300, Ricardo Martincoski wrote:

> +        if self.__class__.__name__ != "TestPython2":
> +            # default test_run for TestPythonPy2<Package> that inherits from this one
> +            self.check_sample_scripts_exist()
> +            self.run_sample_scripts()
> +            return
> +
>          self.version_test("Python 2")
>          self.math_floor_test()
>          self.libc_time_test()
> @@ -64,6 +100,13 @@ class TestPython3(TestPythonBase):
>  
>      def test_run(self):
>          self.login()
> +
> +        if self.__class__.__name__ != "TestPython3":
> +            # default test_run for TestPythonPy3<Package> that inherits from this one
> +            self.check_sample_scripts_exist()
> +            self.run_sample_scripts()
> +            return
> +
>          self.version_test("Python 3")
>          self.math_floor_test()
>          self.libc_time_test()

I don't really like that TestPython2 and TestPython3 are used for two
entirely separate things:

 - As a base class for testing individual Python packages
 - As test cases for the Python interpreter itself

So here is the class hierarchy that I think would make more sense:

 - TestPythonBase, defines the base configuration, login() method and
   that's it

 - TestPythonBase2, that appends the configuration with
   BR2_PACKAGE_PYTHON=y

 - TestPythonBase3, that appends the configuration with
   BR2_PACKAGE_PYTHON3=y

 - TestPythonInterpreter, defines the version_test(),
   math_floor_test(), etc. methods that test the interpreter, and a
   test_run() method that calls all those tests. Indeed, calling those
   tests is currently repeated between Python2/Python3.

 - TestPythonInterpreter2, which inherits from TestPythonInterpreter
   and TestPythonBase2

 - TestPythonInterpreter3, which inherits from TestPythonInterpreter
   and TestPythonBase3

 - TestPythonPackageBase, which has all the logic to copy sample
   scripts, run the scripts on the target, etc

 - TestPythonAutobahn that defines the sample script to use, and
   inherits from TestPythonPackageBase

 - TestPython2Autobahn that inherits from TestPythonAutobahn and
   TestPythonBase2

etc.

Does that make sense ? Basically, I'd like the test case of the
interpreter to not be a "special case".

Does that make sense ?

Best regards,

Thomas
Ricardo Martincoski Oct. 23, 2018, 3:15 a.m. UTC | #2
Hello,

On Mon, Oct 22, 2018 at 04:55 AM, Thomas Petazzoni wrote:

[snip]
>>      def test_run(self):
>>          self.login()
>> +
>> +        if self.__class__.__name__ != "TestPython3":
>> +            # default test_run for TestPythonPy3<Package> that inherits from this one
>> +            self.check_sample_scripts_exist()
>> +            self.run_sample_scripts()
>> +            return
>> +
>>          self.version_test("Python 3")
>>          self.math_floor_test()
>>          self.libc_time_test()
> 
> I don't really like that TestPython2 and TestPython3 are used for two
> entirely separate things:
> 
>  - As a base class for testing individual Python packages
>  - As test cases for the Python interpreter itself
> 
> So here is the class hierarchy that I think would make more sense:
> 
>  - TestPythonBase, defines the base configuration, login() method and
>    that's it
> 
>  - TestPythonBase2, that appends the configuration with
>    BR2_PACKAGE_PYTHON=y
> 
>  - TestPythonBase3, that appends the configuration with
>    BR2_PACKAGE_PYTHON3=y
> 
>  - TestPythonInterpreter, defines the version_test(),
>    math_floor_test(), etc. methods that test the interpreter, and a
>    test_run() method that calls all those tests. Indeed, calling those
>    tests is currently repeated between Python2/Python3.
> 
>  - TestPythonInterpreter2, which inherits from TestPythonInterpreter
>    and TestPythonBase2
> 
>  - TestPythonInterpreter3, which inherits from TestPythonInterpreter
>    and TestPythonBase3
> 
>  - TestPythonPackageBase, which has all the logic to copy sample
>    scripts, run the scripts on the target, etc
> 
>  - TestPythonAutobahn that defines the sample script to use, and
>    inherits from TestPythonPackageBase
> 
>  - TestPython2Autobahn that inherits from TestPythonAutobahn and
>    TestPythonBase2
> 
> etc.
> 
> Does that make sense ? Basically, I'd like the test case of the
> interpreter to not be a "special case".
> 
> Does that make sense ?

Sure it does make sense. I will rework.
Thank you for your review.
I will set this series as Changes Requested.


Regards,
Ricardo
diff mbox series

Patch

diff --git a/support/testing/tests/package/copy-sample-script-to-target.sh b/support/testing/tests/package/copy-sample-script-to-target.sh
new file mode 100755
index 0000000000..6448a80d6d
--- /dev/null
+++ b/support/testing/tests/package/copy-sample-script-to-target.sh
@@ -0,0 +1,7 @@ 
+#!/bin/sh
+set -e
+
+shift
+for file in "$@"; do
+	cp -f "${file}" "${TARGET_DIR}/root/"
+done
diff --git a/support/testing/tests/package/test_python.py b/support/testing/tests/package/test_python.py
index 26cf49947b..a33db26d00 100644
--- a/support/testing/tests/package/test_python.py
+++ b/support/testing/tests/package/test_python.py
@@ -10,6 +10,23 @@  class TestPythonBase(infra.basetest.BRTest):
         # BR2_TARGET_ROOTFS_TAR is not set
         """
     interpreter = "python"
+    config_package = None
+    config_sample_scripts = \
+        """
+        BR2_ROOTFS_POST_BUILD_SCRIPT="{}"
+        BR2_ROOTFS_POST_SCRIPT_ARGS="{}"
+        """.format(infra.filepath("tests/package/copy-sample-script-to-target.sh"),
+                   "{sample_scripts}")
+    sample_scripts = None
+    timeout = -1
+
+    def __init__(self, names):
+        super(TestPythonBase, self).__init__(names)
+        if self.config_package:
+            self.config += self.config_package
+        if self.sample_scripts:
+            scripts = [infra.filepath(s) for s in self.sample_scripts]
+            self.config += self.config_sample_scripts.format(sample_scripts=" ".join(scripts))
 
     def login(self):
         cpio_file = os.path.join(self.builddir, "images", "rootfs.cpio")
@@ -41,6 +58,18 @@  class TestPythonBase(infra.basetest.BRTest):
         _, exit_code = self.emulator.run(cmd, timeout)
         self.assertEqual(exit_code, 1)
 
+    def check_sample_scripts_exist(self):
+        scripts = [os.path.basename(s) for s in self.sample_scripts]
+        cmd = "md5sum " + " ".join(scripts)
+        _, exit_code = self.emulator.run(cmd)
+        self.assertEqual(exit_code, 0)
+
+    def run_sample_scripts(self):
+        for script in self.sample_scripts:
+            cmd = self.interpreter + " " + os.path.basename(script)
+            _, exit_code = self.emulator.run(cmd, timeout=self.timeout)
+            self.assertEqual(exit_code, 0)
+
 
 class TestPython2(TestPythonBase):
     config = TestPythonBase.config + \
@@ -50,6 +79,13 @@  class TestPython2(TestPythonBase):
 
     def test_run(self):
         self.login()
+
+        if self.__class__.__name__ != "TestPython2":
+            # default test_run for TestPythonPy2<Package> that inherits from this one
+            self.check_sample_scripts_exist()
+            self.run_sample_scripts()
+            return
+
         self.version_test("Python 2")
         self.math_floor_test()
         self.libc_time_test()
@@ -64,6 +100,13 @@  class TestPython3(TestPythonBase):
 
     def test_run(self):
         self.login()
+
+        if self.__class__.__name__ != "TestPython3":
+            # default test_run for TestPythonPy3<Package> that inherits from this one
+            self.check_sample_scripts_exist()
+            self.run_sample_scripts()
+            return
+
         self.version_test("Python 3")
         self.math_floor_test()
         self.libc_time_test()