[Buildroot] [PATCH 1/3] testing/infra/emulator: allow to specify pexpect timeout

[Buildroot] [PATCH 1/3] testing/infra/emulator: allow to specify pexpect timeout

[Andrey Smirnov]

Some commands take more than 5 seconds to complete under QEMU, so add
provisions to allow individual unit-test to specify different duration
to avoid false negative test failures.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 support/testing/infra/emulator.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
index a39d59b..4e988a4 100644
--- a/support/testing/infra/emulator.py
+++ b/support/testing/infra/emulator.py
@@ -26,7 +26,10 @@ class Emulator(object):
     #
     # options: array of command line options to pass to Qemu
     #
-    def boot(self, arch, kernel=None, kernel_cmdline=None, options=None):
+    # timeout: timeout to wait for when excuting commands
+    #
+    def boot(self, arch, kernel=None, kernel_cmdline=None,
+             options=None, timeout=5):
         if arch in ["armv7", "armv5"]:
             qemu_arch = "arm"
         else:
@@ -65,7 +68,7 @@ class Emulator(object):
             qemu_cmd += ["-append", " ".join(kernel_cmdline)]
 
         self.logfile.write("> starting qemu with '%s'\n" % " ".join(qemu_cmd))
-        self.qemu = pexpect.spawn(qemu_cmd[0], qemu_cmd[1:], timeout=5)
+        self.qemu = pexpect.spawn(qemu_cmd[0], qemu_cmd[1:], timeout=timeout)
         # We want only stdout into the log to avoid double echo
         self.qemu.logfile_read = self.logfile
 
-- 
2.9.4

[Buildroot] [PATCH 2/3] testing/tests/package/test_python: refactor to support better code reuse

[Andrey Smirnov]

Refactor TestPythonBase class in the following ways:

	 - Split single test_run() function into multiple smaller once
           to allow derivative classes to mix and match what they want
           to test. Also avoid naming any of the functions starting
           with "test_" to prevent nose2 from picking up
           TestPythonBase class as a class that defines any unit
           tests.

	 - Allow derivative classes to specify QEMU/pexpect timeout in
           login() method

	 - Do not hardcode 'python' as a interpreter to use and
           specify that via class variable 'interpreter'. This way
           classes that inherit from TestPythonBase can override this
           particualr aspect of the test code

	 - Change code of libc related test to be both Python2 and
           Python3 compliant so as to be usable for testing both

	 - Create two derivative classes TestPython2 and TestPython3
           that perform all of the tests specified in TestPythonBase
           using Python2 and Python3 correspondingly

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 support/testing/tests/package/test_python.py | 51 ++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 10 deletions(-)

diff --git a/support/testing/tests/package/test_python.py b/support/testing/tests/package/test_python.py
index 5532fb5..b4f52ea 100644
--- a/support/testing/tests/package/test_python.py
+++ b/support/testing/tests/package/test_python.py
@@ -5,31 +5,62 @@ import infra.basetest
 class TestPythonBase(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
 """
-BR2_PACKAGE_PYTHON=y
 BR2_TARGET_ROOTFS_CPIO=y
 # BR2_TARGET_ROOTFS_TAR is not set
 """
+    interpreter = "python"
 
-    def test_run(self):
+    def login(self, timeout=5):
         cpio_file = os.path.join(self.builddir, "images", "rootfs.cpio")
         self.emulator.boot(arch="armv5",
                            kernel="builtin",
-                           options=["-initrd", cpio_file])
+                           options=["-initrd", cpio_file],
+                           timeout)
         self.emulator.login()
-        cmd = "python --version 2>&1 | grep '^Python 2'"
-        _, exit_code = self.emulator.run(cmd)
-        self.assertEqual(exit_code, 0)
 
-        cmd = "python -c 'import math; math.floor(12.3)'"
+    def math_floor_test(self):
+        cmd = self.interpreter + " -c 'import math; math.floor(12.3)'"
         _, exit_code = self.emulator.run(cmd)
         self.assertEqual(exit_code, 0)
 
