Re: [PATCH 1/3] test/py: rewrite common tools for SquashFS tests
Hello, Em qui., 27 de mai. de 2021 às 10:44, Simon Glass escreveu: > Hi Joao, > > On Sun, 23 May 2021 at 20:31, Joao Marcos Costa > wrote: > > > > Remove the previous OOP approach, which was confusing and incomplete. > > Add more test cases by making SquashFS images with various options, > > concerning file fragmentation and its compression. Add comments to > > properly document the code. > > > > Signed-off-by: Joao Marcos Costa > > --- > [...] > > While we are here, can you please be a little more formal with your > function comments? pylint3 will tell you about it if you run it on > this file. > > [...] > > Regards, > Simon > Absolutely, Simon, and thanks for suggesting pylint3! -- Atenciosamente, João Marcos Costa www.linkedin.com/in/jmarcoscosta/ https://github.com/jmarcoscosta
Re: [PATCH 1/3] test/py: rewrite common tools for SquashFS tests
Hi Joao,
On Sun, 23 May 2021 at 20:31, Joao Marcos Costa wrote:
>
> Remove the previous OOP approach, which was confusing and incomplete.
> Add more test cases by making SquashFS images with various options,
> concerning file fragmentation and its compression. Add comments to
> properly document the code.
>
> Signed-off-by: Joao Marcos Costa
> ---
> .../test_fs/test_squashfs/sqfs_common.py | 198 --
> 1 file changed, 133 insertions(+), 65 deletions(-)
>
> diff --git a/test/py/tests/test_fs/test_squashfs/sqfs_common.py
> b/test/py/tests/test_fs/test_squashfs/sqfs_common.py
> index c96f92c1d8..81a378a9f9 100644
> --- a/test/py/tests/test_fs/test_squashfs/sqfs_common.py
> +++ b/test/py/tests/test_fs/test_squashfs/sqfs_common.py
> @@ -1,76 +1,144 @@
> # SPDX-License-Identifier: GPL-2.0
> -# Copyright (C) 2020 Bootlin
> +# Copyright (C) 2021 Bootlin
> # Author: Joao Marcos Costa
>
> import os
> -import random
> -import string
> +import shutil
> import subprocess
>
> -def sqfs_get_random_letters(size):
> -letters = []
> -for i in range(0, size):
> -letters.append(random.choice(string.ascii_letters))
> +"""
> +standard test images table: Each table item is a key:value pair representing
> the
> +output image name and its respective mksquashfs options. This table should be
> +modified only when adding support for new compression algorithms.
> +The 'default' case takes no options but the input and output names, so it
> must
> +be assigned with an empty string.
> +"""
> +standard_table = {
> +'default' : '',
> +'lzo_comp_frag' : '',
> +'lzo_frag' : '',
> +'lzo_no_frag' : '',
> +'zstd_comp_frag' : '',
> +'zstd_frag' : '',
> +'zstd_no_frag' : '',
> +'gzip_comp_frag' : '',
> +'gzip_frag' : '',
> +'gzip_no_frag' : ''
> +}
>
> -return ''.join(letters)
> +"""
> +extra_table: Set this table's keys and values if you want to make squashfs
> images with
> +your own customized options.
> +"""
> +extra_table = {}
>
> -def sqfs_generate_file(path, size):
> -content = sqfs_get_random_letters(size)
> -file = open(path, "w")
> +# path to source directory used to make squashfs test images
> +sqfs_src_dir = 'sqfs_src_dir'
> +
> +"""
> +Combines fragmentation and compression options into a list of strings.
> +opts_list's firts item is an empty string as standard_table's first item is
> +the 'default' case.
> +"""
> +def get_opts_list():
While we are here, can you please be a little more formal with your
function comments? pylint3 will tell you about it if you run it on
this file.
Basically you do something like this (omitting Args or Returns if
there isn't anything):
def get_opts_list():
"""Combine fragmentation and compression options into a list of strings
The first returned item is an empty string as standard_table's first item is
the 'default' case.
Args:
fred: Description
Returns:
what it returns
"""
> +# supported compression options only
> +comp_opts = ['-comp lzo', '-comp zstd', '-comp gzip']
> +# file fragmentation options
> +frag_opts = ['-always-use-fragments', '-always-use-fragments -noF',
> '-no-fragments']
> +
> +opts_list = [' ']
> +for c in comp_opts:
> +for f in frag_opts:
> +opts_list.append(' '.join([c, f]))
> +
> +return opts_list
> +
> +def init_standard_table():
> +opts_list = get_opts_list()
> +
> +for key, value in zip(standard_table.keys(), opts_list):
> +standard_table[key] = value
> +
> +def generate_file(file_name, file_size):
> +# file is filled with file_size * 'x'
> +content = ''
> +for i in range(file_size):
> +content += 'x'
content = 'x' * file_size
> +
> +file = open(file_name, 'w')
> file.write(content)
> file.close()
>
[..]
[..]
> +"""
> +sqfs_src_dir
> +├── empty-dir
> +├── f1000
> +├── f4096
> +├── f5096
> +├── subdir
> +│ └── subdir-file
> +└── sym -> subdir
> +
> +3 directories, 4 files
> +"""
That should be a comment for this function.
> +def generate_sqfs_src_dir(build_dir):
"""What this function does
put above info here
"""
> +path = os.path.join(build_dir, sqfs_src_dir)
Call it root ?
> +print(path)
> +#path = build_dir + '/' + sqfs_src_dir
Can we drop those two lines?
> +# make root directory
> +os.makedirs(path)
> +
> +# 4096: minimum block size
> +file_name = 'f4096'
> +generate_file(os.path.join(path, file_name), 4096)
> +
> +# 5096: minimum block size + 1000 chars (fragment)
> +file_name = 'f5096'
> +generate_file(os.path.join(path, file_name), 5096)
> +
> +# 1000: less than minimum block size (fragment only)
> +file_name = 'f1000'
> +generate_file(os.path.join(path, file_name), 1000)
> +
> +# sub-directory with a single file inside
> +subdir_path = os.path.join(path, 'subdir')
> +os.makedirs(subdir_path)
> +
Re: [PATCH 1/3] test/py: rewrite common tools for SquashFS tests
On Sun, May 23, 2021 at 11:31:31PM -0300, Joao Marcos Costa wrote: > Remove the previous OOP approach, which was confusing and incomplete. > Add more test cases by making SquashFS images with various options, > concerning file fragmentation and its compression. Add comments to > properly document the code. > > Signed-off-by: Joao Marcos Costa > --- > .../test_fs/test_squashfs/sqfs_common.py | 198 -- > 1 file changed, 133 insertions(+), 65 deletions(-) > > diff --git a/test/py/tests/test_fs/test_squashfs/sqfs_common.py > b/test/py/tests/test_fs/test_squashfs/sqfs_common.py > index c96f92c1d8..81a378a9f9 100644 > --- a/test/py/tests/test_fs/test_squashfs/sqfs_common.py > +++ b/test/py/tests/test_fs/test_squashfs/sqfs_common.py > @@ -1,76 +1,144 @@ > # SPDX-License-Identifier: GPL-2.0 > -# Copyright (C) 2020 Bootlin > +# Copyright (C) 2021 Bootlin Unless you have some different internal policy, it's typical to extend copyright years, not replace them. Thanks. -- Tom signature.asc Description: PGP signature
[PATCH 1/3] test/py: rewrite common tools for SquashFS tests
Remove the previous OOP approach, which was confusing and incomplete.
Add more test cases by making SquashFS images with various options,
concerning file fragmentation and its compression. Add comments to
properly document the code.
Signed-off-by: Joao Marcos Costa
---
.../test_fs/test_squashfs/sqfs_common.py | 198 --
1 file changed, 133 insertions(+), 65 deletions(-)
diff --git a/test/py/tests/test_fs/test_squashfs/sqfs_common.py
b/test/py/tests/test_fs/test_squashfs/sqfs_common.py
index c96f92c1d8..81a378a9f9 100644
--- a/test/py/tests/test_fs/test_squashfs/sqfs_common.py
+++ b/test/py/tests/test_fs/test_squashfs/sqfs_common.py
@@ -1,76 +1,144 @@
# SPDX-License-Identifier: GPL-2.0
-# Copyright (C) 2020 Bootlin
+# Copyright (C) 2021 Bootlin
# Author: Joao Marcos Costa
import os
-import random
-import string
+import shutil
import subprocess
-def sqfs_get_random_letters(size):
-letters = []
-for i in range(0, size):
-letters.append(random.choice(string.ascii_letters))
+"""
+standard test images table: Each table item is a key:value pair representing
the
+output image name and its respective mksquashfs options. This table should be
+modified only when adding support for new compression algorithms.
+The 'default' case takes no options but the input and output names, so it must
+be assigned with an empty string.
+"""
+standard_table = {
+'default' : '',
+'lzo_comp_frag' : '',
+'lzo_frag' : '',
+'lzo_no_frag' : '',
+'zstd_comp_frag' : '',
+'zstd_frag' : '',
+'zstd_no_frag' : '',
+'gzip_comp_frag' : '',
+'gzip_frag' : '',
+'gzip_no_frag' : ''
+}
-return ''.join(letters)
+"""
+extra_table: Set this table's keys and values if you want to make squashfs
images with
+your own customized options.
+"""
+extra_table = {}
-def sqfs_generate_file(path, size):
-content = sqfs_get_random_letters(size)
-file = open(path, "w")
+# path to source directory used to make squashfs test images
+sqfs_src_dir = 'sqfs_src_dir'
+
+"""
+Combines fragmentation and compression options into a list of strings.
+opts_list's firts item is an empty string as standard_table's first item is
+the 'default' case.
+"""
+def get_opts_list():
+# supported compression options only
+comp_opts = ['-comp lzo', '-comp zstd', '-comp gzip']
+# file fragmentation options
+frag_opts = ['-always-use-fragments', '-always-use-fragments -noF',
'-no-fragments']
+
+opts_list = [' ']
+for c in comp_opts:
+for f in frag_opts:
+opts_list.append(' '.join([c, f]))
+
+return opts_list
+
+def init_standard_table():
+opts_list = get_opts_list()
+
+for key, value in zip(standard_table.keys(), opts_list):
+standard_table[key] = value
+
+def generate_file(file_name, file_size):
+# file is filled with file_size * 'x'
+content = ''
+for i in range(file_size):
+content += 'x'
+
+file = open(file_name, 'w')
file.write(content)
file.close()
-class Compression:
-def __init__(self, name, files, sizes, block_size = 4096):
-self.name = name
-self.files = files
-self.sizes = sizes
-self.mksquashfs_opts = " -b " + str(block_size) + " -comp " + self.name
-
-def add_opt(self, opt):
-self.mksquashfs_opts += " " + opt
-
-def gen_image(self, build_dir):
-src = os.path.join(build_dir, "sqfs_src/")
-os.mkdir(src)
-for (f, s) in zip(self.files, self.sizes):
-sqfs_generate_file(src + f, s)
-
-# the symbolic link always targets the first file
-os.symlink(self.files[0], src + "sym")
-
-sqfs_img = os.path.join(build_dir, "sqfs-" + self.name)
-i_o = src + " " + sqfs_img
-opts = self.mksquashfs_opts
-try:
-subprocess.run(["mksquashfs " + i_o + opts], shell = True, check =
True)
-except:
-print("mksquashfs error. Compression type: " + self.name)
-raise RuntimeError
-
-def clean_source(self, build_dir):
-src = os.path.join(build_dir, "sqfs_src/")
-for f in self.files:
-os.remove(src + f)
-os.remove(src + "sym")
-os.rmdir(src)
-
-def cleanup(self, build_dir):
-self.clean_source(build_dir)
-sqfs_img = os.path.join(build_dir, "sqfs-" + self.name)
-os.remove(sqfs_img)
-
-files = ["blks_only", "blks_frag", "frag_only"]
-sizes = [4096, 5100, 100]
-gzip = Compression("gzip", files, sizes)
-zstd = Compression("zstd", files, sizes)
-lzo = Compression("lzo", files, sizes)
-
-# use fragment blocks for files larger than block_size
-gzip.add_opt("-always-use-fragments")
-zstd.add_opt("-always-use-fragments")
-
-# avoid fragments if lzo is used
-lzo.add_opt("-no-fragments")
-
-comp_opts = [gzip, zstd, lzo]
+"""
+sqfs_src_dir
+├── empty-dir
+├── f1000
+├── f4096
+├── f5096
+├── subdir
+│ └── subdir-file
+└──
