[U-Boot] [PATCH 1/3] test/py: expose config and log as session scoped fixture

[U-Boot] [PATCH 1/3] test/py: expose config and log as session scoped fixture

[Stefan Brüns]

If a test uses a fixture which is expensive to setup, the fixture can
possibly created with session or module scope. As u_boot_console has
function scope, it can not be used in this case.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 test/py/conftest.py | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/test/py/conftest.py b/test/py/conftest.py
index 1f15e3e..65e1d75 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -298,6 +298,32 @@ def pytest_generate_tests(metafunc):
             continue
         generate_config(metafunc, fn)
 
+ at pytest.fixture(scope='session')
+def u_boot_log(request):
+     """Generate the value of a test's log fixture.
+
+     Args:
+         request: The pytest request.
+
+     Returns:
+         The fixture value.
+     """
+
+     return console.log
+
+ at pytest.fixture(scope='session')
+def u_boot_config(request):
+     """Generate the value of a test's u_boot_config fixture.
+
+     Args:
+         request: The pytest request.
+
+     Returns:
+         The fixture value.
+     """
+
+     return console.config
+
 @pytest.fixture(scope='function')
 def u_boot_console(request):
     """Generate the value of a test's u_boot_console fixture.
-- 
2.10.1

[U-Boot] [PATCH 1/3] test/py: expose config and log as session scoped fixture

[Stephen Warren]

On 11/05/2016 10:45 AM, Stefan Br?ns wrote:
> If a test uses a fixture which is expensive to setup, the fixture can
> possibly created with session or module scope. As u_boot_console has
> function scope, it can not be used in this case.

Acked-by: Stephen Warren <swarren@wwwdotorg.org>

[U-Boot] [U-Boot, 1/3] test/py: expose config and log as session scoped fixture

[Tom Rini]

On Sat, Nov 05, 2016 at 05:45:32PM +0100, Stefan Br?ns wrote:

> If a test uses a fixture which is expensive to setup, the fixture can
> possibly created with session or module scope. As u_boot_console has
> function scope, it can not be used in this case.
> 
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> Acked-by: Stephen Warren <swarren@wwwdotorg.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161121/ec216d89/attachment.sig>

[U-Boot] [PATCH 2/3] test/py: Allow to pass u_boot_log instead of console for run_and_log

[Stefan Brüns]

The runner actually has no console dependency, only on the log provided
by the console. Accept both u_boot_console or a multiplexed_log.

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 test/py/u_boot_utils.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/test/py/u_boot_utils.py b/test/py/u_boot_utils.py
index 2ba4bae..c80cf07 100644
--- a/test/py/u_boot_utils.py
+++ b/test/py/u_boot_utils.py
@@ -153,7 +153,7 @@ def wait_until_file_open_fails(fn, ignore_errors):
         return
     raise Exception('File can still be opened')
 
-def run_and_log(u_boot_console, cmd, ignore_errors=False):
+def run_and_log(u_boot_console_or_log, cmd, ignore_errors=False):
     """Run a command and log its output.
 
     Args:
@@ -171,7 +171,10 @@ def run_and_log(u_boot_console, cmd, ignore_errors=False):
     """
     if isinstance(cmd, str):
         cmd = cmd.split()
-    runner = u_boot_console.log.get_runner(cmd[0], sys.stdout)
+    try:
+        runner = u_boot_console_or_log.get_runner(cmd[0], sys.stdout)
+    except:
+        runner = u_boot_console_or_log.log.get_runner(cmd[0], sys.stdout)
     output = runner.run(cmd, ignore_errors=ignore_errors)
     runner.close()
     return output
@@ -189,7 +192,10 @@ def run_and_log_expect_exception(u_boot_console, cmd, retcode, msg):
         msg: String that should be contained within the command's output.
     """
     try:
+        runner = u_boot_console.get_runner(cmd[0], sys.stdout)
+    except:
         runner = u_boot_console.log.get_runner(cmd[0], sys.stdout)
+    try:
         runner.run(cmd)
     except Exception as e:
         assert(retcode == runner.exit_status)