-        cmd = "python -c 'import ctypes;"
+    def libc_time_test(self):
+        cmd = self.interpreter + " -c 'import ctypes;"
         cmd += "libc = ctypes.cdll.LoadLibrary(\"libc.so.1\");"
-        cmd += "print libc.time(None)'"
+        cmd += "print(libc.time(None))'"
         _, exit_code = self.emulator.run(cmd)
         self.assertEqual(exit_code, 0)
 
-        cmd = "python -c 'import zlib'"
+    def zlib_test(self):
+        cmd = self.interpreter + " -c 'import zlib'"
         _, exit_code = self.emulator.run(cmd)
         self.assertEqual(exit_code, 1)
+
+    def version_test(self, version):
+        cmd = "{} --version 2>&1 | grep '^{}'".format(self.interpreter, version)
+        _, exit_code = self.emulator.run(cmd)
+        self.assertEqual(exit_code, 0)
+
+
+class TestPython2(TestPythonBase):
+    config = TestPythonBase.config + \
+"""
+BR2_PACKAGE_PYTHON=y
+"""
+    def test_run(self):
+        self.login()
+        self.version_test("Python 2")
+        self.math_floor_test()
+        self.libc_time_test()
+        self.zlib_test()
+
+class TestPython3(TestPythonBase):
+    config = TestPythonBase.config + \
+"""
+BR2_PACKAGE_PYTHON3=y
+"""
+    def test_run(self):
+        self.login()
+        self.version_test("Python 3")
+        self.math_floor_test()
+        self.libc_time_test()
+        self.zlib_test()
-- 
2.9.4

[Buildroot] [PATCH 2/3] testing/tests/package/test_python: refactor to support better code reuse

[Thomas Petazzoni]

Hello,

The commit title should mention that a test for Python 3 is added. Or
perhaps you could split this into two commits: one that does the
refactor but keeps only the Python 2 test. And one that only adds the
Python 3 test case on top of that.

On Tue,  4 Jul 2017 11:58:06 -0700, Andrey Smirnov wrote:
> Refactor TestPythonBase class in the following ways:
> 
> 	 - Split single test_run() function into multiple smaller once
>            to allow derivative classes to mix and match what they want
>            to test. Also avoid naming any of the functions starting
>            with "test_" to prevent nose2 from picking up
>            TestPythonBase class as a class that defines any unit
>            tests.
> 
> 	 - Allow derivative classes to specify QEMU/pexpect timeout in
>            login() method
> 
> 	 - Do not hardcode 'python' as a interpreter to use and
>            specify that via class variable 'interpreter'. This way
>            classes that inherit from TestPythonBase can override this
>            particualr aspect of the test code
> 
> 	 - Change code of libc related test to be both Python2 and
>            Python3 compliant so as to be usable for testing both
> 
> 	 - Create two derivative classes TestPython2 and TestPython3
>            that perform all of the tests specified in TestPythonBase
>            using Python2 and Python3 correspondingly

Why use such a huge indentation for the bullet list?


> -    def test_run(self):
> +    def login(self, timeout=5):
>          cpio_file = os.path.join(self.builddir, "images", "rootfs.cpio")
>          self.emulator.boot(arch="armv5",
>                             kernel="builtin",
> -                           options=["-initrd", cpio_file])
> +                           options=["-initrd", cpio_file],
> +                           timeout)
>          self.emulator.login()
> -        cmd = "python --version 2>&1 | grep '^Python 2'"
> -        _, exit_code = self.emulator.run(cmd)
> -        self.assertEqual(exit_code, 0)

I'm not a big fan of having a method called .login() also start the
interpreter, it feels a bit weird.

> +class TestPython2(TestPythonBase):
> +    config = TestPythonBase.config + \
> +"""
> +BR2_PACKAGE_PYTHON=y
> +"""
> +    def test_run(self):
> +        self.login()
> +        self.version_test("Python 2")
> +        self.math_floor_test()
> +        self.libc_time_test()
> +        self.zlib_test()
> +
> +class TestPython3(TestPythonBase):
> +    config = TestPythonBase.config + \
> +"""
> +BR2_PACKAGE_PYTHON3=y
> +"""
> +    def test_run(self):
> +        self.login()
> +        self.version_test("Python 3")
> +        self.math_floor_test()
> +        self.libc_time_test()
> +        self.zlib_test()

