From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CFDDDC4707F for ; Thu, 27 May 2021 13:44:27 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A0B45613AC for ; Thu, 27 May 2021 13:44:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A0B45613AC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7481E82D14; Thu, 27 May 2021 15:44:24 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="RX1h+FqF"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id D7C2982D56; Thu, 27 May 2021 15:44:22 +0200 (CEST) Received: from mail-wr1-x42d.google.com (mail-wr1-x42d.google.com [IPv6:2a00:1450:4864:20::42d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 975F982052 for ; Thu, 27 May 2021 15:44:19 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-wr1-x42d.google.com with SMTP id n2so7958wrm.0 for ; Thu, 27 May 2021 06:44:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=FgN8/YYZskKjm348srJbYrw9n1rpImxTZEJbJ+r5e2g=; b=RX1h+FqFfFz5kMI9uVUS/0T0Y8RPiromRmnLj55JNiFrpSj6aYqvFhSBA0pNhCcf3Z fZRvsDMPpsNhhLdIzl0EyA9/8EfC1MzeS1nZpNvMSDVnAnr6etAGQG35gkO+Zll3zzAH Ym0A43vQdqgKA756xoR9wcT1sP7FnmIBNbdA0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=FgN8/YYZskKjm348srJbYrw9n1rpImxTZEJbJ+r5e2g=; b=gD9wyFE6UCvGNsprNghSZPNgAmLiE4ArPquCIxBDaBrzr+/2dIRwuqpQ1i7YlIcciS p7ZEuVqQN5gUZADizFGn9YwItlzcxAVPTYzI1XcyEz8ZuhKnpfRRuhKb42Dx/MKx6Nbv hntDxOhD8vxUIgcvyVELK01bzJVmAo+0i1FLZ7Ft68zmIN7KPXeFePEaBl50gJ7z9/7c xSWfMA80SWjtKncPYYrE72w3njL+bay0v07ZRxIAe+zrKi9/MJhs9UVmZBAXe60BiLKy 5weiiSc9NQ+IGGkch/6fg99TM7bgdASrjHoH2bfrYftBdBlPFSkNpt/+um7hWa25pna1 g57w== X-Gm-Message-State: AOAM533/zN+Qa+MPuXXhS2pibLSasq/eDnOAV5FxUW+basxkOwoKu4aJ Rz3DeYiQmO2acYeKG35rVCPCs4/54jFZtgo3TKgauA== X-Google-Smtp-Source: ABdhPJzhWRHr1wN8iivKjQYr2szL+y7eXPJkMSHROLYySz9T5OUiYcu43bOhc3I2pPrZQlTdNJ1VsvIkBNWdyRc5k5g= X-Received: by 2002:adf:c44b:: with SMTP id a11mr3593317wrg.204.1622123058798; Thu, 27 May 2021 06:44:18 -0700 (PDT) MIME-Version: 1.0 References: <20210524023133.22100-1-jmcosta944@gmail.com> <20210524023133.22100-2-jmcosta944@gmail.com> In-Reply-To: <20210524023133.22100-2-jmcosta944@gmail.com> From: Simon Glass Date: Thu, 27 May 2021 07:44:06 -0600 Message-ID: Subject: Re: [PATCH 1/3] test/py: rewrite common tools for SquashFS tests To: Joao Marcos Costa Cc: U-Boot Mailing List , Thomas Petazzoni , Miquel Raynal , Richard Genoud Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.102.4 at phobos.denx.de X-Virus-Status: Clean Hi Joao, On Sun, 23 May 2021 at 20:31, Joao Marcos Costa wrot= e: > > 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 =3D [] > - 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 represen= ting the > +output image name and its respective mksquashfs options. This table shou= ld be > +modified only when adding support for new compression algorithms. > +The 'default' case takes no options but the input and output names, so i= t must > +be assigned with an empty string. > +""" > +standard_table =3D { > + '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 squash= fs images with > +your own customized options. > +""" > +extra_table =3D {} > > -def sqfs_generate_file(path, size): > - content =3D sqfs_get_random_letters(size) > - file =3D open(path, "w") > +# path to source directory used to make squashfs test images > +sqfs_src_dir =3D '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 ite= m is the 'default' case. Args: fred: Description Returns: what it returns """ > + # supported compression options only > + comp_opts =3D ['-comp lzo', '-comp zstd', '-comp gzip'] > + # file fragmentation options > + frag_opts =3D ['-always-use-fragments', '-always-use-fragments -noF'= , '-no-fragments'] > + > + opts_list =3D [' '] > + 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 =3D get_opts_list() > + > + for key, value in zip(standard_table.keys(), opts_list): > + standard_table[key] =3D value > + > +def generate_file(file_name, file_size): > + # file is filled with file_size * 'x' > + content =3D '' > + for i in range(file_size): > + content +=3D 'x' content =3D 'x' * file_size > + > + file =3D open(file_name, 'w') > file.write(content) > file.close() > [..] [..] > +""" > +sqfs_src_dir > +=E2=94=9C=E2=94=80=E2=94=80 empty-dir > +=E2=94=9C=E2=94=80=E2=94=80 f1000 > +=E2=94=9C=E2=94=80=E2=94=80 f4096 > +=E2=94=9C=E2=94=80=E2=94=80 f5096 > +=E2=94=9C=E2=94=80=E2=94=80 subdir > +=E2=94=82 =E2=94=94=E2=94=80=E2=94=80 subdir-file > +=E2=94=94=E2=94=80=E2=94=80 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 =3D os.path.join(build_dir, sqfs_src_dir) Call it root ? > + print(path) > + #path =3D build_dir + '/' + sqfs_src_dir Can we drop those two lines? > + # make root directory > + os.makedirs(path) > + > + # 4096: minimum block size > + file_name =3D 'f4096' > + generate_file(os.path.join(path, file_name), 4096) > + > + # 5096: minimum block size + 1000 chars (fragment) > + file_name =3D 'f5096' > + generate_file(os.path.join(path, file_name), 5096) > + > + # 1000: less than minimum block size (fragment only) > + file_name =3D 'f1000' > + generate_file(os.path.join(path, file_name), 1000) > + > + # sub-directory with a single file inside > + subdir_path =3D os.path.join(path, 'subdir') > + os.makedirs(subdir_path) > + generate_file(os.path.join(subdir_path, 'subdir-file'), 100) > + > + # symlink (target: sub-directory) > + os.symlink('subdir', os.path.join(path, 'sym')) > + > + # empty directory > + os.makedirs(os.path.join(path, 'empty-dir')) > + > +def mksquashfs(args): > + subprocess.run(['mksquashfs ' + args], shell =3D True, check =3D Tru= e, > + stdout =3D subprocess.DEVNULL) > + > +def make_all_images(build_dir): > + > + init_standard_table() > + input_path =3D os.path.join(build_dir, sqfs_src_dir) > + > + # make squashfs images according to standard_table > + for out, opts in zip(standard_table.keys(), standard_table.values())= : > + output_path =3D os.path.join(build_dir, out) > + mksquashfs(' '.join([input_path, output_path, opts])) > + > + # make squashfs images according to extra_table > + for out, opts in zip(extra_table.keys(), extra_table.values()): > + output_path =3D os.path.join(build_dir, out) > + mksquashfs(' '.join([input_path, output_path, opts])) > + > +def clean_all_images(build_dir): > + for image_name in standard_table.keys(): > + image_path =3D os.path.join(build_dir, image_name) > + os.remove(image_path) > + > + for image_name in extra_table.keys(): > + image_path =3D os.path.join(build_dir, image_name) > + os.remove(image_path) > + > +def clean_sqfs_src_dir(build_dir): > + path =3D os.path.join(build_dir, sqfs_src_dir) > + shutil.rmtree(path) > -- > 2.25.1 > Regards, Simon