-- 
2.10.1

[U-Boot] [PATCH 2/3] test/py: Allow to pass u_boot_log instead of console for run_and_log

[Stephen Warren]

On 11/05/2016 10:45 AM, Stefan Br?ns wrote:
> The runner actually has no console dependency, only on the log provided
> by the console. Accept both u_boot_console or a multiplexed_log.

> diff --git a/test/py/u_boot_utils.py b/test/py/u_boot_utils.py

> -def run_and_log(u_boot_console, cmd, ignore_errors=False):
> +def run_and_log(u_boot_console_or_log, cmd, ignore_errors=False):
>      """Run a command and log its output.
>
>      Args:

I expect you also need to update the documentation for the function 
parameter in the "Args" section of the docs too.

> @@ -171,7 +171,10 @@ def run_and_log(u_boot_console, cmd, ignore_errors=False):
>      """
>      if isinstance(cmd, str):
>          cmd = cmd.split()
> -    runner = u_boot_console.log.get_runner(cmd[0], sys.stdout)
> +    try:
> +        runner = u_boot_console_or_log.get_runner(cmd[0], sys.stdout)
> +    except:
> +        runner = u_boot_console_or_log.log.get_runner(cmd[0], sys.stdout)

I don't like this because:

a) It duplicates the call to get_runner(), even though both calls are 
logically the same thing, just with different parameter values.

b) It can catch exceptions that occur inside get_runner(), and then 
potentially repeat that call.

Better would be:

if hasattr(u_boot_console_or_log, 'get_runner'):
     get_runner = u_boot_console_or_log.get_runner
else:
     get_runner = u_boot_console_or_log.log.get_runner
runner = get_runner(cmd[0], sys.stdout)

Same comment for the similar change in run_and_log_expect_exception(). 
You could perhaps even create a helper function 
get_get_runner(u_boot_console_or_log) to share the code.

[U-Boot] [PATCH 3/3] test/py: Create tests for ext4 and fat testing on sandbox

[Stefan Brüns]

The following checks are currently implemented:
1. listing a directory
2. verifying size of a file
3. veryfying md5sum for a file region
4. reading the beginning of a file

Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
---
 test/py/tests/test_fs.py | 298 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 298 insertions(+)
 create mode 100644 test/py/tests/test_fs.py