This indeed looks really nice!

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 2/3] testing/tests/package/test_python: refactor to support better code reuse

[Andrey Smirnov]

On Wed, Jul 5, 2017 at 3:59 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> The commit title should mention that a test for Python 3 is added. Or
> perhaps you could split this into two commits: one that does the
> refactor but keeps only the Python 2 test. And one that only adds the
> Python 3 test case on top of that.
>

Yeah, I think this commit should be split into at least two. Will do in v2.

> On Tue,  4 Jul 2017 11:58:06 -0700, Andrey Smirnov wrote:
>> Refactor TestPythonBase class in the following ways:
>>
>>        - Split single test_run() function into multiple smaller once
>>            to allow derivative classes to mix and match what they want
>>            to test. Also avoid naming any of the functions starting
>>            with "test_" to prevent nose2 from picking up
>>            TestPythonBase class as a class that defines any unit
>>            tests.
>>
>>        - Allow derivative classes to specify QEMU/pexpect timeout in
>>            login() method
>>
>>        - Do not hardcode 'python' as a interpreter to use and
>>            specify that via class variable 'interpreter'. This way
>>            classes that inherit from TestPythonBase can override this
>>            particualr aspect of the test code
>>
>>        - Change code of libc related test to be both Python2 and
>>            Python3 compliant so as to be usable for testing both
>>
>>        - Create two derivative classes TestPython2 and TestPython3
>>            that perform all of the tests specified in TestPythonBase
>>            using Python2 and Python3 correspondingly
>
> Why use such a huge indentation for the bullet list?
>

Sorry about that, will fix in v2.

>
>> -    def test_run(self):
>> +    def login(self, timeout=5):
>>          cpio_file = os.path.join(self.builddir, "images", "rootfs.cpio")
>>          self.emulator.boot(arch="armv5",
>>                             kernel="builtin",
>> -                           options=["-initrd", cpio_file])
>> +                           options=["-initrd", cpio_file],
>> +                           timeout)
>>          self.emulator.login()
>> -        cmd = "python --version 2>&1 | grep '^Python 2'"
>> -        _, exit_code = self.emulator.run(cmd)
>> -        self.assertEqual(exit_code, 0)
>
> I'm not a big fan of having a method called .login() also start the
> interpreter, it feels a bit weird.
>

I'll split it in the code into custom "boot()" and "login()" in v2,
unless you have something else in mind.

Thanks,
Andrey Smirnov

[Buildroot] [PATCH 2/3] testing/tests/package/test_python: refactor to support better code reuse

[Arnout Vandecappelle]

On 05-07-17 23:30, Andrey Smirnov wrote:
> On Wed, Jul 5, 2017 at 3:59 AM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
>> Hello,
>>
>> The commit title should mention that a test for Python 3 is added. Or
>> perhaps you could split this into two commits: one that does the
>> refactor but keeps only the Python 2 test. And one that only adds the
>> Python 3 test case on top of that.
>>
> 
> Yeah, I think this commit should be split into at least two. Will do in v2.
> 
>> On Tue,  4 Jul 2017 11:58:06 -0700, Andrey Smirnov wrote:
>>> Refactor TestPythonBase class in the following ways:
>>>
>>>        - Split single test_run() function into multiple smaller once
>>>            to allow derivative classes to mix and match what they want
>>>            to test. Also avoid naming any of the functions starting
>>>            with "test_" to prevent nose2 from picking up
>>>            TestPythonBase class as a class that defines any unit
>>>            tests.
>>>
>>>        - Allow derivative classes to specify QEMU/pexpect timeout in
>>>            login() method
>>>
>>>        - Do not hardcode 'python' as a interpreter to use and
>>>            specify that via class variable 'interpreter'. This way
>>>            classes that inherit from TestPythonBase can override this
>>>            particualr aspect of the test code
>>>
>>>        - Change code of libc related test to be both Python2 and
>>>            Python3 compliant so as to be usable for testing both
>>>
>>>        - Create two derivative classes TestPython2 and TestPython3
>>>            that perform all of the tests specified in TestPythonBase
>>>            using Python2 and Python3 correspondingly
>>
>> Why use such a huge indentation for the bullet list?
>>
> 
> Sorry about that, will fix in v2.
> 
>>
>>> -    def test_run(self):
>>> +    def login(self, timeout=5):
>>>          cpio_file = os.path.join(self.builddir, "images", "rootfs.cpio")
>>>          self.emulator.boot(arch="armv5",
>>>                             kernel="builtin",
>>> -                           options=["-initrd", cpio_file])
>>> +                           options=["-initrd", cpio_file],
>>> +                           timeout)
>>>          self.emulator.login()
>>> -        cmd = "python --version 2>&1 | grep '^Python 2'"
>>> -        _, exit_code = self.emulator.run(cmd)
>>> -        self.assertEqual(exit_code, 0)
>>
>> I'm not a big fan of having a method called .login() also start the
>> interpreter, it feels a bit weird.
>>
> 
> I'll split it in the code into custom "boot()" and "login()" in v2,
> unless you have something else in mind.

 I *think* this is a mistake on Thomas's part, that he thought the login()
