diff mbox series

[2/3] support/testing: replace nose2 with pytest - run-tests

Message ID 20221021091531.2989489-2-oguz.ozhan@mind.be
State Changes Requested
Headers show
Series [1/3] support/testing: replace nose2 with pytest - Dockerfile | expand

Commit Message

Oguz Ozhan Oct. 21, 2022, 9:15 a.m. UTC
From: Oguz Ozhan <oguz.ozhan@mind.com>

- From web page of nose2:
(https://docs.nose2.io/en/latest/)
nose2 vs pytest:
  - pytest is an excellent test framework and we encourage users to consider it for new projects.
  - It has a bigger team of maintainers and a larger community of users.

- pytest is more robust and has more ability compared to nose2

- Changes in this patch:
  - run-tests modified to use pytest instead of nose2 to call tests. A corresponding Collector class added for pytest to list the available tests
  - nose2 plugins removed which are not required anymore
  - in test_iso9660.py, local test_ routine names changed to not be accepted as test_ cases via pytest

Signed-off-by: Oguz Ozhan <oguz.ozhan@mind.be>
---
 support/testing/conf/unittest.cfg        |  3 --
 support/testing/run-tests                | 28 ++++++++++-------
 support/testing/tests/fs/test_iso9660.py | 40 ++++++++++++------------
 3 files changed, 36 insertions(+), 35 deletions(-)

Comments

Yann E. MORIN Oct. 23, 2022, 4:51 p.m. UTC | #1
Oguz, All,

On 2022-10-21 11:15 +0200, Oguz Ozhan spake thusly:
> From: Oguz Ozhan <oguz.ozhan@mind.com>
> 
> - From web page of nose2:
> (https://docs.nose2.io/en/latest/)
> nose2 vs pytest:
>   - pytest is an excellent test framework and we encourage users to consider it for new projects.
>   - It has a bigger team of maintainers and a larger community of users.
> 
> - pytest is more robust and has more ability compared to nose2

Please see my comment in patch 1 about providing a rationale for the
switch.

Please fold your commit logs at 72 chars.

> - Changes in this patch:
>   - run-tests modified to use pytest instead of nose2 to call tests. A corresponding Collector class added for pytest to list the available tests
>   - nose2 plugins removed which are not required anymore

Don't describe the changes, explain them.

E.g. "nose2 plugins removed which are not required anymore" we can see
they get removed, and since the change is about switching away from
nose2, it is obvious they get removed because they are not used anymore.

>   - in test_iso9660.py, local test_ routine names changed to not be accepted as test_ cases via pytest

This last part should be a patch by itself, coming first in the series,
as it can be applied without doing the conversion yet.

I have only had a cursory look at the changes, they look quite OK.

Be sure to run "make check-flake8", I think I could spot too many lines
at one place...

Regards,
Yann E. MORIN.

> Signed-off-by: Oguz Ozhan <oguz.ozhan@mind.be>
> ---
>  support/testing/conf/unittest.cfg        |  3 --
>  support/testing/run-tests                | 28 ++++++++++-------
>  support/testing/tests/fs/test_iso9660.py | 40 ++++++++++++------------
>  3 files changed, 36 insertions(+), 35 deletions(-)
> 
> diff --git a/support/testing/conf/unittest.cfg b/support/testing/conf/unittest.cfg
> index 4f516fb80a..0dfffcfbda 100644
> --- a/support/testing/conf/unittest.cfg
> +++ b/support/testing/conf/unittest.cfg
> @@ -1,5 +1,2 @@
> -[unittest]
> -plugins = nose2.plugins.mp
> -
>  [multiprocess]
>  always-on = True
> diff --git a/support/testing/run-tests b/support/testing/run-tests
> index bf40019362..f887bc6715 100755
> --- a/support/testing/run-tests
> +++ b/support/testing/run-tests
> @@ -4,10 +4,16 @@ import multiprocessing
>  import os
>  import sys
>  
> -import nose2
> +import pytest
>  
>  from infra.basetest import BRConfigTest
>  
> +class PyTestCollector:
> +    def __init__(self):
> +        self.collected = []
> +    def pytest_collection_modifyitems(self, items):
> +        for item in items:
> +            self.collected.append(item.nodeid)
>  
>  def main():
>      parser = argparse.ArgumentParser(description='Run Buildroot tests')
> @@ -42,12 +48,10 @@ def main():
>          BRConfigTest.logtofile = False
>  
>      if args.list:
> -        print("List of tests")
> -        nose2.discover(argv=[script_path,
> -                             "-s", test_dir,
> -                             "-v",
> -                             "--collect-only"],
> -                       plugins=["nose2.plugins.collect"])
> +        collect_plugin = PyTestCollector()
> +        pytest.main(['--collect-only', '-p', 'no:terminal', test_dir], plugins=[collect_plugin])
> +        for nodeid in collect_plugin.collected:
> +            print(nodeid)
>          return 0
>  
>      if args.download is None:
> @@ -106,15 +110,15 @@ def main():
>          return 1
>      BRConfigTest.timeout_multiplier = args.timeout_multiplier
>  
> -    nose2_args = ["-v",
> -                  "-N", str(args.testcases),
> -                  "-s", test_dir,
> +    pytest_args = ["--workers", str(args.testcases),
> +                  "--rootdir", test_dir,
>                    "-c", os.path.join(test_dir, "conf/unittest.cfg")]
>  
>      if args.testname:
> -        nose2_args += args.testname
> +        pytest_args += args.testname
> +
> +    pytest.main(pytest_args)
>  
> -    nose2.discover(argv=nose2_args)
>  
>  
>  if __name__ == "__main__":
> diff --git a/support/testing/tests/fs/test_iso9660.py b/support/testing/tests/fs/test_iso9660.py
> index 692291267e..d2390cccf1 100644
> --- a/support/testing/tests/fs/test_iso9660.py
> +++ b/support/testing/tests/fs/test_iso9660.py
> @@ -25,7 +25,7 @@ BASIC_CONFIG = \
>      """.format(infra.filepath("conf/minimal-x86-qemu-kernel.config"))
>  
>  
> -def test_mount_internal_external(emulator, builddir, internal=True, efi=False):
> +def do_test_mount_internal_external(emulator, builddir, internal=True, efi=False):
>      img = os.path.join(builddir, "images", "rootfs.iso9660")
>      if efi:
>          efi_img = os.path.join(builddir, "images", "OVMF.fd")
> @@ -43,7 +43,7 @@ def test_mount_internal_external(emulator, builddir, internal=True, efi=False):
>      return exit_code
>  
>  
> -def test_touch_file(emulator):
> +def do_test_touch_file(emulator):
>      _, exit_code = emulator.run("touch test")
>      return exit_code
>  
> @@ -63,11 +63,11 @@ class TestIso9660Grub2External(infra.basetest.BRTest):
>          """.format(infra.filepath("conf/grub2.cfg"))
>  
>      def test_run(self):
> -        exit_code = test_mount_internal_external(self.emulator,
> +        exit_code = do_test_mount_internal_external(self.emulator,
>                                                   self.builddir, internal=False)
>          self.assertEqual(exit_code, 0)
>  
> -        exit_code = test_touch_file(self.emulator)
> +        exit_code = do_test_touch_file(self.emulator)
>          self.assertEqual(exit_code, 1)
>  
>  
> @@ -84,11 +84,11 @@ class TestIso9660Grub2ExternalCompress(infra.basetest.BRTest):
>          """.format(infra.filepath("conf/grub2.cfg"))
>  
>      def test_run(self):
> -        exit_code = test_mount_internal_external(self.emulator,
> +        exit_code = do_test_mount_internal_external(self.emulator,
>                                                   self.builddir, internal=False)
>          self.assertEqual(exit_code, 0)
>  
> -        exit_code = test_touch_file(self.emulator)
> +        exit_code = do_test_touch_file(self.emulator)
>          self.assertEqual(exit_code, 1)
>  
>  
> @@ -104,11 +104,11 @@ class TestIso9660Grub2Internal(infra.basetest.BRTest):
>          """.format(infra.filepath("conf/grub2.cfg"))
>  
>      def test_run(self):
> -        exit_code = test_mount_internal_external(self.emulator,
> +        exit_code = do_test_mount_internal_external(self.emulator,
>                                                   self.builddir, internal=True)
>          self.assertEqual(exit_code, 0)
>  
> -        exit_code = test_touch_file(self.emulator)
> +        exit_code = do_test_touch_file(self.emulator)
>          self.assertEqual(exit_code, 0)
>  
>  
> @@ -127,12 +127,12 @@ class TestIso9660Grub2EFI(infra.basetest.BRTest):
>                     infra.filepath("conf/grub2.cfg"))
>  
>      def test_run(self):
> -        exit_code = test_mount_internal_external(self.emulator,
> +        exit_code = do_test_mount_internal_external(self.emulator,
>                                                   self.builddir, internal=True,
>                                                   efi=True)
>          self.assertEqual(exit_code, 0)
>  
> -        exit_code = test_touch_file(self.emulator)
> +        exit_code = do_test_touch_file(self.emulator)
>          self.assertEqual(exit_code, 0)
>  
>  
> @@ -155,22 +155,22 @@ class TestIso9660Grub2Hybrid(infra.basetest.BRTest):
>                     infra.filepath("conf/grub2.cfg"))
>  
>      def test_run(self):
> -        exit_code = test_mount_internal_external(self.emulator,
> +        exit_code = do_test_mount_internal_external(self.emulator,
>                                                   self.builddir, internal=True,
>                                                   efi=False)
>          self.assertEqual(exit_code, 0)
>  
> -        exit_code = test_touch_file(self.emulator)
> +        exit_code = do_test_touch_file(self.emulator)
>          self.assertEqual(exit_code, 0)
>  
>          self.emulator.stop()
>  
> -        exit_code = test_mount_internal_external(self.emulator,
> +        exit_code = do_test_mount_internal_external(self.emulator,
>                                                   self.builddir, internal=True,
>                                                   efi=True)
>          self.assertEqual(exit_code, 0)
>  
> -        exit_code = test_touch_file(self.emulator)
> +        exit_code = do_test_touch_file(self.emulator)
>          self.assertEqual(exit_code, 0)
>  
>  
> @@ -189,11 +189,11 @@ class TestIso9660SyslinuxExternal(infra.basetest.BRTest):
>          """.format(infra.filepath("conf/isolinux.cfg"))
>  
>      def test_run(self):
> -        exit_code = test_mount_internal_external(self.emulator,
> +        exit_code = do_test_mount_internal_external(self.emulator,
>                                                   self.builddir, internal=False)
>          self.assertEqual(exit_code, 0)
>  
> -        exit_code = test_touch_file(self.emulator)
> +        exit_code = do_test_touch_file(self.emulator)
>          self.assertEqual(exit_code, 1)
>  
>  
> @@ -209,11 +209,11 @@ class TestIso9660SyslinuxExternalCompress(infra.basetest.BRTest):
>          """.format(infra.filepath("conf/isolinux.cfg"))
>  
>      def test_run(self):
> -        exit_code = test_mount_internal_external(self.emulator,
> +        exit_code = do_test_mount_internal_external(self.emulator,
>                                                   self.builddir, internal=False)
>          self.assertEqual(exit_code, 0)
>  
> -        exit_code = test_touch_file(self.emulator)
> +        exit_code = do_test_touch_file(self.emulator)
>          self.assertEqual(exit_code, 1)
>  
>  
> @@ -228,9 +228,9 @@ class TestIso9660SyslinuxInternal(infra.basetest.BRTest):
>          """.format(infra.filepath("conf/isolinux.cfg"))
>  
>      def test_run(self):
> -        exit_code = test_mount_internal_external(self.emulator,
> +        exit_code = do_test_mount_internal_external(self.emulator,
>                                                   self.builddir, internal=True)
>          self.assertEqual(exit_code, 0)
>  
> -        exit_code = test_touch_file(self.emulator)
> +        exit_code = do_test_touch_file(self.emulator)
>          self.assertEqual(exit_code, 0)
> -- 
> 2.34.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
Ricardo Martincoski Oct. 30, 2022, 10:20 p.m. UTC | #2
Hello,

On Fri, Oct 21, 2022 at 06:15 AM, Oguz Ozhan wrote:

> - From web page of nose2:
> (https://docs.nose2.io/en/latest/)
> nose2 vs pytest:
>   - pytest is an excellent test framework and we encourage users to consider it for new projects.
>   - It has a bigger team of maintainers and a larger community of users.
> 
> - pytest is more robust and has more ability compared to nose2
> 
> - Changes in this patch:
>   - run-tests modified to use pytest instead of nose2 to call tests. A corresponding Collector class added for pytest to list the available tests
>   - nose2 plugins removed which are not required anymore
>   - in test_iso9660.py, local test_ routine names changed to not be accepted as test_ cases via pytest

Please split the changes to the tests in a new patch (first in the series).
While doing that, please fix flake8 warnings.

> 
> Signed-off-by: Oguz Ozhan <oguz.ozhan@mind.be>
> ---
>  support/testing/conf/unittest.cfg        |  3 --
>  support/testing/run-tests                | 28 ++++++++++-------

Please fix flake8 warnings for run-tests as well.

>  support/testing/tests/fs/test_iso9660.py | 40 ++++++++++++------------
>  3 files changed, 36 insertions(+), 35 deletions(-)

Please update the manual, section 'Using the runtime tests framework', in the
same commit that changes run-tests to use pytest.

[snip]
> @@ -106,15 +110,15 @@ def main():
>          return 1
>      BRConfigTest.timeout_multiplier = args.timeout_multiplier
>  
> -    nose2_args = ["-v",
> -                  "-N", str(args.testcases),
> -                  "-s", test_dir,
> +    pytest_args = ["--workers", str(args.testcases),
> +                  "--rootdir", test_dir,
>                    "-c", os.path.join(test_dir, "conf/unittest.cfg")]

run-tests -s is broken: it does not show the build and runtime console

>  
>      if args.testname:
> -        nose2_args += args.testname
> +        pytest_args += args.testname
> +
> +    pytest.main(pytest_args)
>  
> -    nose2.discover(argv=nose2_args)

The detection of a failed test on GitLab CI is broken, even when tests fail
run-tests returns exit code 0.

Regards,
Ricardo
diff mbox series

Patch

diff --git a/support/testing/conf/unittest.cfg b/support/testing/conf/unittest.cfg
index 4f516fb80a..0dfffcfbda 100644
--- a/support/testing/conf/unittest.cfg
+++ b/support/testing/conf/unittest.cfg
@@ -1,5 +1,2 @@ 
-[unittest]
-plugins = nose2.plugins.mp
-
 [multiprocess]
 always-on = True
diff --git a/support/testing/run-tests b/support/testing/run-tests
index bf40019362..f887bc6715 100755
--- a/support/testing/run-tests
+++ b/support/testing/run-tests
@@ -4,10 +4,16 @@  import multiprocessing
 import os
 import sys
 
-import nose2
+import pytest
 
 from infra.basetest import BRConfigTest
 
+class PyTestCollector:
+    def __init__(self):
+        self.collected = []
+    def pytest_collection_modifyitems(self, items):
+        for item in items:
+            self.collected.append(item.nodeid)
 
 def main():
     parser = argparse.ArgumentParser(description='Run Buildroot tests')
@@ -42,12 +48,10 @@  def main():
         BRConfigTest.logtofile = False
 
     if args.list:
-        print("List of tests")
-        nose2.discover(argv=[script_path,
-                             "-s", test_dir,
-                             "-v",
-                             "--collect-only"],
-                       plugins=["nose2.plugins.collect"])
+        collect_plugin = PyTestCollector()
+        pytest.main(['--collect-only', '-p', 'no:terminal', test_dir], plugins=[collect_plugin])
+        for nodeid in collect_plugin.collected:
+            print(nodeid)
         return 0
 
     if args.download is None:
@@ -106,15 +110,15 @@  def main():
         return 1
     BRConfigTest.timeout_multiplier = args.timeout_multiplier
 
-    nose2_args = ["-v",
-                  "-N", str(args.testcases),
-                  "-s", test_dir,
+    pytest_args = ["--workers", str(args.testcases),
+                  "--rootdir", test_dir,
                   "-c", os.path.join(test_dir, "conf/unittest.cfg")]
 
     if args.testname:
-        nose2_args += args.testname
+        pytest_args += args.testname
+
+    pytest.main(pytest_args)
 
-    nose2.discover(argv=nose2_args)
 
 
 if __name__ == "__main__":
diff --git a/support/testing/tests/fs/test_iso9660.py b/support/testing/tests/fs/test_iso9660.py
index 692291267e..d2390cccf1 100644
--- a/support/testing/tests/fs/test_iso9660.py
+++ b/support/testing/tests/fs/test_iso9660.py
@@ -25,7 +25,7 @@  BASIC_CONFIG = \
     """.format(infra.filepath("conf/minimal-x86-qemu-kernel.config"))
 
 
-def test_mount_internal_external(emulator, builddir, internal=True, efi=False):
+def do_test_mount_internal_external(emulator, builddir, internal=True, efi=False):
     img = os.path.join(builddir, "images", "rootfs.iso9660")
     if efi:
         efi_img = os.path.join(builddir, "images", "OVMF.fd")
@@ -43,7 +43,7 @@  def test_mount_internal_external(emulator, builddir, internal=True, efi=False):
     return exit_code
 
 
-def test_touch_file(emulator):
+def do_test_touch_file(emulator):
     _, exit_code = emulator.run("touch test")
     return exit_code
 
@@ -63,11 +63,11 @@  class TestIso9660Grub2External(infra.basetest.BRTest):
         """.format(infra.filepath("conf/grub2.cfg"))
 
     def test_run(self):
-        exit_code = test_mount_internal_external(self.emulator,
+        exit_code = do_test_mount_internal_external(self.emulator,
                                                  self.builddir, internal=False)
         self.assertEqual(exit_code, 0)
 
-        exit_code = test_touch_file(self.emulator)
+        exit_code = do_test_touch_file(self.emulator)
         self.assertEqual(exit_code, 1)
 
 
@@ -84,11 +84,11 @@  class TestIso9660Grub2ExternalCompress(infra.basetest.BRTest):
         """.format(infra.filepath("conf/grub2.cfg"))
 
     def test_run(self):
-        exit_code = test_mount_internal_external(self.emulator,
+        exit_code = do_test_mount_internal_external(self.emulator,
                                                  self.builddir, internal=False)
         self.assertEqual(exit_code, 0)
 
-        exit_code = test_touch_file(self.emulator)
+        exit_code = do_test_touch_file(self.emulator)
         self.assertEqual(exit_code, 1)
 
 
@@ -104,11 +104,11 @@  class TestIso9660Grub2Internal(infra.basetest.BRTest):
         """.format(infra.filepath("conf/grub2.cfg"))
 
     def test_run(self):
-        exit_code = test_mount_internal_external(self.emulator,
+        exit_code = do_test_mount_internal_external(self.emulator,
                                                  self.builddir, internal=True)
         self.assertEqual(exit_code, 0)
 
-        exit_code = test_touch_file(self.emulator)
+        exit_code = do_test_touch_file(self.emulator)
         self.assertEqual(exit_code, 0)
 
 
@@ -127,12 +127,12 @@  class TestIso9660Grub2EFI(infra.basetest.BRTest):
                    infra.filepath("conf/grub2.cfg"))
 
     def test_run(self):
-        exit_code = test_mount_internal_external(self.emulator,
+        exit_code = do_test_mount_internal_external(self.emulator,
                                                  self.builddir, internal=True,
                                                  efi=True)
         self.assertEqual(exit_code, 0)
 
-        exit_code = test_touch_file(self.emulator)
+        exit_code = do_test_touch_file(self.emulator)
         self.assertEqual(exit_code, 0)
 
 
@@ -155,22 +155,22 @@  class TestIso9660Grub2Hybrid(infra.basetest.BRTest):
                    infra.filepath("conf/grub2.cfg"))
 
     def test_run(self):
-        exit_code = test_mount_internal_external(self.emulator,
+        exit_code = do_test_mount_internal_external(self.emulator,
                                                  self.builddir, internal=True,
                                                  efi=False)
         self.assertEqual(exit_code, 0)
 
-        exit_code = test_touch_file(self.emulator)
+        exit_code = do_test_touch_file(self.emulator)
         self.assertEqual(exit_code, 0)
 
         self.emulator.stop()
 
-        exit_code = test_mount_internal_external(self.emulator,
+        exit_code = do_test_mount_internal_external(self.emulator,
                                                  self.builddir, internal=True,
                                                  efi=True)
         self.assertEqual(exit_code, 0)
 
-        exit_code = test_touch_file(self.emulator)
+        exit_code = do_test_touch_file(self.emulator)
         self.assertEqual(exit_code, 0)
 
 
@@ -189,11 +189,11 @@  class TestIso9660SyslinuxExternal(infra.basetest.BRTest):
         """.format(infra.filepath("conf/isolinux.cfg"))
 
     def test_run(self):
-        exit_code = test_mount_internal_external(self.emulator,
+        exit_code = do_test_mount_internal_external(self.emulator,
                                                  self.builddir, internal=False)
         self.assertEqual(exit_code, 0)
 
-        exit_code = test_touch_file(self.emulator)
+        exit_code = do_test_touch_file(self.emulator)
         self.assertEqual(exit_code, 1)
 
 
@@ -209,11 +209,11 @@  class TestIso9660SyslinuxExternalCompress(infra.basetest.BRTest):
         """.format(infra.filepath("conf/isolinux.cfg"))
 
     def test_run(self):
-        exit_code = test_mount_internal_external(self.emulator,
+        exit_code = do_test_mount_internal_external(self.emulator,
                                                  self.builddir, internal=False)
         self.assertEqual(exit_code, 0)
 
-        exit_code = test_touch_file(self.emulator)
+        exit_code = do_test_touch_file(self.emulator)
         self.assertEqual(exit_code, 1)
 
 
@@ -228,9 +228,9 @@  class TestIso9660SyslinuxInternal(infra.basetest.BRTest):
         """.format(infra.filepath("conf/isolinux.cfg"))
 
     def test_run(self):
-        exit_code = test_mount_internal_external(self.emulator,
+        exit_code = do_test_mount_internal_external(self.emulator,
                                                  self.builddir, internal=True)
         self.assertEqual(exit_code, 0)
 
-        exit_code = test_touch_file(self.emulator)
+        exit_code = do_test_touch_file(self.emulator)
         self.assertEqual(exit_code, 0)