diff --git a/test/py/tests/test_fs.py b/test/py/tests/test_fs.py
new file mode 100644
index 0000000..5ac91e4
--- /dev/null
+++ b/test/py/tests/test_fs.py
@@ -0,0 +1,298 @@
+# Copyright (c) 2016, Stefan Bruens <stefan.bruens@rwth-aachen.de>
+#
+# SPDX-License-Identifier: GPL-2.0
+
+# Test U-Boot's filesystem implementations
+
+import hashlib
+import pytest
+import os
+import random
+import re
+import u_boot_utils as util
+
+
+mkfs_opts = {
+	"fat" :'-t vfat',
+	"ext4" : '-t ext4 -F',
+}
+
+fs_commands = {
+	"fat" : {
+		'listcmd'   : 'ls',
+		'readcmd'   : 'load',
+		'sizecmd'   : 'size',
+		'writecmd'  : 'size',
+	},
+	"ext4" : {
+		'listcmd'   : 'ls',
+		'readcmd'   : 'load',
+		'sizecmd'   : 'size',
+		'writecmd'  : 'size',
+	},
+}
+
+cmd_parameters = {
+	"hostfs" : {
+		'prefix'    : 'host ',
+		'interface' : 'hostfs -',
+	},
+	"generic" : {
+		'prefix'    : '',
+		'interface' : 'host 0:0',
+	},
+}
+
+files = {
+    "empty.file" : [(0, 0)],
+    "1MB.file"   : [(0, 1e6)],
+    "1MB.sparse.file"   : [(1e6-1, 1e6)],
+}
+    # "2_5GB.sparse.file"   : [(0, 1e6), (1e9, 1e9+1e6), (2.5e9-1e6, 2.5e9)],
+
+ at pytest.fixture(scope="session")
+def prereq_commands():
+    from distutils.spawn import find_executable
+    for command in ["mkfs", "mount", "umount"]:
+        if find_executable(command) is None:
+            pytest.skip('Filesystem tests, "{0}" not in PATH'.format(command))
+
+class FsImage:
+    def __init__(self, fstype, imagename, mountpath):
+        self.fstype = fstype
+        self.imagename = imagename
+        self.mountpath = mountpath
+        self.md5s = {}
+        with open(self.imagename, 'w') as fd:
+            fd.truncate(0)
+            fd.seek(3e9)
+            fd.write(bytes([0]))
+
+    def mkfs(self, log):
+        mkfsopts = mkfs_opts.get(self.fstype)
+        util.run_and_log(log,
+            'mkfs {0} {1}'.format(mkfsopts, self.imagename))
+
+    def create_file(self, log, filename):
+        md5sums = []
+        with open(self.mountpath + "/" + filename, 'w') as fd:
+            for stride in files[filename]:
+                length = int(stride[1] - stride[0])
+                data = bytearray(random.getrandbits(8) for _ in xrange(length))
+                md5 = hashlib.md5(data).hexdigest()
+                md5sums.append(md5)
+                log.info("{0}: write {1} bytes @ {2} : {3}".format(
+                    filename, int(stride[1] - stride[0]),
+                    int(stride[0]), md5))
+                fd.seek(stride[0])
+                fd.write(data);
+        self.md5s[filename] = md5sums
+
+    def create_files(self, log):
+        with log.section("Create initial files"):
+            for filename in files:
+                self.create_file(log, filename)
+            log.info("Created test files in {0}".format(self.mountpath))
+            util.run_and_log(log, 'ls -la {0}'.format(self.mountpath))
+            util.run_and_log(log, 'sync {0}'.format(self.mountpath))
+
+    def mount(self, log):
+        if not os.path.exists(self.mountpath):
+            os.mkdir(self.mountpath)
+        log.info("Mounting {0} at {1}".format(self.imagename, self.mountpath))
+        if self.fstype == "ext4":
+            cmd = 'sudo -n mount -o loop,rw {0} {1}'.format(self.imagename, self.mountpath)
+        else:
+            cmd = 'sudo -n mount -o loop,rw,umask=000 {0} {1}'.format(self.imagename, self.mountpath)
+        util.run_and_log(log, cmd)
+        if self.fstype == "ext4":
+            cmd = 'sudo -n chmod og+rw {0}'.format(self.mountpath)
+            return util.run_and_log(log, cmd)
+
+    def unmount(self, log):
+        log.info("Unmounting {0}".format(self.imagename))
+        cmd = 'sudo -n umount -l {0}'.format(self.mountpath)
+        util.run_and_log(log, cmd, ignore_errors=True)
+
+
+ at pytest.fixture(scope="module", params=["fat", "ext4"])
+def fsimage(prereq_commands, u_boot_config, u_boot_log, request):
+    datadir = u_boot_config.result_dir + '/'
+    fstype = request.param
+    imagename = datadir + "3GB." + fstype + ".img"
+    mountpath = datadir + "mnt_" + fstype
+
+    with u_boot_log.section('Create image {0}'.format(imagename)):
+        fsimage = FsImage(fstype, imagename, mountpath)
+        fsimage.mkfs(u_boot_log)
+
+    yield fsimage
+    fsimage.unmount(u_boot_log)
+
+ at pytest.fixture(scope="module")
+def populated_image(fsimage, u_boot_log):
+    try:
+        fsimage.mount(u_boot_log)
+    except Exception as e:
+        pytest.skip('{0}: could not mount {1}'.format(
+            fsimage.fstype, fsimage.imagename))
+        yield None
+
+    fsimage.create_files(u_boot_log)
+    fsimage.unmount(u_boot_log)
+    yield fsimage
+
+# 'Transfer' the image to the client and make it accessible
+ at pytest.fixture(scope="function")
+def boundimage(populated_image, u_boot_console, request):
+    image = populated_image
+    if request.cls.scenario == "hostfs":
+        image.mount(u_boot_console.log)
+        image.rootpath = image.mountpath + "/"
+        yield image
+        image.unmount(u_boot_console.log)
+    else:
+        output = u_boot_console.run_command_list(
+            ["host bind 0 {0}".format(image.imagename)])
+        image.rootpath = "/"
+        yield image
+        output = u_boot_console.run_command_list(["host bind 0 "])
+
+
+# Dummy test to create an image file with filesystem
+# Useful to isolate fixture setup from actual tests
+def test_fs_prepare_image(u_boot_config, fsimage, request):
+    if not fsimage:
+        pytest.fail("Failed to create image")
+
+# Dummy test to create initial filesystem contents
+def test_fs_populate_image(populated_image, request):
+    if not populated_image:
+        pytest.fail("Failed create initial image content")
+
+# Scenarios:
+#   hostfs: access image contents through the sandbox hostfs
+#       facility, using the filesytem implementation of
+#       the sandbox host, e.g. Linux kernel
+#   generic: test u-boots native filesystem implementations,
+#       using the 'generic' command names, e.g. 'load'
+#   TODO -
+#   fscommands: test u-boots native filesystem implementations,
+#       using the fs specific commands, e.g. 'ext4load'
+ at pytest.fixture(scope="class", params=["generic", "hostfs"])
+def scenario(request):
+    request.cls.scenario = request.param
+    return request.param
+
+ at pytest.mark.usefixtures("u_boot_console", "scenario")
+class TestFilesystems:
+    ignore_cleanup_errors = True
+    filesize_regex = re.compile("^filesize=([A-Fa-f0-9]+)")
+    md5sum_regex = re.compile("^md5 for .* ==> ([A-Fa-f0-9]{32})")
+    dirlist_regex = re.compile("\s+(\d+)\s+(\S+.file\S*)")
+
+    def get_filesize(self, filename):
+        strides = files[filename]
+        return int(strides[-1][1])
+
+    def check_dirlist(self, string, filenames):
+        m = self.dirlist_regex.findall(string)
+        assert(m)
+        for i, e in enumerate(m):
+            m[i] = (int(e[0]), e[1].lower())
+        for f in filenames:
+            e = (self.get_filesize(f), f.lower())
+            assert(e in m)
+
+    def check_filesize(self, string, size):
+        m = self.filesize_regex.match(string)
+        assert(m)
+        assert(int(m.group(1), 16) == size)
+
+    def check_md5sum(self, string, md5):
+        m = self.md5sum_regex.match(string)
+        assert(m)
+        assert(len(m.group(1)) == 32)
+        assert(m.group(1) == md5)
+
+    def run_listcmd(self, rootpath, dirname):
+        cmd = "{0}{1} {2} {3}".format(
+            self.fs_params.get('prefix'),
+            self.fs_commands.get('listcmd'),
+            self.fs_params.get('interface'),
+            rootpath + dirname)
+        with self.console.log.section('List "{0}"'.format(dirname)):
+            output = self.console.run_command_list([cmd])
+            return output[0]
+
+    def run_readcmd(self, rootpath, filename, offset, length):
+        cmd = "{0}{1} {2} {3} {4} 0x{5:x} 0x{6:x}".format(
+            self.fs_params.get('prefix'),
+            self.fs_commands.get('readcmd'),
+            self.fs_params.get('interface'),
+            "0", # address
+            rootpath + filename,
+            length, offset)
+        with self.console.log.section('Read file "{0}"'.format(filename)):
+            output = self.console.run_command_list(
+                [cmd, "env print filesize",
+                 "md5sum 0 $filesize", "env set filesize"])
+            return output[1:3]
+
+    def run_sizecmd(self, rootpath, filename):
+        cmd = "{0}{1} {2} {3}".format(
+            self.fs_params.get('prefix'),
+            self.fs_commands.get('sizecmd'),
+            self.fs_params.get('interface'),
+            rootpath + filename)
+        with self.console.log.section('Get size of "{0}"'.format(filename)):
+            output = self.console.run_command_list(
+                [cmd, "env print filesize", "env set filesize"])
+            return output[1]
+
+    @pytest.mark.parametrize("dirname", ["", "./"])
+    def test_fs_ls(self, boundimage, u_boot_console, dirname):
+        self.console = u_boot_console
+        self.fs_params = cmd_parameters.get(self.scenario)
+        self.fs_commands = fs_commands.get(boundimage.fstype)
+
+        output = self.run_listcmd(boundimage.rootpath, dirname)
+        self.check_dirlist(output, files.keys())
+
+    @pytest.mark.parametrize("filename", files.keys())
+    def test_fs_filesize(self, boundimage, u_boot_console, filename):
+        self.console = u_boot_console
+        self.fs_params = cmd_parameters.get(self.scenario)
+        self.fs_commands = fs_commands.get(boundimage.fstype)
+        filesize = self.get_filesize(filename)
+
+        output = self.run_sizecmd(boundimage.rootpath, filename)
+        self.check_filesize(output, filesize)
+
+    @pytest.mark.parametrize("filename", files.keys())
+    def test_fs_read(self, boundimage, u_boot_console, filename):
+        self.console = u_boot_console
+        self.fs_params = cmd_parameters.get(self.scenario)
+        self.fs_commands = fs_commands.get(boundimage.fstype)
+        md5s = boundimage.md5s[filename]
+
+        for i, stride in enumerate(files[filename]):
+            length = int(stride[1]) - int(stride[0])
+            output = self.run_readcmd(
+                boundimage.rootpath, filename, int(stride[0]), length)
+            self.check_filesize(output[0], length)
+            self.console.log.info("md5: {0}".format(md5s[i]))
+            self.check_md5sum(output[1], md5s[i])
+
+    @pytest.mark.parametrize("filename", files.keys())
+    def test_fs_read_head(self, boundimage, u_boot_console, filename):
+        self.console = u_boot_console
+        self.fs_params = cmd_parameters.get(self.scenario)
+        self.fs_commands = fs_commands.get(boundimage.fstype)
+
+        filesize = self.get_filesize(filename)
+        filesize = min(filesize, 4e6)
+
+        output = self.run_readcmd(
+            boundimage.rootpath, filename, 0, filesize)
+        self.check_filesize(output[0], filesize)
-- 
2.10.1