function would boot, login and start Python. I.e., he missed the - in front of
the line. For me at least it's very natural that the login() function does the
boot + login.


 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 2/3] testing/tests/package/test_python: refactor to support better code reuse

[Thomas Petazzoni]

Hello,

On Thu, 6 Jul 2017 00:09:10 +0200, Arnout Vandecappelle wrote:

> >>> +    def login(self, timeout=5):
> >>>          cpio_file = os.path.join(self.builddir, "images", "rootfs.cpio")
> >>>          self.emulator.boot(arch="armv5",
> >>>                             kernel="builtin",
> >>> -                           options=["-initrd", cpio_file])
> >>> +                           options=["-initrd", cpio_file],
> >>> +                           timeout)
> >>>          self.emulator.login()
> >>> -        cmd = "python --version 2>&1 | grep '^Python 2'"
> >>> -        _, exit_code = self.emulator.run(cmd)
> >>> -        self.assertEqual(exit_code, 0)  
> >>
> >> I'm not a big fan of having a method called .login() also start the
> >> interpreter, it feels a bit weird.
> >>  
> > 
> > I'll split it in the code into custom "boot()" and "login()" in v2,
> > unless you have something else in mind.  
> 
>  I *think* this is a mistake on Thomas's part, that he thought the login()
> function would boot, login and start Python. I.e., he missed the - in front of
> the line. For me at least it's very natural that the login() function does the
> boot + login.

Ah, indeed. My bad. It's perfectly fine for me if a .login() or .boot()
method does both the boot+login.

Sorry for having misread the patch :/

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 3/3] testing/tests/package: add basic unit test for IPython

[Andrey Smirnov]

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 support/testing/tests/package/test_ipython.py | 32 +++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 support/testing/tests/package/test_ipython.py

diff --git a/support/testing/tests/package/test_ipython.py b/support/testing/tests/package/test_ipython.py
new file mode 100644
index 0000000..164a768
--- /dev/null
+++ b/support/testing/tests/package/test_ipython.py
@@ -0,0 +1,32 @@
+import os
+
+import infra.basetest
+from tests.package.test_python import TestPythonBase
+
+class TestIPythonPy2(TestPythonBase):
+    config = TestPythonBase.config + \
+"""
+BR2_PACKAGE_PYTHON=y
+BR2_PACKAGE_PYTHON_IPYTHON=y
+"""
+    interpreter = "ipython"
+
+    def test_run(self):
+        self.login(40)  # It takes IPython a while to start on QEMU
+        self.math_floor_test()
+        self.libc_time_test()
+
+class TestIPythonPy3(TestPythonBase):
+    config = TestPythonBase.config + \
+"""
+BR2_PACKAGE_PYTHON3=y
+BR2_PACKAGE_PYTHON_IPYTHON=y
+"""
+    interpreter = "ipython"
+
+    def test_run(self):
+        self.login(40)  # It takes IPython a while to start on QEMU
+        self.math_floor_test()
+        self.libc_time_test()
+
+
-- 
2.9.4

