support/testing: standardize defconfig fragments style

Postpone the strip out of leading spaces in defconfig fragments from the __init__ to the setUp method. It allows test cases to post-process the defconfig in their own __init__ before calling the __init__ method from the base class.

Indent the only fragment in the tree that currently need this (in the ccache test case), taking advantage of
"cf3cd4388a support/tests: allow properly indented config fragment".

Signed-off-by Ricardo Martincoski <[email protected]>

---
Tested by hacking support/testing/infra/builder.py
@@ -30,2 +30,3 @@ class Builder(object):
raise SystemError("Cannot olddefconfig")
+ raise SystemError("Stop")
and diffing the resulting *-build.log files against old ones: few empty lines are added/removed. Also tested diffing the resulting .config files against old ones: they are the same.

Warnings from flake8 change from 100 to 99.

---
 support/testing/infra/basetest.py                | 4 ++--
 support/testing/tests/toolchain/test_external.py | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

support/testing/infra/basetest.py +2 -2

index d205119b2c..c41852949a 100644
@@ -42,8 +42,6 @@ class BRTest(unittest.TestCase):
         self.testname = self.__class__.__name__
         self.builddir = self.outputdir and os.path.join(self.outputdir, self.testname)
         self.emulator = None
-        self.config = '\n'.join([line.lstrip() for line in
-                                 self.config.splitlines()]) + '\n'
         self.config += "BR2_JLEVEL={}\n".format(self.jlevel)
 
     def show_msg(self, msg):

@@ -51,6 +49,8 @@ class BRTest(unittest.TestCase):
                                     self.testname, msg)
     def setUp(self):
         self.show_msg("Starting")
+        self.config = '\n'.join([line.lstrip() for line in
+                                 self.config.splitlines()]) + '\n'
         self.b = Builder(self.config, self.builddir, self.logtofile)
 
         if not self.keepbuilds:

support/testing/tests/toolchain/test_external.py +4 -4

index 1bb5e9497b..ad2f56a20e 100644
@@ -232,10 +232,10 @@ class TestExternalToolchainBuildrootuClibc(TestExternalToolchain):
 
 class TestExternalToolchainCCache(TestExternalToolchainBuildrootuClibc):
     extraconfig = \
-"""
-BR2_CCACHE=y
-BR2_CCACHE_DIR="{builddir}/ccache-dir"
-"""
+        """
+        BR2_CCACHE=y
+        BR2_CCACHE_DIR="{builddir}/ccache-dir"
+        """
 
     def __init__(self, names):
         super(TestExternalToolchainBuildrootuClibc, self).__init__(names)

Signature · 1 line

2.13.0

On 2017-09-28 23:27 -0300, Ricardo Martincoski spake thusly:

Quoted 4 lines

Postpone the strip out of leading spaces in defconfig fragments from the __init__ to the setUp method. It allows test cases to post-process the defconfig in their own __init__ before calling the __init__ method from the base class.

Ideally, this should have been the very first patch in the series.

Quoted 4 lines

Indent the only fragment in the tree that currently need this (in the ccache test case), taking advantage of
"cf3cd4388a support/tests: allow properly indented config fragment".

And then that wcould have gone in the other patch without any issue.

Once you split and reorder the patches, you can add my:

    Acked-by: "Yann E. MORIN" <[email protected]>

Quoted 18 lines

+++ b/support/testing/infra/basetest.py

support/testing/infra/basetest.py +2 -2

@@ -42,8 +42,6 @@ class BRTest(unittest.TestCase):
         self.testname = self.__class__.__name__
         self.builddir = self.outputdir and os.path.join(self.outputdir, self.testname)
         self.emulator = None
-        self.config = '\n'.join([line.lstrip() for line in
-                                 self.config.splitlines()]) + '\n'
         self.config += "BR2_JLEVEL={}\n".format(self.jlevel)
 
     def show_msg(self, msg):

@@ -51,6 +49,8 @@ class BRTest(unittest.TestCase):
                                     self.testname, msg)
     def setUp(self):
         self.show_msg("Starting")
+        self.config = '\n'.join([line.lstrip() for line in
+                                 self.config.splitlines()]) + '\n'
         self.b = Builder(self.config, self.builddir, self.logtofile)
 

support/testing/infra/basetest.py class BRTest(unittest.TestCase): · line 56

Quoted 1 line

support/testing/infra/basetest.py +0 -0

         if not self.keepbuilds:

Quoted 7 lines

On 2017-09-28 23:27 -0300, Ricardo Martincoski spake thusly:

Quoted 4 lines

Postpone the strip out of leading spaces in defconfig fragments from the __init__ to the setUp method. It allows test cases to post-process the defconfig in their own __init__ before calling the __init__ method from the base class.

Ideally, this should have been the very first patch in the series.

Good idea. I will do, but...

Quoted 5 lines

Quoted 4 lines

Indent the only fragment in the tree that currently need this (in the ccache test case), taking advantage of
"cf3cd4388a support/tests: allow properly indented config fragment".

Quoted 1 line

And then that wcould have gone in the other patch without any issue.

It should. But the reordering shows the jlevel handling is fragile.

Both the rootfs overlay and yaffs2 tests currently have defconfig fragments that don't end in a newline, so the new patch 1 ends up silently disabling jlevel:
...s/core/rootfs-overlay2"BR2_JLEVEL=1 BR2_TARGET_ROOTFS_YAFFS2=yBR2_JLEVEL=1

So I plan to add another patch before the series, to make jlevel handling more robust (to other changes in the infra) and at same time use the same style.

          self.config += \
              """
              BR2_JLEVEL={}
              """.format(self.jlevel)

It also made me realize that this patch implies that I need to resend another one (tests for the git downloader) since there I override the setUp method.
http://patchwork.ozlabs.org/patch/806161/
I would need to duplicate there the logic to remove the indentation of defconfig fragments.

Perhaps instead of moving the logic to the setUp of BRTest I could move it to the builder init. This way any new test class that reimplements setUp and uses builder will "inherit" it. Those that reimplement setUp but don't use builder don't need that code.
class Builder(object):
def __init__(self, config, builddir, logtofile):
self.config = '\n'.join([line.lstrip() for line in
config.splitlines()]) + '\n'
What do you think?

On 2017-10-01 22:03 -0300, Ricardo Martincoski spake thusly:

Quoted 19 lines · Yann E. MORIN

Quoted 7 lines

On 2017-09-28 23:27 -0300, Ricardo Martincoski spake thusly:

Quoted 4 lines

Postpone the strip out of leading spaces in defconfig fragments from the __init__ to the setUp method. It allows test cases to post-process the defconfig in their own __init__ before calling the __init__ method from the base class.

Ideally, this should have been the very first patch in the series.

Good idea. I will do, but...

Quoted 6 lines

Quoted 4 lines

Indent the only fragment in the tree that currently need this (in the ccache test case), taking advantage of
"cf3cd4388a support/tests: allow properly indented config fragment".

And then that wcould have gone in the other patch without any issue.

It should. But the reordering shows the jlevel handling is fragile.

Quoted 5 lines

Both the rootfs overlay and yaffs2 tests currently have defconfig fragments that don't end in a newline, so the new patch 1 ends up silently disabling jlevel:
...s/core/rootfs-overlay2"BR2_JLEVEL=1 BR2_TARGET_ROOTFS_YAFFS2=yBR2_JLEVEL=1

Weird, because on line 46, we explicitly add a trainling '\n' :
https://git.buildroot.org/buildroot/tree/support/testing/infra/basetest.py#n45

   45:  self.config = '\n'.join([line.lstrip() for line in
                                self.config.splitlines()]) + '\n'
   46:  self.config += "BR2_JLEVEL={}\n".format(self.jlevel)

Quoted 7 lines

So I plan to add another patch before the series, to make jlevel handling more robust (to other changes in the infra) and at same time use the same style.

          self.config += \
              """
              BR2_JLEVEL={}
              """.format(self.jlevel)

Well, I doubt this is useful. Just prepend a \n, if it really is useful :

    self.config += "\nBR2_JLEVEL={}\n".format(self.jlevel)

Quoted 7 lines

It also made me realize that this patch implies that I need to resend another one (tests for the git downloader) since there I override the setUp method.
http://patchwork.ozlabs.org/patch/806161/
I would need to duplicate there the logic to remove the indentation of defconfig fragments.

Quoted 9 lines

Perhaps instead of moving the logic to the setUp of BRTest I could move it to the builder init. This way any new test class that reimplements setUp and uses builder will "inherit" it. Those that reimplement setUp but don't use builder don't need that code.
class Builder(object):
def __init__(self, config, builddir, logtofile):
self.config = '\n'.join([line.lstrip() for line in
config.splitlines()]) + '\n'
What do you think?

Yes, that would probably be better.

Fix the trivial warnings from flake8:
- remove modules imported but unused;
- use 2 lines before class or module level method;
- remove blank line at end of file.

Signed-off-by Ricardo Martincoski <[email protected]>

---
Warnings from flake8 change from 99 to 17.

---
 support/testing/infra/__init__.py                 |  7 ++++++-
 support/testing/infra/basetest.py                 |  2 ++
 support/testing/infra/builder.py                  |  1 +
 support/testing/run-tests                         |  2 ++
 support/testing/tests/core/test_post_scripts.py   |  1 +
 support/testing/tests/core/test_rootfs_overlay.py |  2 ++
 support/testing/tests/core/test_timezone.py       |  4 ++++
 support/testing/tests/fs/test_ext.py              |  9 +++++++--
 support/testing/tests/fs/test_iso9660.py          |  8 ++++++--
 support/testing/tests/fs/test_jffs2.py            |  2 ++
 support/testing/tests/fs/test_squashfs.py         |  1 +
 support/testing/tests/fs/test_ubi.py              |  1 +
 support/testing/tests/fs/test_yaffs2.py           |  1 +
 support/testing/tests/init/base.py                |  1 +
 support/testing/tests/init/test_busybox.py        |  5 +----
 support/testing/tests/init/test_none.py           |  1 +
 support/testing/tests/init/test_systemd.py        |  7 +------
 support/testing/tests/package/test_dropbear.py    |  1 +
 support/testing/tests/package/test_ipython.py     |  8 +++-----
 support/testing/tests/package/test_python.py      |  5 +++++
 support/testing/tests/toolchain/test_external.py  | 10 ++++++++++
 21 files changed, 59 insertions(+), 20 deletions(-)

support/testing/infra/__init__.py +6 -1

index 27e2a2708d..b03e891771 100644
@@ -1,4 +1,3 @@
-import contextlib
 import os
 import re
 import sys

@@ -8,6 +7,7 @@ from urllib2 import urlopen, HTTPError, URLError
 
 ARTIFACTS_URL = "http://autobuild.buildroot.net/artefacts/"
 
+
 def open_log_file(builddir, stage, logtofile=True):
     """
     Open a file for logging and return its handler.

@@ -20,9 +20,11 @@ def open_log_file(builddir, stage, logtofile=True):
         fhandle = sys.stdout
     return fhandle
 
+
 def filepath(relpath):
     return os.path.join(os.getcwd(), "support/testing", relpath)
 
+
 def download(dldir, filename):
     finalpath = os.path.join(dldir, filename)
     if os.path.exists(finalpath):

@@ -46,6 +48,7 @@ def download(dldir, filename):
     os.rename(tmpfile, finalpath)
     return finalpath
 
+
 def get_elf_arch_tag(builddir, prefix, fpath, tag):
     """
     Runs the cross readelf on 'fpath', then extracts the value of tag 'tag'.

@@ -66,9 +69,11 @@ def get_elf_arch_tag(builddir, prefix, fpath, tag):
         return m.group(1)
     return None
 
+
 def get_file_arch(builddir, prefix, fpath):
     return get_elf_arch_tag(builddir, prefix, fpath, "Tag_CPU_arch")
 