[U-Boot] [PATCH 3/3] test/py: Create tests for ext4 and fat testing on sandbox

[Simon Glass]

Hi,

On 5 November 2016 at 10:45, Stefan Br?ns <stefan.bruens@rwth-aachen.de> wrote:
>
> The following checks are currently implemented:
> 1. listing a directory
> 2. verifying size of a file
> 3. veryfying md5sum for a file region
> 4. reading the beginning of a file
>
> Signed-off-by: Stefan Br?ns <stefan.bruens@rwth-aachen.de>
> ---
>  test/py/tests/test_fs.py | 298 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 298 insertions(+)
>  create mode 100644 test/py/tests/test_fs.py
>
> diff --git a/test/py/tests/test_fs.py b/test/py/tests/test_fs.py
> new file mode 100644
> index 0000000..5ac91e4
> --- /dev/null
> +++ b/test/py/tests/test_fs.py
> @@ -0,0 +1,298 @@
> +# Copyright (c) 2016, Stefan Bruens <stefan.bruens@rwth-aachen.de>
> +#
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# Test U-Boot's filesystem implementations

Can you add a few details here about what this tests?

> +
> +import hashlib
> +import pytest
> +import os
> +import random
> +import re
> +import u_boot_utils as util
> +
> +
> +mkfs_opts = {
> +       "fat" :'-t vfat',
> +       "ext4" : '-t ext4 -F',

Can you please use a single quote unless you can't? That is the style
used in U-Boot.

> +}
> +
> +fs_commands = {
> +       "fat" : {
> +               'listcmd'   : 'ls',
> +               'readcmd'   : 'load',
> +               'sizecmd'   : 'size',
> +               'writecmd'  : 'size',
> +       },
> +       "ext4" : {
> +               'listcmd'   : 'ls',
> +               'readcmd'   : 'load',
> +               'sizecmd'   : 'size',
> +               'writecmd'  : 'size',
> +       },
> +}
> +
> +cmd_parameters = {
> +       "hostfs" : {
> +               'prefix'    : 'host ',
> +               'interface' : 'hostfs -',
> +       },
> +       "generic" : {
> +               'prefix'    : '',
> +               'interface' : 'host 0:0',
> +       },
> +}
> +
> +files = {
> +    "empty.file" : [(0, 0)],
> +    "1MB.file"   : [(0, 1e6)],
> +    "1MB.sparse.file"   : [(1e6-1, 1e6)],
> +}
> +    # "2_5GB.sparse.file"   : [(0, 1e6), (1e9, 1e9+1e6), (2.5e9-1e6, 2.5e9)],