[Buildroot] [PATCH 3/3] testing/tests/package: add basic unit test for IPython

[Ricardo Martincoski]

Hello,

2 nits below.

On Tue, Jul 04, 2017 at 03:58 PM, Andrey Smirnov wrote:

> +++ b/support/testing/tests/package/test_ipython.py
> @@ -0,0 +1,32 @@
> +import os
> +
> +import infra.basetest

These 2 imports are not used.

> +from tests.package.test_python import TestPythonBase
[snip]
> +        self.libc_time_test()
> +
> +
> -- 

These 2 empty lines at the end of file are not needed.

Regards,
Ricardo

[Buildroot] [PATCH 1/3] testing/infra/emulator: allow to specify pexpect timeout

[Thomas Petazzoni]

Hello,

First of all, thanks a lot for working on the testing infrastructure.
It's nice to see people progressively making use of it!

I however have one comment below.

On Tue,  4 Jul 2017 11:58:05 -0700, Andrey Smirnov wrote:
> Some commands take more than 5 seconds to complete under QEMU, so add
> provisions to allow individual unit-test to specify different duration
> to avoid false negative test failures.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  support/testing/infra/emulator.py | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
> index a39d59b..4e988a4 100644
> --- a/support/testing/infra/emulator.py
> +++ b/support/testing/infra/emulator.py
> @@ -26,7 +26,10 @@ class Emulator(object):
>      #
>      # options: array of command line options to pass to Qemu
>      #
> -    def boot(self, arch, kernel=None, kernel_cmdline=None, options=None):
> +    # timeout: timeout to wait for when excuting commands
> +    #
> +    def boot(self, arch, kernel=None, kernel_cmdline=None,
> +             options=None, timeout=5):

I don't really like the fact that the timeout is passed at boot() and
then applies to the entire pexpect session.

Indeed, within a single pexpect session, we may have some commands that
are expected to be short, some commands that are expected to be long.

The .expect() method of the spawn class does have a timeout argument,
so I believe we could do that per-command.

Do you think it would be possible to instead add the timeout to
emulator.run() instead, and use it only when starting the interpreter ?

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH 1/3] testing/infra/emulator: allow to specify pexpect timeout

[Andrey Smirnov]

On Wed, Jul 5, 2017 at 3:57 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> First of all, thanks a lot for working on the testing infrastructure.
> It's nice to see people progressively making use of it!
>
> I however have one comment below.
>
> On Tue,  4 Jul 2017 11:58:05 -0700, Andrey Smirnov wrote:
>> Some commands take more than 5 seconds to complete under QEMU, so add
>> provisions to allow individual unit-test to specify different duration
>> to avoid false negative test failures.
>>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  support/testing/infra/emulator.py | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/support/testing/infra/emulator.py b/support/testing/infra/emulator.py
>> index a39d59b..4e988a4 100644
>> --- a/support/testing/infra/emulator.py
>> +++ b/support/testing/infra/emulator.py
>> @@ -26,7 +26,10 @@ class Emulator(object):
>>      #
>>      # options: array of command line options to pass to Qemu
>>      #
>> -    def boot(self, arch, kernel=None, kernel_cmdline=None, options=None):
>> +    # timeout: timeout to wait for when excuting commands
>> +    #
>> +    def boot(self, arch, kernel=None, kernel_cmdline=None,
>> +             options=None, timeout=5):
>
> I don't really like the fact that the timeout is passed at boot() and
> then applies to the entire pexpect session.
>
> Indeed, within a single pexpect session, we may have some commands that
> are expected to be short, some commands that are expected to be long.
>
> The .expect() method of the spawn class does have a timeout argument,
> so I believe we could do that per-command.
>
> Do you think it would be possible to instead add the timeout to
> emulator.run() instead, and use it only when starting the interpreter ?
>

Yeah, I agree, it's a better idea to make "timeout" a parameter for
"emulator.run()". Will do in v2.

Thanks,
Andrey Smirnov