+
 def get_elf_prog_interpreter(builddir, prefix, fpath):
     """
     Runs the cross readelf on 'fpath' to extract the program interpreter

support/testing/infra/basetest.py +2 -0

index c41852949a..9d5f714b02 100644
@@ -28,6 +28,7 @@ MINIMAL_CONFIG = \
     # BR2_TARGET_ROOTFS_TAR is not set
     """
 
+
 class BRTest(unittest.TestCase):
     config = None
     downloaddir = None

@@ -47,6 +48,7 @@ class BRTest(unittest.TestCase):
     def show_msg(self, msg):
         print "{} {:40s} {}".format(datetime.datetime.now().strftime("%H:%M:%S"),
                                     self.testname, msg)
+
     def setUp(self):
         self.show_msg("Starting")
         self.config = '\n'.join([line.lstrip() for line in

support/testing/infra/builder.py +1 -0

index 905b127c91..ce1ec74029 100644
@@ -4,6 +4,7 @@ import subprocess
 
 import infra
 
+
 class Builder(object):
     def __init__(self, config, builddir, logtofile):
         self.config = config

support/testing/run-tests +2 -0

index ae0bd336b5..270e78cff7 100755
@@ -7,6 +7,7 @@ import multiprocessing
 
 from infra.basetest import BRTest
 
+
 def main():
     parser = argparse.ArgumentParser(description='Run Buildroot tests')
     parser.add_argument('testname', nargs='*',

@@ -116,5 +117,6 @@ def main():
 
     nose2.discover(argv=nose2_args)
 
+
 if __name__ == "__main__":
     sys.exit(main())

support/testing/tests/core/test_post_scripts.py +1 -0

index 7ca9b9d836..1db568b0d6 100644
@@ -3,6 +3,7 @@ import csv
 
 import infra.basetest
 
+
 class TestPostScripts(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         """

support/testing/tests/core/test_rootfs_overlay.py +2 -0

index 22ef9722d6..fedd40d8ac 100644
@@ -3,9 +3,11 @@ import subprocess
 
 import infra.basetest
 
+
 def compare_file(file1, file2):
     return subprocess.call(["cmp", file1, file2])
 
+
 class TestRootfsOverlay(infra.basetest.BRTest):
 
     rootfs_overlay_path = infra.filepath("tests/core/rootfs-overlay")

support/testing/tests/core/test_timezone.py +4 -0

index f4ba5039ca..050624e0aa 100644
@@ -2,12 +2,14 @@ import os
 
 import infra.basetest
 
+
 def boot_armv5_cpio(emulator, builddir):
         img = os.path.join(builddir, "images", "rootfs.cpio")
         emulator.boot(arch="armv5", kernel="builtin",
                       options=["-initrd", img])
         emulator.login()
 
+
 class TestNoTimezone(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         """

@@ -23,6 +25,7 @@ class TestNoTimezone(infra.basetest.BRTest):
         tz, _ = self.emulator.run("TZ=America/Los_Angeles date +%Z")
         self.assertEqual(tz[0].strip(), "UTC")
 
+
 class TestGlibcAllTimezone(infra.basetest.BRTest):
     config = \
         """

@@ -44,6 +47,7 @@ class TestGlibcAllTimezone(infra.basetest.BRTest):
         tz, _ = self.emulator.run("TZ=Europe/Paris date +%Z")
         self.assertEqual(tz[0].strip(), "CET")
 
+
 class TestGlibcNonDefaultLimitedTimezone(infra.basetest.BRTest):
     config = \
         """

support/testing/tests/fs/test_ext.py +7 -2

index 49bce45dc8..f5f9e9fdf1 100644
@@ -12,6 +12,7 @@ RESBLKCNT_PROP = "Reserved block count"
 
 CHECK_FS_TYPE_CMD = "mount | grep '/dev/root on / type {}'"
 
+
 def dumpe2fs_run(builddir, image):
     cmd = ["host/sbin/dumpe2fs", os.path.join("images", image)]
     ret = subprocess.check_output(cmd,

@@ -20,12 +21,14 @@ def dumpe2fs_run(builddir, image):
                                   env={"LANG": "C"})
     return ret.strip().splitlines()
 
+
 def dumpe2fs_getprop(out, prop):
     for line in out:
         fields = line.split(": ")
         if fields[0] == prop:
             return fields[1].strip()
 
+
 def boot_img_and_check_fs_type(emulator, builddir, fs_type):
     img = os.path.join(builddir, "images", "rootfs.{}".format(fs_type))
     emulator.boot(arch="armv7",

@@ -37,6 +40,7 @@ def boot_img_and_check_fs_type(emulator, builddir, fs_type):
     _, exit_code = emulator.run(CHECK_FS_TYPE_CMD.format(fs_type))
     return exit_code
 
+
 class TestExt2(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         """

@@ -55,6 +59,7 @@ class TestExt2(infra.basetest.BRTest):
                                                self.builddir, "ext2")
         self.assertEqual(exit_code, 0)
 
+
 class TestExt2r1(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         """

@@ -74,6 +79,7 @@ class TestExt2r1(infra.basetest.BRTest):
                                                self.builddir, "ext2")
         self.assertEqual(exit_code, 0)
 
+
 class TestExt3(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         """

@@ -92,6 +98,7 @@ class TestExt3(infra.basetest.BRTest):
                                                self.builddir, "ext3")
         self.assertEqual(exit_code, 0)
 
+
 class TestExt4(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         """

@@ -116,5 +123,3 @@ class TestExt4(infra.basetest.BRTest):
         exit_code = boot_img_and_check_fs_type(self.emulator,
                                                self.builddir, "ext4")
         self.assertEqual(exit_code, 0)
-
-

support/testing/tests/fs/test_iso9660.py +6 -2

index 60dcbdc139..41430eb4d2 100644
@@ -25,6 +25,7 @@ BASIC_CONFIG = \
     # BR2_TARGET_ROOTFS_TAR is not set
     """.format(infra.filepath("conf/minimal-x86-qemu-kernel.config"))
 
+
 def test_mount_internal_external(emulator, builddir, internal=True):
     img = os.path.join(builddir, "images", "rootfs.iso9660")
     emulator.boot(arch="i386", options=["-cdrom", img])

@@ -38,13 +39,14 @@ def test_mount_internal_external(emulator, builddir, internal=True):
     _, exit_code = emulator.run(cmd)
     return exit_code
 
+
 def test_touch_file(emulator):
     _, exit_code = emulator.run("touch test")
     return exit_code
 
 #
 # Grub 2
-#
+
 
 class TestIso9660Grub2External(infra.basetest.BRTest):
     config = BASIC_CONFIG + \

@@ -65,6 +67,7 @@ class TestIso9660Grub2External(infra.basetest.BRTest):
         exit_code = test_touch_file(self.emulator)
         self.assertEqual(exit_code, 1)
 
+
 class TestIso9660Grub2Internal(infra.basetest.BRTest):
     config = BASIC_CONFIG + \
         """

@@ -86,7 +89,7 @@ class TestIso9660Grub2Internal(infra.basetest.BRTest):
 
 #
 # Syslinux
-#
+
 
 class TestIso9660SyslinuxExternal(infra.basetest.BRTest):
     config = BASIC_CONFIG + \

@@ -106,6 +109,7 @@ class TestIso9660SyslinuxExternal(infra.basetest.BRTest):
         exit_code = test_touch_file(self.emulator)
         self.assertEqual(exit_code, 1)
 
+
 class TestIso9660SyslinuxInternal(infra.basetest.BRTest):
     config = BASIC_CONFIG + \
         """

support/testing/tests/fs/test_jffs2.py +2 -0

index a84c858238..2ff5099180 100644
@@ -3,6 +3,7 @@ import subprocess
 
 import infra.basetest
 
+
 def jffs2dump_find_file(files_list, fname):
     for file_name in files_list:
         file_name = file_name.strip()

@@ -10,6 +11,7 @@ def jffs2dump_find_file(files_list, fname):
             return True
     return False
 
+
 class TestJffs2(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         """

support/testing/tests/fs/test_squashfs.py +1 -0

index 9fad28f834..066c054342 100644
@@ -3,6 +3,7 @@ import subprocess
 
 import infra.basetest
 
+
 class TestSquashfs(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         """

support/testing/tests/fs/test_ubi.py +1 -0

index 360932a0bf..015d82f769 100644
@@ -3,6 +3,7 @@ import os
 
 import infra.basetest
 
+
 class TestUbi(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         """

support/testing/tests/fs/test_yaffs2.py +1 -0

index a11c1a40a7..b60e90e660 100644
@@ -2,6 +2,7 @@ import os
 
 import infra.basetest
 
+
 class TestYaffs2(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         infra.basetest.MINIMAL_CONFIG + \

support/testing/tests/init/base.py +1 -0

index a261d7dd46..c09ee46eb0 100644
@@ -2,6 +2,7 @@ import os
 import subprocess
 import infra.basetest
 
+
 class InitSystemBase(infra.basetest.BRTest):
 
     def startEmulator(self, fs_type, kernel=None, dtb=None, init=None):

support/testing/tests/init/test_busybox.py +1 -4

index c3e425bf5d..6c75f685ad 100644
@@ -1,6 +1,7 @@
 import infra.basetest
 from tests.init.base import InitSystemBase as InitSystemBase
 
+
 class InitSystemBusyboxBase(InitSystemBase):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         """

@@ -11,7 +12,6 @@ class InitSystemBusyboxBase(InitSystemBase):
         super(InitSystemBusyboxBase, self).checkInit("/bin/busybox")
 
 
-#-------------------------------------------------------------------------------
 class TestInitSystemBusyboxRo(InitSystemBusyboxBase):
     config = InitSystemBusyboxBase.config + \
         """

@@ -25,7 +25,6 @@ class TestInitSystemBusyboxRo(InitSystemBusyboxBase):
         self.checkNetwork("eth0", 1)
 
 
-#-------------------------------------------------------------------------------
 class TestInitSystemBusyboxRw(InitSystemBusyboxBase):
     config = InitSystemBusyboxBase.config + \
         """

@@ -38,7 +37,6 @@ class TestInitSystemBusyboxRw(InitSystemBusyboxBase):
         self.checkNetwork("eth0", 1)
 
 
-#-------------------------------------------------------------------------------
 class TestInitSystemBusyboxRoNet(InitSystemBusyboxBase):
     config = InitSystemBusyboxBase.config + \
         """

@@ -53,7 +51,6 @@ class TestInitSystemBusyboxRoNet(InitSystemBusyboxBase):
         self.checkNetwork("eth0")
 
 
-#-------------------------------------------------------------------------------
 class TestInitSystemBusyboxRwNet(InitSystemBusyboxBase):
     config = InitSystemBusyboxBase.config + \
         """

support/testing/tests/init/test_none.py +1 -0

index f55db5d3e0..c8a79f0bab 100644
@@ -3,6 +3,7 @@ import pexpect
 import infra.basetest
 from tests.init.base import InitSystemBase as InitSystemBase
 
+
 class TestInitSystemNone(InitSystemBase):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         """

support/testing/tests/init/test_systemd.py +1 -6

index b1b6517373..77d734895b 100644
@@ -1,6 +1,7 @@
 import infra.basetest
 from tests.init.base import InitSystemBase as InitSystemBase
 
+
 class InitSystemSystemdBase(InitSystemBase):
     config = \
         """

@@ -21,7 +22,6 @@ class InitSystemSystemdBase(InitSystemBase):
         super(InitSystemSystemdBase, self).checkInit("/lib/systemd/systemd")
 
 
-#-------------------------------------------------------------------------------
 class TestInitSystemSystemdRoNetworkd(InitSystemSystemdBase):
     config = InitSystemSystemdBase.config + \
         """

@@ -43,7 +43,6 @@ class TestInitSystemSystemdRoNetworkd(InitSystemSystemdBase):
         self.assertEqual(out[0], "foobar")
 
 
-#-------------------------------------------------------------------------------
 class TestInitSystemSystemdRwNetworkd(InitSystemSystemdBase):
     config = InitSystemSystemdBase.config + \
         """

@@ -57,7 +56,6 @@ class TestInitSystemSystemdRwNetworkd(InitSystemSystemdBase):
         self.checkNetwork("eth0")
 
 
-#-------------------------------------------------------------------------------
 class TestInitSystemSystemdRoIfupdown(InitSystemSystemdBase):
     config = InitSystemSystemdBase.config + \
         """

@@ -73,7 +71,6 @@ class TestInitSystemSystemdRoIfupdown(InitSystemSystemdBase):
         self.checkNetwork("eth0")
 
 
-#-------------------------------------------------------------------------------
 class TestInitSystemSystemdRwIfupdown(InitSystemSystemdBase):
     config = InitSystemSystemdBase.config + \
         """

@@ -89,7 +86,6 @@ class TestInitSystemSystemdRwIfupdown(InitSystemSystemdBase):
         self.checkNetwork("eth0")
 
 
-#-------------------------------------------------------------------------------
 class TestInitSystemSystemdRoFull(InitSystemSystemdBase):
     config = InitSystemSystemdBase.config + \
         """

@@ -121,7 +117,6 @@ class TestInitSystemSystemdRoFull(InitSystemSystemdBase):
         self.checkNetwork("eth0")
 
 
-#-------------------------------------------------------------------------------
 class TestInitSystemSystemdRwFull(InitSystemSystemdBase):
     config = InitSystemSystemdBase.config + \
         """

support/testing/tests/package/test_dropbear.py +1 -0

index da569d9b1b..8f7f1fee82 100644
@@ -2,6 +2,7 @@ import os
 
 import infra.basetest
 
+
 class TestDropbear(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         """

support/testing/tests/package/test_ipython.py +3 -5

index a943101372..3b291d9583 100644
@@ -1,5 +1,3 @@
-import os
-
 from tests.package.test_python import TestPythonBase
 #
 # The following pythong tests are not being used here:

@@ -8,7 +6,8 @@ from tests.package.test_python import TestPythonBase
 #
 # - zlib_test: IPython does not return a non-zero code the way CPython
 #              does, so this test ends up being a false-negative
-#
+
+
 class TestIPythonPy2(TestPythonBase):
     config = TestPythonBase.config + \
         """

@@ -22,6 +21,7 @@ class TestIPythonPy2(TestPythonBase):
         self.math_floor_test(40)
         self.libc_time_test(40)
 
+
 class TestIPythonPy3(TestPythonBase):
     config = TestPythonBase.config + \
         """

@@ -34,5 +34,3 @@ class TestIPythonPy3(TestPythonBase):
         self.login()
         self.math_floor_test(40)
         self.libc_time_test(40)
-
-

support/testing/tests/package/test_python.py +5 -0

index 79773fff63..26cf49947b 100644
@@ -2,6 +2,7 @@ import os
 
 import infra.basetest
 
+
 class TestPythonBase(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
         """

@@ -40,11 +41,13 @@ class TestPythonBase(infra.basetest.BRTest):
         _, exit_code = self.emulator.run(cmd, timeout)
         self.assertEqual(exit_code, 1)
 
+
 class TestPython2(TestPythonBase):
     config = TestPythonBase.config + \
         """
         BR2_PACKAGE_PYTHON=y
         """
+
     def test_run(self):
         self.login()
         self.version_test("Python 2")

@@ -52,11 +55,13 @@ class TestPython2(TestPythonBase):
         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")

support/testing/tests/toolchain/test_external.py +10 -0

index ad2f56a20e..072cf0048e 100644
@@ -7,6 +7,7 @@ BASIC_CONFIG = \
     # BR2_TARGET_ROOTFS_TAR is not set
     """
 
+
 def has_broken_links(path):
     for root, dirs, files in os.walk(path):
         for f in files:

@@ -15,6 +16,7 @@ def has_broken_links(path):
                 return True
     return False
 
+
 class TestExternalToolchain(infra.basetest.BRTest):
     def common_check(self):
         # Check for broken symlinks

@@ -30,6 +32,7 @@ class TestExternalToolchain(infra.basetest.BRTest):
         interp_path = os.path.join(self.builddir, "target", interp[1:])
         self.assertTrue(os.path.exists(interp_path))
 
+
 class TestExternalToolchainSourceryArmv4(TestExternalToolchain):
     config = BASIC_CONFIG + \
         """

@@ -61,6 +64,7 @@ class TestExternalToolchainSourceryArmv4(TestExternalToolchain):
                            options=["-initrd", img])
         self.emulator.login()
 
+
 class TestExternalToolchainSourceryArmv5(TestExternalToolchain):
     config = BASIC_CONFIG + \
         """

@@ -86,6 +90,7 @@ class TestExternalToolchainSourceryArmv5(TestExternalToolchain):
                            options=["-initrd", img])
         self.emulator.login()
 
+
 class TestExternalToolchainSourceryArmv7(TestExternalToolchain):
     config = BASIC_CONFIG + \
         """

@@ -124,6 +129,7 @@ class TestExternalToolchainSourceryArmv7(TestExternalToolchain):
                            options=["-initrd", img])
         self.emulator.login()
 
+
 class TestExternalToolchainLinaroArm(TestExternalToolchain):
     config = BASIC_CONFIG + \
         """

@@ -155,6 +161,7 @@ class TestExternalToolchainLinaroArm(TestExternalToolchain):
                            options=["-initrd", img])
         self.emulator.login()
 
+
 class TestExternalToolchainBuildrootMusl(TestExternalToolchain):
     config = BASIC_CONFIG + \
         """

@@ -180,6 +187,7 @@ class TestExternalToolchainBuildrootMusl(TestExternalToolchain):
                            options=["-initrd", img])
         self.emulator.login()
 
+
 class TestExternalToolchainCtngMusl(TestExternalToolchain):
     config = BASIC_CONFIG + \
         """

@@ -206,6 +214,7 @@ class TestExternalToolchainCtngMusl(TestExternalToolchain):
                            options=["-initrd", img])
         self.emulator.login()
 
+
 class TestExternalToolchainBuildrootuClibc(TestExternalToolchain):
     config = BASIC_CONFIG + \
         """

@@ -230,6 +239,7 @@ class TestExternalToolchainBuildrootuClibc(TestExternalToolchain):
                            options=["-initrd", img])
         self.emulator.login()
 
+
 class TestExternalToolchainCCache(TestExternalToolchainBuildrootuClibc):
     extraconfig = \
         """

Signature · 1 line

2.13.0

Fix the remaining code style warnings from flake8:
- break lines at 79 columns;
- replace some long lines by equivalent code;
- make flake8 ignore some long lines that need to be that way;
- properly indent continuation lines;
- use simpler code to test a parameter is not None.

Signed-off-by Ricardo Martincoski <[email protected]>

---
Warnings from flake8 change from 17 to 0.

---
 support/testing/infra/basetest.py                 |  7 ++++---
 support/testing/tests/core/test_post_scripts.py   | 15 ++++++++++-----
 support/testing/tests/core/test_rootfs_overlay.py |  3 ++-
 support/testing/tests/fs/test_iso9660.py          |  4 ++--
 support/testing/tests/fs/test_squashfs.py         |  9 ++++++---
 support/testing/tests/init/base.py                |  6 +++---
 support/testing/tests/init/test_none.py           |  4 +++-
 7 files changed, 30 insertions(+), 18 deletions(-)

support/testing/infra/basetest.py +0 -0

index 9d5f714b02..045f5e80a4 100644

support/testing/tests/fs/test_squashfs.py +2 -1

         self.assertEqual(out[3], "Compression lz4")
 
         img = os.path.join(self.builddir, "images", "rootfs.squashfs")

@@ -30,7 +32,8 @@ class TestSquashfs(infra.basetest.BRTest):
                            kernel="builtin",
                            kernel_cmdline=["root=/dev/mmcblk0",
                                            "rootfstype=squashfs"],
-                           options=["-drive", "file={},if=sd,format=raw".format(img)])
+                           options=["-drive",
+                                    "file={},if=sd,format=raw".format(img)])
         self.emulator.login()
 
         cmd = "mount | grep '/dev/root on / type squashfs'"

support/testing/tests/init/base.py +3 -3

index c09ee46eb0..3764d6ab95 100644
@@ -6,7 +6,7 @@ import infra.basetest
 class InitSystemBase(infra.basetest.BRTest):
 
     def startEmulator(self, fs_type, kernel=None, dtb=None, init=None):
-        img = os.path.join(self.builddir, "images", "rootfs.{}".format(fs_type))
+        img = os.path.join(self.builddir, "images", "rootfs." + fs_type)
         subprocess.call(["truncate", "-s", "%1M", img])
 
         options = ["-drive",

@@ -18,7 +18,7 @@ class InitSystemBase(infra.basetest.BRTest):
         else:
             kernel = os.path.join(self.builddir, "images", kernel)
             options.extend(["-dtb", os.path.join(self.builddir, "images",
-                                             "{}.dtb".format(dtb))])
+                                                 "{}.dtb".format(dtb))])
 
         kernel_cmdline = ["root=/dev/mmcblk0",
                           "rootfstype={}".format(fs_type),

@@ -26,7 +26,7 @@ class InitSystemBase(infra.basetest.BRTest):
                           "ro",
                           "console=ttyAMA0"]
 
-        if not init is None:
+        if init:
             kernel_cmdline.extend(["init={}".format(init)])
 
         self.emulator.boot(arch="armv7",

support/testing/tests/init/test_none.py +3 -1

index c8a79f0bab..22e4850853 100644
@@ -14,7 +14,9 @@ class TestInitSystemNone(InitSystemBase):
 
     def test_run(self):
         self.startEmulator(fs_type="squashfs", init="/bin/sh")
-        index = self.emulator.qemu.expect(["/bin/sh: can't access tty; job control turned off", pexpect.TIMEOUT], timeout=60)
+        index = self.emulator.qemu.expect(["/bin/sh: can't access tty; "
+                                           "job control turned off",
+                                           pexpect.TIMEOUT], timeout=60)
         if index != 0:
             self.emulator.logfile.write("==> System does not boot")
             raise SystemError("System does not boot")

Signature · 1 line

2.13.0

On 2017-09-28 23:27 -0300, Ricardo Martincoski spake thusly:

Quoted 6 lines

support/testing/infra/basetest.py +0 -0

index 9d5f714b02..045f5e80a4 100644
@@ -41,13 +41,14 @@ class BRTest(unittest.TestCase):
     def __init__(self, names):

support/testing/infra/basetest.py class BRTest(unittest.TestCase): · line 45

Quoted 5 lines

support/testing/infra/basetest.py +2 -1

         super(BRTest, self).__init__(names)
         self.testname = self.__class__.__name__
-        self.builddir = self.outputdir and os.path.join(self.outputdir, self.testname)
+        self.builddir = self.outputdir and os.path.join(self.outputdir,
+                                                        self.testname)

I think that in such situation, a better break would be right after the 'and' operator [...]

On 2017-09-28 23:27 -0300, Ricardo Martincoski spake thusly:

Quoted 6 lines

Fix the remaining code style warnings from flake8:
- break lines at 79 columns;
- replace some long lines by equivalent code;
- make flake8 ignore some long lines that need to be that way;
- properly indent continuation lines;
- use simpler code to test a parameter is not None.

NACK for that last one. PEP8 https://www.python.org/dev/peps/pep-0008/
says:

---8<---
Also, beware of writing if x when you really mean if x is not None -- e.g. when testing whether a variable or argument that defaults to None was set to some other value. The other value might have a type (such as a container) that could be false in a boolean context! ---8<---

But you are right in a sense, in that we should write "if x is not None" instead of the current "if not x is None".

And that is exactly what flake8 reports, by the way. ;-)

support/testing/infra/basetest.py class BRTest(unittest.TestCase): · line 45

Quoted 8 lines

+++ b/support/testing/infra/basetest.py

support/testing/infra/basetest.py +2 -1

@@ -41,13 +41,14 @@ class BRTest(unittest.TestCase):
     def __init__(self, names):
         super(BRTest, self).__init__(names)
         self.testname = self.__class__.__name__
-        self.builddir = self.outputdir and os.path.join(self.outputdir, self.testname)
+        self.builddir = self.outputdir and os.path.join(self.outputdir,
+                                                        self.testname)

I think that in such situation, a better break would be right after the 'and' operator (but Python is stupid and errors if there is no explicit line continuation):

    self.builddir = self.outputdir and \
                    os.path.join(self.outputdir, self.testname)

However, I don't understand what this was supposed to do in the first place... If self.outputdir does not evaluate to False, I understand. But if it does evaluate to "False" (in fact, probably because it is "None"), then we assign "None" to self.builddir.

However, we do ensure that self.outputdir is indeed set, because we do require the -o option to be passed.

Hmm... Unless we are just listing the tests, in which case -o is not mandatory... Is that it? If so, this is confusing... :-/

[--SNIP--]

support/testing/tests/core/test_rootfs_overlay.py class TestRootfsOverlay(infra.basetest.BRTest): · line 28

Quoted 11 lines

support/testing/tests/core/test_rootfs_overlay.py +2 -1

index fedd40d8ac..31d6c0fb5e 100644
@@ -24,7 +24,8 @@ class TestRootfsOverlay(infra.basetest.BRTest):
         ret = compare_file(overlay_file, target_file)
         self.assertEqual(ret, 0)
 
-        target_file = os.path.join(self.builddir, "target", "etc", "test-file2")
+        target_file = os.path.join(self.builddir, "target", "etc",
+                                   "test-file2")

Sometimes, rules are stupid... :-/ I would again have found that a better break would have been right between self.builddir and "target", like so:

target_file = os.path.join(self.builddir,
"target", "etc", "test-file2")

But why is the "target/etc/test-file2" path split in the first place? It should probably be a single argument, no?

Or are we afraid that this might have to run on a system where path separator are not forward slashes? (note: only Windows uses backslash as a path separator, and we never claimed we would work on Windows. And if one uses cygwin, then Cygwin works very well with forward slashes. There's just this new fancy WSL stuff, but I haven't touched a Windows in a decade, so I can't say.)

So, back to the point: why not make it a siungle argument?

Especially since we do have such path in a lot of other places all over the tree (e.g. [...].format(infra.filepath("conf/grub2.cfg")) below [0]).

[0] ... here. OK, I made my point, I guess! ;-)

support/testing/tests/core/test_rootfs_overlay.py

Quoted 9 lines

out = subprocess.check_output(unsquashfs_cmd,
cwd=self.builddir, env={"LANG": "C"})
out = out.splitlines() self.assertEqual(out[0],

support/testing/tests/core/test_rootfs_overlay.py +2 -1

-                         "Found a valid SQUASHFS 4:0 superblock on images/rootfs.squashfs.")
+                         "Found a valid SQUASHFS 4:0 superblock on "
+                         "images/rootfs.squashfs.")

This is typically another case where splitting is just nut. Don't; it only brings readabilitiy issues.

But I would like to allow for a bit longer lines, because we already have quite some deeply indented code, and 79 chars wide lines are all the shorter.

So I would suggest we add this file:

    $ cat support/testing/setup.cfg:
    [flake8]
    max-line-length=132

Thoughts?

Quoted 1 line

On 2017-09-28 23:27 -0300, Ricardo Martincoski spake thusly:

Quoted 9 lines

But I would like to allow for a bit longer lines, because we already have quite some deeply indented code, and 79 chars wide lines are all the shorter.

So I would suggest we add this file:

    $ cat support/testing/setup.cfg:
    [flake8]
    max-line-length=132

I didn't know about this option. Nice!

Quoted 1 line

Quoted 1 line

Thoughts?

My personal preference is to use the default. We can add " # noqa" to the exceptions.
The beauty of using default values is that anyone can call the tool without thinking about config files or command line options.

But well... we can add a command line to the manual (as we have for git format-patch), or either add such config file for the test infra and also for all directories that have Python files.

If we follow this path of adopting command line arguments or a config file, don't worry about the test files in the package directories
(http://patchwork.ozlabs.org/patch/814544/), I will send a RFC patch that also checks files pointed by symlinks.

The bikeshedding challenge is to define the magic number: 100? 132? 150?

Quoted 6 lines

If we follow this path of adopting command line arguments or a config file, don't worry about the test files in the package directories
(http://patchwork.ozlabs.org/patch/814544/), I will send a RFC patch that also checks files pointed by symlinks.

The bikeshedding challenge is to define the magic number: 100? 132? 150?

I did not choose 132 randomly; I choose it for a reason.

https://en.wikipedia.org/wiki/132_(number)

The number of columns of a Line printer printing in landscape mode on 14-inch paper. /(while in portrait it was 80)/

Also, 132 was the alternative line width on a VT (vt200? my memory of it is a bit fuzzy... ;-] )

So, 132 is a good fit. ;-)

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Arnout" == Arnout Vandecappelle <[email protected]> writes:

For me, the ideal is to wrap at 80 columns most of the time, but allow
exceptions up to something like 105 columns. 105 is still reasonable for side-by-side on a wide display with some room for scrollbars. I know, Yann, that you can easily fit 132 characters on side-by-side, but you've got an eagle-eyed font size :-) That said, I can live with 132 as the absolute maximum. However, I do think we should _normally_ wrap at 80 columns.

Standardizing on 80 columns is good because it's what we do in the rest of
Buildroot. Allowing exceptions is good because there are some lines you really don't want to split (strings, mostly).

So ideally, I'd like to have a warning for anything about 80 columns, and an
error for anything over 105 (or 132). Unfortunately, I don't think flake8 allows that. So let's say we put 132 in the config file, but enforce 80 columns in review.

Use method naming convention from PEP8 as other test cases already do.

sed \
-e 's,startEmulator,start_emulator,g' \
-e 's,checkInit,check_init,g' \
-e 's,checkNetwork,check_network,g' \
-i support/testing/tests/init/*.py

Signed-off-by Ricardo Martincoski <[email protected]>

---
0 warnings from flake8

---
 support/testing/tests/init/base.py         |  6 ++---
 support/testing/tests/init/test_busybox.py | 28 ++++++++++-----------
 support/testing/tests/init/test_none.py    |  4 +--
 support/testing/tests/init/test_systemd.py | 40 +++++++++++++++---------------
 4 files changed, 39 insertions(+), 39 deletions(-)

support/testing/tests/init/base.py +3 -3

index 3764d6ab95..1f9e33b620 100644
@@ -5,7 +5,7 @@ import infra.basetest
 
 class InitSystemBase(infra.basetest.BRTest):
 
-    def startEmulator(self, fs_type, kernel=None, dtb=None, init=None):
+    def start_emulator(self, fs_type, kernel=None, dtb=None, init=None):
         img = os.path.join(self.builddir, "images", "rootfs." + fs_type)
         subprocess.call(["truncate", "-s", "%1M", img])
 

@@ -37,12 +37,12 @@ class InitSystemBase(infra.basetest.BRTest):
         if init is None:
             self.emulator.login()
 
-    def checkInit(self, path):
+    def check_init(self, path):
         cmd = "cmp /proc/1/exe {}".format(path)
         _, exit_code = self.emulator.run(cmd)
         self.assertEqual(exit_code, 0)
 
-    def checkNetwork(self, interface, exitCode=0):
+    def check_network(self, interface, exitCode=0):
         cmd = "ip addr show {} |grep inet".format(interface)
         _, exit_code = self.emulator.run(cmd)
         self.assertEqual(exit_code, exitCode)

support/testing/tests/init/test_busybox.py +14 -14

index 6c75f685ad..3be4dea35f 100644
@@ -8,8 +8,8 @@ class InitSystemBusyboxBase(InitSystemBase):
         # BR2_TARGET_ROOTFS_TAR is not set
         """
 
-    def checkInit(self):
-        super(InitSystemBusyboxBase, self).checkInit("/bin/busybox")
+    def check_init(self):
+        super(InitSystemBusyboxBase, self).check_init("/bin/busybox")
 
 
 class TestInitSystemBusyboxRo(InitSystemBusyboxBase):

@@ -20,9 +20,9 @@ class TestInitSystemBusyboxRo(InitSystemBusyboxBase):
         """
 
     def test_run(self):
-        self.startEmulator("squashfs")
-        self.checkInit()
-        self.checkNetwork("eth0", 1)
+        self.start_emulator("squashfs")
+        self.check_init()
+        self.check_network("eth0", 1)
 
 
 class TestInitSystemBusyboxRw(InitSystemBusyboxBase):

@@ -32,9 +32,9 @@ class TestInitSystemBusyboxRw(InitSystemBusyboxBase):
         """
 
     def test_run(self):
-        self.startEmulator("ext2")
-        self.checkInit()
-        self.checkNetwork("eth0", 1)
+        self.start_emulator("ext2")
+        self.check_init()
+        self.check_network("eth0", 1)
 
 
 class TestInitSystemBusyboxRoNet(InitSystemBusyboxBase):

@@ -46,9 +46,9 @@ class TestInitSystemBusyboxRoNet(InitSystemBusyboxBase):
         """
 
     def test_run(self):
-        self.startEmulator("squashfs")
-        self.checkInit()
-        self.checkNetwork("eth0")
+        self.start_emulator("squashfs")
+        self.check_init()
+        self.check_network("eth0")
 
 
 class TestInitSystemBusyboxRwNet(InitSystemBusyboxBase):

@@ -59,6 +59,6 @@ class TestInitSystemBusyboxRwNet(InitSystemBusyboxBase):
         """
 
     def test_run(self):
-        self.startEmulator("ext2")
-        self.checkInit()
-        self.checkNetwork("eth0")
+        self.start_emulator("ext2")
+        self.check_init()
+        self.check_network("eth0")

support/testing/tests/init/test_none.py +2 -2

index 22e4850853..49ea3b276a 100644
@@ -13,7 +13,7 @@ class TestInitSystemNone(InitSystemBase):
         """
 
     def test_run(self):
-        self.startEmulator(fs_type="squashfs", init="/bin/sh")
+        self.start_emulator(fs_type="squashfs", init="/bin/sh")
         index = self.emulator.qemu.expect(["/bin/sh: can't access tty; "
                                            "job control turned off",
                                            pexpect.TIMEOUT], timeout=60)

@@ -32,4 +32,4 @@ class TestInitSystemNone(InitSystemBase):
         _, exit_code = self.emulator.run("mount -t proc none /proc")
         self.assertEqual(exit_code, 0)
 
-        self.checkInit("/bin/sh")
+        self.check_init("/bin/sh")

support/testing/tests/init/test_systemd.py +20 -20

index 77d734895b..48fac1490f 100644
@@ -18,8 +18,8 @@ class InitSystemSystemdBase(InitSystemBase):
         # BR2_TARGET_ROOTFS_TAR is not set
         """
 
-    def checkInit(self):
-        super(InitSystemSystemdBase, self).checkInit("/lib/systemd/systemd")
+    def check_init(self):
+        super(InitSystemSystemdBase, self).check_init("/lib/systemd/systemd")
 
 
 class TestInitSystemSystemdRoNetworkd(InitSystemSystemdBase):

@@ -32,9 +32,9 @@ class TestInitSystemSystemdRoNetworkd(InitSystemSystemdBase):
         """.format(infra.filepath("tests/init/systemd-factory"))
 
     def test_run(self):
-        self.startEmulator("squashfs", "zImage", "vexpress-v2p-ca9")
-        self.checkInit()
-        self.checkNetwork("eth0")
+        self.start_emulator("squashfs", "zImage", "vexpress-v2p-ca9")
+        self.check_init()
+        self.check_network("eth0")
 
         # This one must be executed on the target, to check that
         # the factory feature works as expected

@@ -51,9 +51,9 @@ class TestInitSystemSystemdRwNetworkd(InitSystemSystemdBase):
         """
 
     def test_run(self):
-        self.startEmulator("ext2", "zImage", "vexpress-v2p-ca9")
-        self.checkInit()
-        self.checkNetwork("eth0")
+        self.start_emulator("ext2", "zImage", "vexpress-v2p-ca9")
+        self.check_init()
+        self.check_network("eth0")
 
 
 class TestInitSystemSystemdRoIfupdown(InitSystemSystemdBase):

@@ -66,9 +66,9 @@ class TestInitSystemSystemdRoIfupdown(InitSystemSystemdBase):
         """
 
     def test_run(self):
-        self.startEmulator("squashfs", "zImage", "vexpress-v2p-ca9")
-        self.checkInit()
-        self.checkNetwork("eth0")
+        self.start_emulator("squashfs", "zImage", "vexpress-v2p-ca9")
+        self.check_init()
+        self.check_network("eth0")
 
 
 class TestInitSystemSystemdRwIfupdown(InitSystemSystemdBase):

@@ -81,9 +81,9 @@ class TestInitSystemSystemdRwIfupdown(InitSystemSystemdBase):
         """
 
     def test_run(self):
-        self.startEmulator("ext2", "zImage", "vexpress-v2p-ca9")
-        self.checkInit()
-        self.checkNetwork("eth0")
+        self.start_emulator("ext2", "zImage", "vexpress-v2p-ca9")
+        self.check_init()
+        self.check_network("eth0")
 
 
 class TestInitSystemSystemdRoFull(InitSystemSystemdBase):

@@ -112,9 +112,9 @@ class TestInitSystemSystemdRoFull(InitSystemSystemdBase):
         """
 
     def test_run(self):
-        self.startEmulator("squashfs", "zImage", "vexpress-v2p-ca9")
-        self.checkInit()
-        self.checkNetwork("eth0")
+        self.start_emulator("squashfs", "zImage", "vexpress-v2p-ca9")
+        self.check_init()
+        self.check_network("eth0")
 
 
 class TestInitSystemSystemdRwFull(InitSystemSystemdBase):

@@ -142,6 +142,6 @@ class TestInitSystemSystemdRwFull(InitSystemSystemdBase):
         """
 
     def test_run(self):
-        self.startEmulator("ext2", "zImage", "vexpress-v2p-ca9")
-        self.checkInit()
-        self.checkNetwork("eth0")
+        self.start_emulator("ext2", "zImage", "vexpress-v2p-ca9")
+        self.check_init()
+        self.check_network("eth0")

Signature · 1 line

2.13.0

Change almost all defconfig fragments to take advantage of "cf3cd4388a support/tests: allow properly indented config fragment". Only the defconfig fragment for the ccache test case cannot be changed without additional changes to the infra.

Make each defconfig fragment:
- start after a backslash;
- be declared as a string literal;
- be indented one level more than the variable that contains it.

Signed-off-by Ricardo Martincoski <[email protected]>

---
Tested by hacking support/testing/infra/builder.py
@@ -30,2 +30,3 @@ class Builder(object):
raise SystemError("Cannot olddefconfig")
+ raise SystemError("Stop")
and diffing the resulting *-build.log files against old ones: few empty lines are added/removed. Also tested diffing the resulting .config files against old ones: they are the same.

Warnings from flake8 change from 132 to 100.

 $ git show --stat -w
 support/testing/infra/basetest.py                 | 0
 support/testing/tests/core/test_post_scripts.py   | 0
 support/testing/tests/core/test_rootfs_overlay.py | 5 +++--
 support/testing/tests/core/test_timezone.py       | 6 ++++--
 support/testing/tests/fs/test_ext.py              | 0
 support/testing/tests/fs/test_iso9660.py          | 0
 support/testing/tests/fs/test_jffs2.py            | 0
 support/testing/tests/fs/test_squashfs.py         | 0
 support/testing/tests/fs/test_ubi.py              | 0
 support/testing/tests/fs/test_yaffs2.py           | 4 +++-
 support/testing/tests/package/test_dropbear.py    | 0
 support/testing/tests/package/test_ipython.py     | 0
 support/testing/tests/package/test_python.py      | 0
 support/testing/tests/toolchain/test_external.py  | 0
 14 files changed, 10 insertions(+), 5 deletions(-)
---
 support/testing/infra/basetest.py                 |  38 +++---
 support/testing/tests/core/test_post_scripts.py   |  18 +--
 support/testing/tests/core/test_rootfs_overlay.py |   7 +-
 support/testing/tests/core/test_timezone.py       |  44 +++----
 support/testing/tests/fs/test_ext.py              |  50 ++++----
 support/testing/tests/fs/test_iso9660.py          | 102 ++++++++--------
 support/testing/tests/fs/test_jffs2.py            |  18 +--
 support/testing/tests/fs/test_squashfs.py         |  12 +-
 support/testing/tests/fs/test_ubi.py              |  16 +--
 support/testing/tests/fs/test_yaffs2.py           |   6 +-
 support/testing/tests/package/test_dropbear.py    |  14 +--
 support/testing/tests/package/test_ipython.py     |  16 +--
 support/testing/tests/package/test_python.py      |  20 ++--
 support/testing/tests/toolchain/test_external.py  | 136 +++++++++++-----------
 14 files changed, 251 insertions(+), 246 deletions(-)

support/testing/infra/basetest.py +19 -19

index 493dea5125..d205119b2c 100644
@@ -6,27 +6,27 @@ from infra.builder import Builder
 from infra.emulator import Emulator
 
 BASIC_TOOLCHAIN_CONFIG = \
-"""
-BR2_arm=y
-BR2_TOOLCHAIN_EXTERNAL=y
-BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
-BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
-BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-2015.05-1190-g4a48479.tar.bz2"
-BR2_TOOLCHAIN_EXTERNAL_GCC_4_7=y
-BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_10=y
-BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
-# BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
-BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y
-BR2_TOOLCHAIN_EXTERNAL_CXX=y
-"""
+    """
+    BR2_arm=y
+    BR2_TOOLCHAIN_EXTERNAL=y
+    BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
+    BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
+    BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-2015.05-1190-g4a48479.tar.bz2"
+    BR2_TOOLCHAIN_EXTERNAL_GCC_4_7=y
+    BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_10=y
+    BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
+    # BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
+    BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y
+    BR2_TOOLCHAIN_EXTERNAL_CXX=y
+    """
 
 MINIMAL_CONFIG = \
-"""
-BR2_INIT_NONE=y
-BR2_SYSTEM_BIN_SH_NONE=y
-# BR2_PACKAGE_BUSYBOX is not set
-# BR2_TARGET_ROOTFS_TAR is not set
-"""
+    """
+    BR2_INIT_NONE=y
+    BR2_SYSTEM_BIN_SH_NONE=y
+    # BR2_PACKAGE_BUSYBOX is not set
+    # BR2_TARGET_ROOTFS_TAR is not set
+    """
 
 class BRTest(unittest.TestCase):
     config = None

support/testing/tests/core/test_post_scripts.py +9 -9

index d2a0be15bf..7ca9b9d836 100644
@@ -5,15 +5,15 @@ import infra.basetest
 
 class TestPostScripts(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
-"""
-BR2_INIT_NONE=y
-BR2_SYSTEM_BIN_SH_NONE=y
-# BR2_PACKAGE_BUSYBOX is not set
-BR2_ROOTFS_POST_BUILD_SCRIPT="{}"
-BR2_ROOTFS_POST_IMAGE_SCRIPT="{}"
-BR2_ROOTFS_POST_SCRIPT_ARGS="foobar baz"
-""".format(infra.filepath("tests/core/post-build.sh"),
-           infra.filepath("tests/core/post-image.sh"))
+        """
+        BR2_INIT_NONE=y
+        BR2_SYSTEM_BIN_SH_NONE=y
+        # BR2_PACKAGE_BUSYBOX is not set
+        BR2_ROOTFS_POST_BUILD_SCRIPT="{}"
+        BR2_ROOTFS_POST_IMAGE_SCRIPT="{}"
+        BR2_ROOTFS_POST_SCRIPT_ARGS="foobar baz"
+        """.format(infra.filepath("tests/core/post-build.sh"),
+                   infra.filepath("tests/core/post-image.sh"))
 
     def check_post_log_file(self, path, what):
         lines = {}

support/testing/tests/core/test_rootfs_overlay.py +4 -3

index 42d890ba20..22ef9722d6 100644
@@ -9,11 +9,12 @@ def compare_file(file1, file2):
 class TestRootfsOverlay(infra.basetest.BRTest):
 
     rootfs_overlay_path = infra.filepath("tests/core/rootfs-overlay")
-    rootfs_overlay = "BR2_ROOTFS_OVERLAY=\"{0}1 {0}2\"".format(rootfs_overlay_path)
 
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
-            infra.basetest.MINIMAL_CONFIG + \
-            rootfs_overlay
+        infra.basetest.MINIMAL_CONFIG + \
+        """
+        BR2_ROOTFS_OVERLAY="{0}1 {0}2"
+        """.format(rootfs_overlay_path)
 
     def test_run(self):
         target_file = os.path.join(self.builddir, "target", "test-file1")

support/testing/tests/core/test_timezone.py +23 -21

index 9776b4bcee..f4ba5039ca 100644
@@ -10,11 +10,11 @@ def boot_armv5_cpio(emulator, builddir):
 
 class TestNoTimezone(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
-             """
-# BR2_TARGET_TZ_INFO is not set
-BR2_TARGET_ROOTFS_CPIO=y
-# BR2_TARGET_ROOTFS_TAR is not set
-"""
+        """
+        # BR2_TARGET_TZ_INFO is not set
+        BR2_TARGET_ROOTFS_CPIO=y
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
 
     def test_run(self):
         boot_armv5_cpio(self.emulator, self.builddir)

@@ -24,13 +24,14 @@ BR2_TARGET_ROOTFS_CPIO=y
         self.assertEqual(tz[0].strip(), "UTC")
 
 class TestGlibcAllTimezone(infra.basetest.BRTest):
-    config = """
-BR2_arm=y
-BR2_TOOLCHAIN_EXTERNAL=y
-BR2_TARGET_TZ_INFO=y
-BR2_TARGET_ROOTFS_CPIO=y
-# BR2_TARGET_ROOTFS_TAR is not set
-"""
+    config = \
+        """
+        BR2_arm=y
+        BR2_TOOLCHAIN_EXTERNAL=y
+        BR2_TARGET_TZ_INFO=y
+        BR2_TARGET_ROOTFS_CPIO=y
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
 
     def test_run(self):
         boot_armv5_cpio(self.emulator, self.builddir)

@@ -44,15 +45,16 @@ BR2_TARGET_ROOTFS_CPIO=y
         self.assertEqual(tz[0].strip(), "CET")
 
 class TestGlibcNonDefaultLimitedTimezone(infra.basetest.BRTest):
-    config = """
-BR2_arm=y
-BR2_TOOLCHAIN_EXTERNAL=y
-BR2_TARGET_TZ_INFO=y
-BR2_TARGET_TZ_ZONELIST="northamerica"
-BR2_TARGET_LOCALTIME="America/New_York"
-BR2_TARGET_ROOTFS_CPIO=y
-# BR2_TARGET_ROOTFS_TAR is not set
-"""
+    config = \
+        """
+        BR2_arm=y
+        BR2_TOOLCHAIN_EXTERNAL=y
+        BR2_TARGET_TZ_INFO=y
+        BR2_TARGET_TZ_ZONELIST="northamerica"
+        BR2_TARGET_LOCALTIME="America/New_York"
+        BR2_TARGET_ROOTFS_CPIO=y
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
 
     def test_run(self):
         boot_armv5_cpio(self.emulator, self.builddir)

support/testing/tests/fs/test_ext.py +25 -25

index 0832ba4ce2..49bce45dc8 100644
@@ -39,12 +39,12 @@ def boot_img_and_check_fs_type(emulator, builddir, fs_type):
 
 class TestExt2(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
-"""
-BR2_TARGET_ROOTFS_EXT2=y
-BR2_TARGET_ROOTFS_EXT2_2r0=y
-BR2_TARGET_ROOTFS_EXT2_LABEL="foobaz"
-# BR2_TARGET_ROOTFS_TAR is not set
-"""
+        """
+        BR2_TARGET_ROOTFS_EXT2=y
+        BR2_TARGET_ROOTFS_EXT2_2r0=y
+        BR2_TARGET_ROOTFS_EXT2_LABEL="foobaz"
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
 
     def test_run(self):
         out = dumpe2fs_run(self.builddir, "rootfs.ext2")

@@ -57,12 +57,12 @@ BR2_TARGET_ROOTFS_EXT2_LABEL="foobaz"
 
 class TestExt2r1(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
-"""
-BR2_TARGET_ROOTFS_EXT2=y
-BR2_TARGET_ROOTFS_EXT2_2r1=y
-BR2_TARGET_ROOTFS_EXT2_LABEL="foobar"
-# BR2_TARGET_ROOTFS_TAR is not set
-"""
+        """
+        BR2_TARGET_ROOTFS_EXT2=y
+        BR2_TARGET_ROOTFS_EXT2_2r1=y
+        BR2_TARGET_ROOTFS_EXT2_LABEL="foobar"
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
 
     def test_run(self):
         out = dumpe2fs_run(self.builddir, "rootfs.ext2")

@@ -76,11 +76,11 @@ BR2_TARGET_ROOTFS_EXT2_LABEL="foobar"
 
 class TestExt3(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
-"""
-BR2_TARGET_ROOTFS_EXT2=y
-BR2_TARGET_ROOTFS_EXT2_3=y
-# BR2_TARGET_ROOTFS_TAR is not set
-"""
+        """
+        BR2_TARGET_ROOTFS_EXT2=y
+        BR2_TARGET_ROOTFS_EXT2_3=y
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
 
     def test_run(self):
         out = dumpe2fs_run(self.builddir, "rootfs.ext3")

@@ -94,14 +94,14 @@ BR2_TARGET_ROOTFS_EXT2_3=y
 
 class TestExt4(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
-"""
-BR2_TARGET_ROOTFS_EXT2=y
-BR2_TARGET_ROOTFS_EXT2_4=y
-BR2_TARGET_ROOTFS_EXT2_SIZE="16384"
-BR2_TARGET_ROOTFS_EXT2_INODES=3000
-BR2_TARGET_ROOTFS_EXT2_RESBLKS=10
-# BR2_TARGET_ROOTFS_TAR is not set
-"""
+        """
+        BR2_TARGET_ROOTFS_EXT2=y
+        BR2_TARGET_ROOTFS_EXT2_4=y
+        BR2_TARGET_ROOTFS_EXT2_SIZE="16384"
+        BR2_TARGET_ROOTFS_EXT2_INODES=3000
+        BR2_TARGET_ROOTFS_EXT2_RESBLKS=10
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
 
     def test_run(self):
         out = dumpe2fs_run(self.builddir, "rootfs.ext4")

support/testing/tests/fs/test_iso9660.py +51 -51

index e804808c96..60dcbdc139 100644
@@ -3,27 +3,27 @@ import os
 import infra.basetest
 
 BASIC_CONFIG = \
-"""
-BR2_x86_pentium4=y
-BR2_TOOLCHAIN_EXTERNAL=y
-BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
-BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
-BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-i386-pentium4-full-2015.05-496-g85945aa.tar.bz2"
-BR2_TOOLCHAIN_EXTERNAL_GCC_4_9=y
-BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_2=y
-BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
-# BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
-BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y
-BR2_TOOLCHAIN_EXTERNAL_CXX=y
-BR2_TARGET_GENERIC_GETTY_PORT="ttyS0"
-BR2_TARGET_GENERIC_GETTY_BAUDRATE_115200=y
-BR2_LINUX_KERNEL=y
-BR2_LINUX_KERNEL_CUSTOM_VERSION=y
-BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.0"
-BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y
-BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="{}"
-# BR2_TARGET_ROOTFS_TAR is not set
-""".format(infra.filepath("conf/minimal-x86-qemu-kernel.config"))
+    """
+    BR2_x86_pentium4=y
+    BR2_TOOLCHAIN_EXTERNAL=y
+    BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
+    BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
+    BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-i386-pentium4-full-2015.05-496-g85945aa.tar.bz2"
+    BR2_TOOLCHAIN_EXTERNAL_GCC_4_9=y
+    BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_2=y
+    BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
+    # BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
+    BR2_TOOLCHAIN_EXTERNAL_INET_RPC=y
+    BR2_TOOLCHAIN_EXTERNAL_CXX=y
+    BR2_TARGET_GENERIC_GETTY_PORT="ttyS0"
+    BR2_TARGET_GENERIC_GETTY_BAUDRATE_115200=y
+    BR2_LINUX_KERNEL=y
+    BR2_LINUX_KERNEL_CUSTOM_VERSION=y
+    BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.0"
+    BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y
+    BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="{}"
+    # BR2_TARGET_ROOTFS_TAR is not set
+    """.format(infra.filepath("conf/minimal-x86-qemu-kernel.config"))
 
 def test_mount_internal_external(emulator, builddir, internal=True):
     img = os.path.join(builddir, "images", "rootfs.iso9660")

@@ -48,14 +48,14 @@ def test_touch_file(emulator):
 
 class TestIso9660Grub2External(infra.basetest.BRTest):
     config = BASIC_CONFIG + \
-"""
-BR2_TARGET_ROOTFS_ISO9660=y
-# BR2_TARGET_ROOTFS_ISO9660_INITRD is not set
-BR2_TARGET_GRUB2=y
-BR2_TARGET_GRUB2_BOOT_PARTITION="cd"
-BR2_TARGET_GRUB2_BUILTIN_MODULES="boot linux ext2 fat part_msdos part_gpt normal biosdisk iso9660"
-BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
-""".format(infra.filepath("conf/grub2.cfg"))
+        """
+        BR2_TARGET_ROOTFS_ISO9660=y
+        # BR2_TARGET_ROOTFS_ISO9660_INITRD is not set
+        BR2_TARGET_GRUB2=y
+        BR2_TARGET_GRUB2_BOOT_PARTITION="cd"
+        BR2_TARGET_GRUB2_BUILTIN_MODULES="boot linux ext2 fat part_msdos part_gpt normal biosdisk iso9660"
+        BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
+        """.format(infra.filepath("conf/grub2.cfg"))
 
     def test_run(self):
         exit_code = test_mount_internal_external(self.emulator,

@@ -67,14 +67,14 @@ BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
 
 class TestIso9660Grub2Internal(infra.basetest.BRTest):
     config = BASIC_CONFIG + \
-"""
-BR2_TARGET_ROOTFS_ISO9660=y
-BR2_TARGET_ROOTFS_ISO9660_INITRD=y
-BR2_TARGET_GRUB2=y
-BR2_TARGET_GRUB2_BOOT_PARTITION="cd"
-BR2_TARGET_GRUB2_BUILTIN_MODULES="boot linux ext2 fat part_msdos part_gpt normal biosdisk iso9660"
-BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
-""".format(infra.filepath("conf/grub2.cfg"))
+        """
+        BR2_TARGET_ROOTFS_ISO9660=y
+        BR2_TARGET_ROOTFS_ISO9660_INITRD=y
+        BR2_TARGET_GRUB2=y
+        BR2_TARGET_GRUB2_BOOT_PARTITION="cd"
+        BR2_TARGET_GRUB2_BUILTIN_MODULES="boot linux ext2 fat part_msdos part_gpt normal biosdisk iso9660"
+        BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
+        """.format(infra.filepath("conf/grub2.cfg"))
 
     def test_run(self):
         exit_code = test_mount_internal_external(self.emulator,

@@ -90,13 +90,13 @@ BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
 
 class TestIso9660SyslinuxExternal(infra.basetest.BRTest):
     config = BASIC_CONFIG + \
-"""
-BR2_TARGET_ROOTFS_ISO9660=y
-# BR2_TARGET_ROOTFS_ISO9660_INITRD is not set
-BR2_TARGET_ROOTFS_ISO9660_HYBRID=y
-BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
-BR2_TARGET_SYSLINUX=y
-""".format(infra.filepath("conf/isolinux.cfg"))
+        """
+        BR2_TARGET_ROOTFS_ISO9660=y
+        # BR2_TARGET_ROOTFS_ISO9660_INITRD is not set
+        BR2_TARGET_ROOTFS_ISO9660_HYBRID=y
+        BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
+        BR2_TARGET_SYSLINUX=y
+        """.format(infra.filepath("conf/isolinux.cfg"))
 
     def test_run(self):
         exit_code = test_mount_internal_external(self.emulator,

@@ -108,13 +108,13 @@ BR2_TARGET_SYSLINUX=y
 
 class TestIso9660SyslinuxInternal(infra.basetest.BRTest):
     config = BASIC_CONFIG + \
-"""
-BR2_TARGET_ROOTFS_ISO9660=y
-BR2_TARGET_ROOTFS_ISO9660_INITRD=y
-BR2_TARGET_ROOTFS_ISO9660_HYBRID=y
-BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
-BR2_TARGET_SYSLINUX=y
-""".format(infra.filepath("conf/isolinux.cfg"))
+        """
+        BR2_TARGET_ROOTFS_ISO9660=y
+        BR2_TARGET_ROOTFS_ISO9660_INITRD=y
+        BR2_TARGET_ROOTFS_ISO9660_HYBRID=y
+        BR2_TARGET_ROOTFS_ISO9660_BOOT_MENU="{}"
+        BR2_TARGET_SYSLINUX=y
+        """.format(infra.filepath("conf/isolinux.cfg"))
 
     def test_run(self):
         exit_code = test_mount_internal_external(self.emulator,

support/testing/tests/fs/test_jffs2.py +9 -9

index 7185df7899..a84c858238 100644
@@ -12,15 +12,15 @@ def jffs2dump_find_file(files_list, fname):
 
 class TestJffs2(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
-"""
-BR2_TARGET_ROOTFS_JFFS2=y
-BR2_TARGET_ROOTFS_JFFS2_CUSTOM=y
-BR2_TARGET_ROOTFS_JFFS2_CUSTOM_EBSIZE=0x80000
-BR2_TARGET_ROOTFS_JFFS2_NOCLEANMARKER=y
-BR2_TARGET_ROOTFS_JFFS2_PAD=y
-BR2_TARGET_ROOTFS_JFFS2_PADSIZE=0x4000000
-# BR2_TARGET_ROOTFS_TAR is not set
-"""
+        """
+        BR2_TARGET_ROOTFS_JFFS2=y
+        BR2_TARGET_ROOTFS_JFFS2_CUSTOM=y
+        BR2_TARGET_ROOTFS_JFFS2_CUSTOM_EBSIZE=0x80000
+        BR2_TARGET_ROOTFS_JFFS2_NOCLEANMARKER=y
+        BR2_TARGET_ROOTFS_JFFS2_PAD=y
+        BR2_TARGET_ROOTFS_JFFS2_PADSIZE=0x4000000
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
 
     # TODO: there are some scary JFFS2 messages when one starts to
     # write files in the rootfs: "jffs2: Newly-erased block contained

support/testing/tests/fs/test_squashfs.py +6 -6

index b205b6a55a..9fad28f834 100644
@@ -5,12 +5,12 @@ import infra.basetest
 
 class TestSquashfs(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
-"""
-BR2_TARGET_ROOTFS_SQUASHFS=y
-# BR2_TARGET_ROOTFS_SQUASHFS4_GZIP is not set
-BR2_TARGET_ROOTFS_SQUASHFS4_LZ4=y
-# BR2_TARGET_ROOTFS_TAR is not set
-"""
+        """
+        BR2_TARGET_ROOTFS_SQUASHFS=y
+        # BR2_TARGET_ROOTFS_SQUASHFS4_GZIP is not set
+        BR2_TARGET_ROOTFS_SQUASHFS4_LZ4=y
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
 
     def test_run(self):
         unsquashfs_cmd = ["host/bin/unsquashfs", "-s", "images/rootfs.squashfs"]

support/testing/tests/fs/test_ubi.py +8 -8

index ede4999aa1..360932a0bf 100644
@@ -5,14 +5,14 @@ import infra.basetest
 
 class TestUbi(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
-"""
-BR2_TARGET_ROOTFS_UBIFS=y
-BR2_TARGET_ROOTFS_UBIFS_LEBSIZE=0x7ff80
-BR2_TARGET_ROOTFS_UBIFS_MINIOSIZE=0x1
-BR2_TARGET_ROOTFS_UBI=y
-BR2_TARGET_ROOTFS_UBI_PEBSIZE=0x80000
-BR2_TARGET_ROOTFS_UBI_SUBSIZE=1
-"""
+        """
+        BR2_TARGET_ROOTFS_UBIFS=y
+        BR2_TARGET_ROOTFS_UBIFS_LEBSIZE=0x7ff80
+        BR2_TARGET_ROOTFS_UBIFS_MINIOSIZE=0x1
+        BR2_TARGET_ROOTFS_UBI=y
+        BR2_TARGET_ROOTFS_UBI_PEBSIZE=0x80000
+        BR2_TARGET_ROOTFS_UBI_SUBSIZE=1
+        """
 
     # TODO: if you boot Qemu twice on the same UBI image, it fails to
     # attach the image the second time, with "ubi0 error:

support/testing/tests/fs/test_yaffs2.py +4 -2

index 0ffb758083..a11c1a40a7 100644
@@ -4,8 +4,10 @@ import infra.basetest
 
 class TestYaffs2(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
-            infra.basetest.MINIMAL_CONFIG + \
-            "BR2_TARGET_ROOTFS_YAFFS2=y"
+        infra.basetest.MINIMAL_CONFIG + \
+        """
+        BR2_TARGET_ROOTFS_YAFFS2=y
+        """
 
     def test_run(self):
         img = os.path.join(self.builddir, "images", "rootfs.yaffs2")

support/testing/tests/package/test_dropbear.py +7 -7

index a64b782229..da569d9b1b 100644
@@ -4,13 +4,13 @@ import infra.basetest
 
 class TestDropbear(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
-"""
-BR2_TARGET_GENERIC_ROOT_PASSWD="testpwd"
-BR2_SYSTEM_DHCP="eth0"
-BR2_PACKAGE_DROPBEAR=y
-BR2_TARGET_ROOTFS_CPIO=y
-# BR2_TARGET_ROOTFS_TAR is not set
-"""
+        """
+        BR2_TARGET_GENERIC_ROOT_PASSWD="testpwd"
+        BR2_SYSTEM_DHCP="eth0"
+        BR2_PACKAGE_DROPBEAR=y
+        BR2_TARGET_ROOTFS_CPIO=y
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
 
     def test_run(self):
         img = os.path.join(self.builddir, "images", "rootfs.cpio")

support/testing/tests/package/test_ipython.py +8 -8

index dd8bf500a3..a943101372 100644
@@ -11,10 +11,10 @@ from tests.package.test_python import TestPythonBase
 #
 class TestIPythonPy2(TestPythonBase):
     config = TestPythonBase.config + \
-"""
-BR2_PACKAGE_PYTHON=y
-BR2_PACKAGE_PYTHON_IPYTHON=y
-"""
+        """
+        BR2_PACKAGE_PYTHON=y
+        BR2_PACKAGE_PYTHON_IPYTHON=y
+        """
     interpreter = "ipython"
 
     def test_run(self):

@@ -24,10 +24,10 @@ BR2_PACKAGE_PYTHON_IPYTHON=y
 
 class TestIPythonPy3(TestPythonBase):
     config = TestPythonBase.config + \
-"""
-BR2_PACKAGE_PYTHON3=y
-BR2_PACKAGE_PYTHON_IPYTHON=y
-"""
+        """
+        BR2_PACKAGE_PYTHON3=y
+        BR2_PACKAGE_PYTHON_IPYTHON=y
+        """
     interpreter = "ipython"
 
     def test_run(self):

support/testing/tests/package/test_python.py +10 -10

index bddf09ddbc..79773fff63 100644
@@ -4,10 +4,10 @@ import infra.basetest
 
 class TestPythonBase(infra.basetest.BRTest):
     config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
-"""
-BR2_TARGET_ROOTFS_CPIO=y
-# BR2_TARGET_ROOTFS_TAR is not set
-"""
+        """
+        BR2_TARGET_ROOTFS_CPIO=y
+        # BR2_TARGET_ROOTFS_TAR is not set
+        """
     interpreter = "python"
 
     def login(self):

@@ -42,9 +42,9 @@ BR2_TARGET_ROOTFS_CPIO=y
 
 class TestPython2(TestPythonBase):
     config = TestPythonBase.config + \
-"""
-BR2_PACKAGE_PYTHON=y
-"""
+        """
+        BR2_PACKAGE_PYTHON=y
+        """
     def test_run(self):
         self.login()
         self.version_test("Python 2")

@@ -54,9 +54,9 @@ BR2_PACKAGE_PYTHON=y
 
 class TestPython3(TestPythonBase):
     config = TestPythonBase.config + \
-"""
-BR2_PACKAGE_PYTHON3=y
-"""
+        """
+        BR2_PACKAGE_PYTHON3=y
+        """
     def test_run(self):
         self.login()
         self.version_test("Python 3")

support/testing/tests/toolchain/test_external.py +68 -68

index 394fa17345..1bb5e9497b 100644
@@ -2,10 +2,10 @@ import os
 import infra
 
 BASIC_CONFIG = \
-"""
-BR2_TARGET_ROOTFS_CPIO=y
-# BR2_TARGET_ROOTFS_TAR is not set
-"""
+    """
+    BR2_TARGET_ROOTFS_CPIO=y
+    # BR2_TARGET_ROOTFS_TAR is not set
+    """
 
 def has_broken_links(path):
     for root, dirs, files in os.walk(path):

@@ -32,12 +32,12 @@ class TestExternalToolchain(infra.basetest.BRTest):
 
 class TestExternalToolchainSourceryArmv4(TestExternalToolchain):
     config = BASIC_CONFIG + \
-"""
-BR2_arm=y
-BR2_arm920t=y
-BR2_TOOLCHAIN_EXTERNAL=y
-BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_ARM=y
-"""
+        """
+        BR2_arm=y
+        BR2_arm920t=y
+        BR2_TOOLCHAIN_EXTERNAL=y
+        BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_ARM=y
+        """
     toolchain_prefix = "arm-none-linux-gnueabi"
 
     def test_run(self):

@@ -63,11 +63,11 @@ BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_ARM=y
 
 class TestExternalToolchainSourceryArmv5(TestExternalToolchain):
     config = BASIC_CONFIG + \
-"""
-BR2_arm=y
-BR2_TOOLCHAIN_EXTERNAL=y
-BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_ARM=y
-"""
+        """
+        BR2_arm=y
+        BR2_TOOLCHAIN_EXTERNAL=y
+        BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_ARM=y
+        """
     toolchain_prefix = "arm-none-linux-gnueabi"
 
     def test_run(self):

@@ -88,14 +88,14 @@ BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_ARM=y
 
 class TestExternalToolchainSourceryArmv7(TestExternalToolchain):
     config = BASIC_CONFIG + \
-"""
-BR2_arm=y
-BR2_cortex_a8=y
-BR2_ARM_EABI=y
-BR2_ARM_INSTRUCTIONS_THUMB2=y
-BR2_TOOLCHAIN_EXTERNAL=y
-BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_ARM=y
-"""
+        """
+        BR2_arm=y
+        BR2_cortex_a8=y
+        BR2_ARM_EABI=y
+        BR2_ARM_INSTRUCTIONS_THUMB2=y
+        BR2_TOOLCHAIN_EXTERNAL=y
+        BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_ARM=y
+        """
     toolchain_prefix = "arm-none-linux-gnueabi"
 
     def test_run(self):

@@ -126,12 +126,12 @@ BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_ARM=y
 
 class TestExternalToolchainLinaroArm(TestExternalToolchain):
     config = BASIC_CONFIG + \
-"""
-BR2_arm=y
-BR2_cortex_a8=y
-BR2_TOOLCHAIN_EXTERNAL=y
-BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM=y
-"""
+        """
+        BR2_arm=y
+        BR2_cortex_a8=y
+        BR2_TOOLCHAIN_EXTERNAL=y
+        BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM=y
+        """
     toolchain_prefix = "arm-linux-gnueabihf"
 
     def test_run(self):

@@ -157,19 +157,19 @@ BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM=y
 
 class TestExternalToolchainBuildrootMusl(TestExternalToolchain):
     config = BASIC_CONFIG + \
-"""
-BR2_arm=y
-BR2_cortex_a9=y
-BR2_ARM_ENABLE_VFP=y
-BR2_TOOLCHAIN_EXTERNAL=y
-BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
-BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
-BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-cortex-a9-musl-2017.05-444-g6c704ba.tar.bz2"
-BR2_TOOLCHAIN_EXTERNAL_GCC_6=y
-BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_11=y
-BR2_TOOLCHAIN_EXTERNAL_CUSTOM_MUSL=y
-BR2_TOOLCHAIN_EXTERNAL_CXX=y
-"""
+        """
+        BR2_arm=y
+        BR2_cortex_a9=y
+        BR2_ARM_ENABLE_VFP=y
+        BR2_TOOLCHAIN_EXTERNAL=y
+        BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
+        BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
+        BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-cortex-a9-musl-2017.05-444-g6c704ba.tar.bz2"
+        BR2_TOOLCHAIN_EXTERNAL_GCC_6=y
+        BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_11=y
+        BR2_TOOLCHAIN_EXTERNAL_CUSTOM_MUSL=y
+        BR2_TOOLCHAIN_EXTERNAL_CXX=y
+        """
     toolchain_prefix = "arm-linux"
 
     def test_run(self):

@@ -182,20 +182,20 @@ BR2_TOOLCHAIN_EXTERNAL_CXX=y
 
 class TestExternalToolchainCtngMusl(TestExternalToolchain):
     config = BASIC_CONFIG + \
-"""
-BR2_arm=y
-BR2_cortex_a9=y
-BR2_ARM_ENABLE_VFP=y
-BR2_TOOLCHAIN_EXTERNAL=y
-BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
-BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
-BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.net/toolchains/tarballs/arm-ctng-linux-musleabihf.tar.xz"
-BR2_TOOLCHAIN_EXTERNAL_CUSTOM_PREFIX="arm-ctng-linux-musleabihf"
-BR2_TOOLCHAIN_EXTERNAL_GCC_7=y
-BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_10=y
-BR2_TOOLCHAIN_EXTERNAL_CUSTOM_MUSL=y
-BR2_TOOLCHAIN_EXTERNAL_CXX=y
-"""
+        """
+        BR2_arm=y
+        BR2_cortex_a9=y
+        BR2_ARM_ENABLE_VFP=y
+        BR2_TOOLCHAIN_EXTERNAL=y
+        BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
+        BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
+        BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.net/toolchains/tarballs/arm-ctng-linux-musleabihf.tar.xz"
+        BR2_TOOLCHAIN_EXTERNAL_CUSTOM_PREFIX="arm-ctng-linux-musleabihf"
+        BR2_TOOLCHAIN_EXTERNAL_GCC_7=y
+        BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_10=y
+        BR2_TOOLCHAIN_EXTERNAL_CUSTOM_MUSL=y
+        BR2_TOOLCHAIN_EXTERNAL_CXX=y
+        """
     toolchain_prefix = "arm-ctng-linux-musleabihf"
 
     def test_run(self):

@@ -208,18 +208,18 @@ BR2_TOOLCHAIN_EXTERNAL_CXX=y
 
 class TestExternalToolchainBuildrootuClibc(TestExternalToolchain):
     config = BASIC_CONFIG + \
-"""
-BR2_arm=y
-BR2_TOOLCHAIN_EXTERNAL=y
-BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
-BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
-BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-2017.05-444-g6c704ba.tar.bz2"
-BR2_TOOLCHAIN_EXTERNAL_GCC_4_9=y
-BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_10=y
-BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
-# BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
-BR2_TOOLCHAIN_EXTERNAL_CXX=y
-"""
+        """
+        BR2_arm=y
+        BR2_TOOLCHAIN_EXTERNAL=y
+        BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
+        BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
+        BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-arm-full-2017.05-444-g6c704ba.tar.bz2"
+        BR2_TOOLCHAIN_EXTERNAL_GCC_4_9=y
+        BR2_TOOLCHAIN_EXTERNAL_HEADERS_3_10=y
+        BR2_TOOLCHAIN_EXTERNAL_LOCALE=y
+        # BR2_TOOLCHAIN_EXTERNAL_HAS_THREADS_DEBUG is not set
+        BR2_TOOLCHAIN_EXTERNAL_CXX=y
+        """
     toolchain_prefix = "arm-linux"
 
     def test_run(self):

Signature · 1 line

2.13.0

On 2017-09-28 23:27 -0300, Ricardo Martincoski spake thusly:

Quoted 1 line

Warnings from flake8 change from 132 to 100.

What does this mean?

Use method naming convention from PEP8 as other test cases already do.

sed \
-e 's,startEmulator,start_emulator,g' \
-e 's,checkInit,check_init,g' \
-e 's,checkNetwork,check_network,g' \
-i support/testing/tests/init/*.py

Signed-off-by Ricardo Martincoski <[email protected]>

Acked-by "Yann E. MORIN" <[email protected]>

We recommend wrapping at 80 columns but we accept 132 columns when it makes more readable.

Follow up of discussion at this thread:
http://lists.busybox.net/pipermail/buildroot/2017-October/203624.html

Cc Arnout Vandecappelle <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Yann E. MORIN <[email protected]>

Signed-off-by Ricardo Martincoski <[email protected]>

---
Changes v1 -> v2:
- new patch

Following patch is replaced by this one:
http://patchwork.ozlabs.org/patch/820324/

Number of warnings reported by flake8 for the test infra:

before:  17
after:    2
---
 .flake8 | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 .flake8

.flake8 new file +2 -0

new file mode 100644
index 0000000000..afdb967930
@@ -0,0 +1,2 @@
+[flake8]
+max-line-length=132

Fix the remaining code style warnings from flake8:
- properly indent continuation lines;
- use proper code to test a parameter is not None.

Cc Yann E. MORIN <[email protected]>

Signed-off-by Ricardo Martincoski <[email protected]>

---
Changes v1 -> v2:
- renumbered 4 -> 6;
- keep only a small part of the old patch;
- use 'if x is not None' instead of 'if x:' (Yann E. MORIN);
- update commit log.

Number of warnings reported by flake8 for the test infra:

before:   2
after:    0

On 05-10-17 23:42, Ricardo Martincoski wrote:

Quoted 3 lines

Number of warnings reported by flake8 for the test infra:

before:   2
after:    0

I still have a warning:

$ python -m flake8 support/testing/
support/testing/tests/init/base.py:45:9: N803 argument name should be lowercase

Any idea why you didn't catch this?

Also, there are still warnings in support/scripts and utils, care to take a
look at those?

Also, you used to have a patch that ran flake8 from .gitlab-ci.yml, but it
seems to be gone from patchwork.

And finally, if you have nothing else to do :-), maybe you could make the tests
python3 compatible. For me, it'd be fine to even require python3 to be able to run the tests, if that simplifies things.

Hello,

Quoted 1 line · Arnout Vandecappelle

On 05-10-17 23:42, Ricardo Martincoski wrote:

[snip]

Quoted 9 lines

Quoted 3 lines

Number of warnings reported by flake8 for the test infra:

before:   2
after:    0

I still have a warning:

$ python -m flake8 support/testing/
support/testing/tests/init/base.py:45:9: N803 argument name should be lowercase

Quoted 1 line

Any idea why you didn't catch this?

That's because you have flake8 plugin pep8-naming installed, which default to on.
To always disable it (to ignore such warnings and get the same result with or without it) this line could be added to .flake8:
ignore=N

I don't know a way to enforce it is always installed (to always generate such warnings and get the same result).

To also ignore warnings generated when the plugin flake8-docstrings is installed, this line could be used instead:
ignore=N,D

Quoted 2 lines

Also, there are still warnings in support/scripts and utils, care to take a
look at those?

I will.

Quoted 3 lines

Also, you used to have a patch that ran flake8 from .gitlab-ci.yml, but it
seems to be gone from patchwork.

Yes, that was
http://patchwork.ozlabs.org/patch/820324/

The main purpose (to check the test infra even if/when the tests become placed in each package directory) was replaced by the .flake8 file in the main directory.
http://patchwork.ozlabs.org/patch/822105/

I should have replied to the first one explaining I marked it as Superseded by the second one.

If you think the first patch is useful feel free to review it so I can respin, or even take over if you wish.

Quoted 3 lines

And finally, if you have nothing else to do :-), maybe you could make the tests
python3 compatible. For me, it'd be fine to even require python3 to be able to run the tests, if that simplifies things.

I will do later (I guess in next December/January).

Regards,
Ricardo

Quoted 14 lines · Ricardo Martincoski

Quoted 1 line · Arnout Vandecappelle

On 05-10-17 23:42, Ricardo Martincoski wrote:

[snip]

Quoted 10 lines

Quoted 3 lines

Number of warnings reported by flake8 for the test infra:

before:   2
after:    0

I still have a warning:

$ python -m flake8 support/testing/
support/testing/tests/init/base.py:45:9: N803 argument name should be lowercase

Any idea why you didn't catch this?

Quoted 5 lines

That's because you have flake8 plugin pep8-naming installed, which default to on.
To always disable it (to ignore such warnings and get the same result with or without it) this line could be added to .flake8:
ignore=N

I don't think we want that.

Quoted 3 lines

I don't know a way to enforce it is always installed (to always generate such warnings and get the same result).

We don't need to enforce it. What we need is to make sure that GitLab-CI does
have this, so it *will* check the naming (when we have a test for flake8, obviously). You can do that by updating support/docker/Dockerfile (and I'll rebuild the docker image when I apply the patch).

Quoted 3 lines

To also ignore warnings generated when the plugin flake8-docstrings is installed, this line could be used instead:
ignore=N,D

Good idea, let's check the docstrings as well!

Quoted 4 lines

Quoted 2 lines

Also, there are still warnings in support/scripts and utils, care to take a
look at those?

I will.

It's not urgent. Well, that is, until we start failing gitlab-CI because of it :-)

Quoted 10 lines

Quoted 2 lines

Also, you used to have a patch that ran flake8 from .gitlab-ci.yml, but it
seems to be gone from patchwork.

Yes, that was
http://patchwork.ozlabs.org/patch/820324/

The main purpose (to check the test infra even if/when the tests become placed in each package directory) was replaced by the .flake8 file in the main directory.
http://patchwork.ozlabs.org/patch/822105/

I don't really see any relation between the two... Patch 820324 creates a
flake8 test, patch 822105 creates a flake8 config.

OK, now I look at patch 820324, I think it's not the right approach. There is
no point making a unit test for that. All we want is an update to .gitlab-ci.yml.in that adds something like this:

check-flake8:
before_script:
- apt-get update -qq
- apt-get install -y -qq --no-install-recommends python3-flake8 python3-pep8-naming python3-flake8-docstrings
script:
- flake8

There is no real need to add flake8 to the Docker image since it's only used for this one test. And we can simply run flake8 at the top level; it's exit code will indicate failure.

Oh, now I notice that this won't work because our test infra is not Python3
compliant. That would be nice to fix as well :-) Apparently docstrings only exists for python3 (at least on my Debian, don't know about Ubuntu).

Quoted 5 lines

Quoted 3 lines

And finally, if you have nothing else to do :-), maybe you could make the tests
python3 compatible. For me, it'd be fine to even require python3 to be able to run the tests, if that simplifies things.

I will do later (I guess in next December/January).

Hello,

Quoted 1 line · Arnout Vandecappelle

On 23-10-17 04:30, Ricardo Martincoski wrote:

Quoted 7 lines

Quoted 2 lines

I don't know a way to enforce it is always installed (to always generate such warnings and get the same result).

We don't need to enforce it. What we need is to make sure that GitLab-CI does
have this, so it *will* check the naming (when we have a test for flake8, obviously). You can do that by updating support/docker/Dockerfile (and I'll rebuild the docker image when I apply the patch).

Good to know how to add things to the image..
I won't do that because I understand from you (below) that it's better to add apt install python-flake8
to .gitlab-ci.yml.in in this case.

Quoted 6 lines

Quoted 4 lines

To also ignore warnings generated when the plugin flake8-docstrings is installed, this line could be used instead:
ignore=N,D

Good idea, let's check the docstrings as well!

Eventually...

By default the plugin would generate 200+ warnings because it expects every single package, module, function, method to be documented.
I don't think it is useful to add a lot of obvious documentation, the good thing IMO is to document the tricky ones.

So I suggest to use 'flake8 --ignore=D1' that will check the style of the docstrings but would not generate a lot of 'Missing docstring in'. And during the review ask for documentation on tricky parts of the code.

Of course it's not good to let a test failing in the gitlab-ci for a long time, so let's fix style of the docstring already in the code before adding this. I will eventually do that.
But first I will fix warnings in support/scripts and utils.

Quoted 3 lines

Oh, now I notice that this won't work because our test infra is not Python3
compliant. That would be nice to fix as well :-) Apparently docstrings only exists for python3 (at least on my Debian, don't know about Ubuntu).

I started the image buildroot/base locally and using pip it succeeds for python 2:
apt update
apt install python-pip
pip install flake8-docstrings

Move the strip out of leading spaces in defconfig fragments from the BRTest class to the Builder class. It actually postpones the strip out, consequentially allowing test cases to post-process the defconfig in their own __init__ before calling the __init__ method from BRTest.

Moving this code to the Builder class also allows any new test class that inherits from BRTest to reuse the same code even if the new class overrides the setUp method.

At same time, prepend a newline to the jlevel handling otherwise it would stop working for defconfig fragments that don't end in a newline.

Signed-off-by Ricardo Martincoski <[email protected]>

Cc Yann E. MORIN <[email protected]>