What is that line for?

> +
> + at pytest.fixture(scope="session")
> +def prereq_commands():
> +    from distutils.spawn import find_executable

Why not import this at the top of the file?

> +    for command in ["mkfs", "mount", "umount"]:
> +        if find_executable(command) is None:
> +            pytest.skip('Filesystem tests, "{0}" not in PATH'.format(command))
> +
> +class FsImage:
> +    def __init__(self, fstype, imagename, mountpath):

Please add comments as to what these params are.

> +        self.fstype = fstype
> +        self.imagename = imagename
> +        self.mountpath = mountpath
> +        self.md5s = {}
> +        with open(self.imagename, 'w') as fd:
> +            fd.truncate(0)
> +            fd.seek(3e9)
> +            fd.write(bytes([0]))
> +
> +    def mkfs(self, log):
> +        mkfsopts = mkfs_opts.get(self.fstype)
> +        util.run_and_log(log,
> +            'mkfs {0} {1}'.format(mkfsopts, self.imagename))
> +
> +    def create_file(self, log, filename):

Please add a short comment on each non-function describing what it
does and what the args are (and return value if any).

> +        md5sums = []
> +        with open(self.mountpath + "/" + filename, 'w') as fd:

I think this is better as os.path.join(self.mountpath, filename)

> +            for stride in files[filename]:
> +                length = int(stride[1] - stride[0])
> +                data = bytearray(random.getrandbits(8) for _ in xrange(length))
> +                md5 = hashlib.md5(data).hexdigest()
> +                md5sums.append(md5)
> +                log.info("{0}: write {1} bytes @ {2} : {3}".format(
> +                    filename, int(stride[1] - stride[0]),
> +                    int(stride[0]), md5))
> +                fd.seek(stride[0])
> +                fd.write(data);
> +        self.md5s[filename] = md5sums
> +
> +    def create_files(self, log):
> +        with log.section("Create initial files"):
> +            for filename in files:
> +                self.create_file(log, filename)
> +            log.info("Created test files in {0}".format(self.mountpath))
> +            util.run_and_log(log, 'ls -la {0}'.format(self.mountpath))
> +            util.run_and_log(log, 'sync {0}'.format(self.mountpath))
> +
> +    def mount(self, log):
> +        if not os.path.exists(self.mountpath):
> +            os.mkdir(self.mountpath)
> +        log.info("Mounting {0} at {1}".format(self.imagename, self.mountpath))
> +        if self.fstype == "ext4":
> +            cmd = 'sudo -n mount -o loop,rw {0} {1}'.format(self.imagename, self.mountpath)
> +        else:
> +            cmd = 'sudo -n mount -o loop,rw,umask=000 {0} {1}'.format(self.imagename, self.mountpath)
> +        util.run_and_log(log, cmd)
> +        if self.fstype == "ext4":
> +            cmd = 'sudo -n chmod og+rw {0}'.format(self.mountpath)
> +            return util.run_and_log(log, cmd)
> +
> +    def unmount(self, log):
> +        log.info("Unmounting {0}".format(self.imagename))
> +        cmd = 'sudo -n umount -l {0}'.format(self.mountpath)
> +        util.run_and_log(log, cmd, ignore_errors=True)
> +
> +
> + at pytest.fixture(scope="module", params=["fat", "ext4"])
> +def fsimage(prereq_commands, u_boot_config, u_boot_log, request):
> +    datadir = u_boot_config.result_dir + '/'
> +    fstype = request.param
> +    imagename = datadir + "3GB." + fstype + ".img"
> +    mountpath = datadir + "mnt_" + fstype
> +
> +    with u_boot_log.section('Create image {0}'.format(imagename)):
> +        fsimage = FsImage(fstype, imagename, mountpath)
> +        fsimage.mkfs(u_boot_log)
> +
> +    yield fsimage
> +    fsimage.unmount(u_boot_log)
> +
> + at pytest.fixture(scope="module")
> +def populated_image(fsimage, u_boot_log):
> +    try:
> +        fsimage.mount(u_boot_log)
> +    except Exception as e:
> +        pytest.skip('{0}: could not mount {1}'.format(
> +            fsimage.fstype, fsimage.imagename))
> +        yield None
> +
> +    fsimage.create_files(u_boot_log)
> +    fsimage.unmount(u_boot_log)
> +    yield fsimage
> +
> +# 'Transfer' the image to the client and make it accessible

Please see other files to see how function comments should be formatted.

> + at pytest.fixture(scope="function")
> +def boundimage(populated_image, u_boot_console, request):
> +    image = populated_image
> +    if request.cls.scenario == "hostfs":
> +        image.mount(u_boot_console.log)
> +        image.rootpath = image.mountpath + "/"
> +        yield image
> +        image.unmount(u_boot_console.log)
> +    else:
> +        output = u_boot_console.run_command_list(
> +            ["host bind 0 {0}".format(image.imagename)])
> +        image.rootpath = "/"
> +        yield image
> +        output = u_boot_console.run_command_list(["host bind 0 "])
> +
> +
> +# Dummy test to create an image file with filesystem
> +# Useful to isolate fixture setup from actual tests
> +def test_fs_prepare_image(u_boot_config, fsimage, request):
> +    if not fsimage:
> +        pytest.fail("Failed to create image")
> +
> +# Dummy test to create initial filesystem contents
> +def test_fs_populate_image(populated_image, request):
> +    if not populated_image:
> +        pytest.fail("Failed create initial image content")
> +
> +# Scenarios:
> +#   hostfs: access image contents through the sandbox hostfs
> +#       facility, using the filesytem implementation of
> +#       the sandbox host, e.g. Linux kernel
> +#   generic: test u-boots native filesystem implementations,
> +#       using the 'generic' command names, e.g. 'load'
> +#   TODO -
> +#   fscommands: test u-boots native filesystem implementations,
> +#       using the fs specific commands, e.g. 'ext4load'
> + at pytest.fixture(scope="class", params=["generic", "hostfs"])
> +def scenario(request):
> +    request.cls.scenario = request.param
> +    return request.param
> +
> + at pytest.mark.usefixtures("u_boot_console", "scenario")
> +class TestFilesystems:
> +    ignore_cleanup_errors = True
> +    filesize_regex = re.compile("^filesize=([A-Fa-f0-9]+)")
> +    md5sum_regex = re.compile("^md5 for .* ==> ([A-Fa-f0-9]{32})")
> +    dirlist_regex = re.compile("\s+(\d+)\s+(\S+.file\S*)")

Should these be self. ?

> +
> +    def get_filesize(self, filename):
> +        strides = files[filename]
> +        return int(strides[-1][1])
> +
> +    def check_dirlist(self, string, filenames):
> +        m = self.dirlist_regex.findall(string)
> +        assert(m)
> +        for i, e in enumerate(m):
> +            m[i] = (int(e[0]), e[1].lower())
> +        for f in filenames:
> +            e = (self.get_filesize(f), f.lower())
> +            assert(e in m)
> +
> +    def check_filesize(self, string, size):
> +        m = self.filesize_regex.match(string)
> +        assert(m)
> +        assert(int(m.group(1), 16) == size)
> +
> +    def check_md5sum(self, string, md5):
> +        m = self.md5sum_regex.match(string)
> +        assert(m)
> +        assert(len(m.group(1)) == 32)
> +        assert(m.group(1) == md5)
> +
> +    def run_listcmd(self, rootpath, dirname):
> +        cmd = "{0}{1} {2} {3}".format(
> +            self.fs_params.get('prefix'),
> +            self.fs_commands.get('listcmd'),
> +            self.fs_params.get('interface'),
> +            rootpath + dirname)
> +        with self.console.log.section('List "{0}"'.format(dirname)):
> +            output = self.console.run_command_list([cmd])
> +            return output[0]
> +
> +    def run_readcmd(self, rootpath, filename, offset, length):
> +        cmd = "{0}{1} {2} {3} {4} 0x{5:x} 0x{6:x}".format(
> +            self.fs_params.get('prefix'),
> +            self.fs_commands.get('readcmd'),
> +            self.fs_params.get('interface'),
> +            "0", # address
> +            rootpath + filename,
> +            length, offset)
> +        with self.console.log.section('Read file "{0}"'.format(filename)):
> +            output = self.console.run_command_list(
> +                [cmd, "env print filesize",
> +                 "md5sum 0 $filesize", "env set filesize"])
> +            return output[1:3]
> +
> +    def run_sizecmd(self, rootpath, filename):
> +        cmd = "{0}{1} {2} {3}".format(
> +            self.fs_params.get('prefix'),
> +            self.fs_commands.get('sizecmd'),
> +            self.fs_params.get('interface'),
> +            rootpath + filename)
> +        with self.console.log.section('Get size of "{0}"'.format(filename)):
> +            output = self.console.run_command_list(
> +                [cmd, "env print filesize", "env set filesize"])
> +            return output[1]
> +
> +    @pytest.mark.parametrize("dirname", ["", "./"])
> +    def test_fs_ls(self, boundimage, u_boot_console, dirname):
> +        self.console = u_boot_console
> +        self.fs_params = cmd_parameters.get(self.scenario)
> +        self.fs_commands = fs_commands.get(boundimage.fstype)
> +
> +        output = self.run_listcmd(boundimage.rootpath, dirname)
> +        self.check_dirlist(output, files.keys())
> +
> +    @pytest.mark.parametrize("filename", files.keys())
> +    def test_fs_filesize(self, boundimage, u_boot_console, filename):
> +        self.console = u_boot_console
> +        self.fs_params = cmd_parameters.get(self.scenario)
> +        self.fs_commands = fs_commands.get(boundimage.fstype)
> +        filesize = self.get_filesize(filename)
> +
> +        output = self.run_sizecmd(boundimage.rootpath, filename)
> +        self.check_filesize(output, filesize)
> +
> +    @pytest.mark.parametrize("filename", files.keys())
> +    def test_fs_read(self, boundimage, u_boot_console, filename):
> +        self.console = u_boot_console
> +        self.fs_params = cmd_parameters.get(self.scenario)
> +        self.fs_commands = fs_commands.get(boundimage.fstype)
> +        md5s = boundimage.md5s[filename]
> +
> +        for i, stride in enumerate(files[filename]):
> +            length = int(stride[1]) - int(stride[0])
> +            output = self.run_readcmd(
> +                boundimage.rootpath, filename, int(stride[0]), length)
> +            self.check_filesize(output[0], length)
> +            self.console.log.info("md5: {0}".format(md5s[i]))
> +            self.check_md5sum(output[1], md5s[i])
> +
> +    @pytest.mark.parametrize("filename", files.keys())
> +    def test_fs_read_head(self, boundimage, u_boot_console, filename):
> +        self.console = u_boot_console
> +        self.fs_params = cmd_parameters.get(self.scenario)
> +        self.fs_commands = fs_commands.get(boundimage.fstype)
> +
> +        filesize = self.get_filesize(filename)
> +        filesize = min(filesize, 4e6)
> +
> +        output = self.run_readcmd(
> +            boundimage.rootpath, filename, 0, filesize)
> +        self.check_filesize(output[0], filesize)
> --
> 2.10.1
>

Regards,
Simon