[PATCH] patman: do not hardcode coverage tool

[PATCH] patman: do not hardcode coverage tool

[Michal Suchanek]

The coverage tool name varies across distributions.

Add COVERAGE variable to specify the tool name.

Also there is one place where prefix is prepended to the tool path,
remove the prefix.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 tools/patman/test_util.py | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index 0f6d1aa902..e11806b626 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -15,6 +15,8 @@ from patman import command
 
 from io import StringIO
 
+coverage = os.environ.get('COVERAGE', 'python3-coverage')
+
 buffer_outputs = True
 use_concurrent = True
 try:
@@ -58,11 +60,11 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
     prefix = ''
     if build_dir:
         prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
-    cmd = ('%spython3-coverage run '
-           '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
+    cmd = ('%s run '
+           '--omit "%s" %s %s %s -P1' % (coverage, ','.join(glob_list),
                                          prog, extra_args or '', test_cmd))
     os.system(cmd)
-    stdout = command.output('python3-coverage', 'report')
+    stdout = command.output(coverage, 'report')
     lines = stdout.splitlines()
     if required:
         # Convert '/path/to/name.py' just the module name 'name'
@@ -76,13 +78,13 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
             print(stdout)
             ok = False
 
-    coverage = lines[-1].split(' ')[-1]
+    cov_result = lines[-1].split(' ')[-1]
     ok = True
-    print(coverage)
-    if coverage != '100%':
+    print(cov_result)
+    if cov_result != '100%':
         print(stdout)
-        print("To get a report in 'htmlcov/index.html', type: python3-coverage html")
-        print('Coverage error: %s, but should be 100%%' % coverage)
+        print("To get a report in 'htmlcov/index.html', type: %s html" % coverage)
+        print('Coverage error: %s, but should be 100%%' % cov_result)
         ok = False
     if not ok:
         raise ValueError('Test coverage failure')
-- 
2.37.1

[PATCH] tests: Do not hardcode sudo tool

[Michal Suchanek]

In some situations it may be needed to pass parameters to sudo or to use
a different tool to gain root access. Add SUDO variable to specify the
sudo tool.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 test/fs/fat-noncontig-test.sh     |  9 +++++----
 test/fs/fs-test.sh                | 26 ++++++++++++++------------
 test/py/tests/test_fs/conftest.py |  8 +++++---
 test/py/tests/test_ut.py          | 14 ++++++++------
 4 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/test/fs/fat-noncontig-test.sh b/test/fs/fat-noncontig-test.sh
index b02dae765f..7e478c6705 100755
--- a/test/fs/fat-noncontig-test.sh
+++ b/test/fs/fat-noncontig-test.sh
@@ -60,6 +60,7 @@ testfn=noncontig.img
 mnttestfn=${mnt}/${testfn}
 crcaddr=0
 loadaddr=1000
+[ -n "$SUDO" ] || SUDO=sudo
 
 for prereq in fallocate mkfs.fat dd crc32; do
     if [ ! -x "`which $prereq`" ]; then
@@ -87,7 +88,7 @@ if [ ! -f ${img} ]; then
         exit $?
     fi
 
-    sudo mount -o loop,uid=$(id -u) ${img} ${mnt}
+    $SUDO mount -o loop,uid=$(id -u) ${img} ${mnt}
     if [ $? -ne 0 ]; then
         echo Could not mount test filesystem
         exit $?
@@ -106,20 +107,20 @@ if [ ! -f ${img} ]; then
     # sector size (ignoring sizes that are multiples of both).
     dd if=${fill} of=${mnttestfn} bs=511 >/dev/null 2>&1
 
-    sudo umount ${mnt}
+    $SUDO umount ${mnt}
     if [ $? -ne 0 ]; then
         echo Could not unmount test filesystem
         exit $?
     fi
 fi
 
-sudo mount -o ro,loop,uid=$(id -u) ${img} ${mnt}
+$SUDO mount -o ro,loop,uid=$(id -u) ${img} ${mnt}
 if [ $? -ne 0 ]; then
     echo Could not mount test filesystem
     exit $?
 fi
 crc=0x`crc32 ${mnttestfn}`
-sudo umount ${mnt}
+$SUDO umount ${mnt}
 if [ $? -ne 0 ]; then
     echo Could not unmount test filesystem
     exit $?
diff --git a/test/fs/fs-test.sh b/test/fs/fs-test.sh
index b87748106c..bd55ff51b6 100755
--- a/test/fs/fs-test.sh
+++ b/test/fs/fs-test.sh
@@ -55,6 +55,8 @@ OUT="${OUT_DIR}/fs-test"
 MB1="${MOUNT_DIR}/${SMALL_FILE}"
 GB2p5="${MOUNT_DIR}/${BIG_FILE}"
 
+[ -n "$SUDO" ] || SUDO=sudo
+
 # ************************
 # * Functions start here *
 # ************************
@@ -351,34 +353,34 @@ EOF
 function create_files() {
 	# Mount the image so we can populate it.
 	mkdir -p "$MOUNT_DIR"
-	sudo mount -o loop,rw "$1" "$MOUNT_DIR"
+	$SUDO mount -o loop,rw "$1" "$MOUNT_DIR"
 
 	# Create a subdirectory.
-	sudo mkdir -p "$MOUNT_DIR/SUBDIR"
+	$SUDO mkdir -p "$MOUNT_DIR/SUBDIR"
 
 	# Create big file in this image.
 	# Note that we work only on the start 1MB, couple MBs in the 2GB range
 	# and the last 1 MB of the huge 2.5GB file.
 	# So, just put random values only in those areas.
 	if [ ! -f "${GB2p5}" ]; then
-		sudo dd if=/dev/urandom of="${GB2p5}" bs=1M count=1 \
+		$SUDO dd if=/dev/urandom of="${GB2p5}" bs=1M count=1 \
 			&> /dev/null
-		sudo dd if=/dev/urandom of="${GB2p5}" bs=1M count=2 seek=2047 \
+		$SUDO dd if=/dev/urandom of="${GB2p5}" bs=1M count=2 seek=2047 \
 			&> /dev/null
-		sudo dd if=/dev/urandom of="${GB2p5}" bs=1M count=1 seek=2499 \
+		$SUDO dd if=/dev/urandom of="${GB2p5}" bs=1M count=1 seek=2499 \
 			&> /dev/null
 	fi
 
 	# Create a small file in this image.
 	if [ ! -f "${MB1}" ]; then
-		sudo dd if=/dev/urandom of="${MB1}" bs=1M count=1 \
+		$SUDO dd if=/dev/urandom of="${MB1}" bs=1M count=1 \
 			&> /dev/null
 	fi
 
 	# Delete the small file copies which possibly are written as part of a
 	# previous test.
-	sudo rm -f "${MB1}.w"
-	sudo rm -f "${MB1}.w2"
+	$SUDO rm -f "${MB1}.w"
+	$SUDO rm -f "${MB1}.w2"
 
 	# Generate the md5sums of reads that we will test against small file
 	dd if="${MB1}" bs=1M skip=0 count=1 2> /dev/null | md5sum > "$2"
@@ -405,7 +407,7 @@ function create_files() {
 		2> /dev/null | md5sum >> "$2"
 
 	sync
-	sudo umount "$MOUNT_DIR"
+	$SUDO umount "$MOUNT_DIR"
 	rmdir "$MOUNT_DIR"
 }
 
@@ -597,13 +599,13 @@ for fs in ext4 fat16 fat32; do
 		uid=""
 		;;
 	esac
-	sudo mount -o loop,rw,$uid "$IMAGE" "$MOUNT_DIR"
-	sudo chmod 777 "$MOUNT_DIR"
+	$SUDO mount -o loop,rw,$uid "$IMAGE" "$MOUNT_DIR"
+	$SUDO chmod 777 "$MOUNT_DIR"
 
 	OUT_FILE="${OUT}.sb.${fs}.out"
 	test_image $IMAGE $fs $SMALL_FILE $BIG_FILE sb `pwd`/$MOUNT_DIR \
 		> ${OUT_FILE} 2>&1
-	sudo umount "$MOUNT_DIR"
+	$SUDO umount "$MOUNT_DIR"
 	rmdir "$MOUNT_DIR"
 
 	check_results $OUT_FILE $MD5_FILE_FS $SMALL_FILE $BIG_FILE
diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py
index b638284e07..21fc8ce9e2 100644
--- a/test/py/tests/test_fs/conftest.py
+++ b/test/py/tests/test_fs/conftest.py
@@ -10,6 +10,8 @@ from subprocess import call, check_call, check_output, CalledProcessError
 from fstest_defs import *
 import u_boot_utils as util
 
+sudo = os.environ.get('SUDO', 'sudo')
+
 supported_fs_basic = ['fat16', 'fat32', 'ext4']
 supported_fs_ext = ['fat16', 'fat32']
 supported_fs_mkdir = ['fat16', 'fat32']
@@ -222,11 +224,11 @@ def mount_fs(fs_type, device, mount_point):
     if re.match('fat', fs_type):
         mount_opt += ',umask=0000'
 
-    check_call('sudo mount -o %s %s %s'
+    check_call(sudo + ' mount -o %s %s %s'
         % (mount_opt, device, mount_point), shell=True)
 
     # may not be effective for some file systems
-    check_call('sudo chmod a+rw %s' % mount_point, shell=True)
+    check_call(sudo + ' chmod a+rw %s' % mount_point, shell=True)
 
 def umount_fs(mount_point):
     """Unmount a volume.
@@ -251,7 +253,7 @@ def umount_fs(mount_point):
             pass
 
     else:
-        call('sudo umount %s' % mount_point, shell=True)
+        call(sudo + ' umount %s' % mount_point, shell=True)
 
 #
 # Fixture for basic fs test
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index 35fb393c1f..698c3db448 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -8,6 +8,8 @@ import pytest
 
 import u_boot_utils
 
+sudo = os.environ.get('SUDO', 'sudo')
+
 def mkdir_cond(dirname):
     """Create a directory if it doesn't already exist
 
@@ -25,7 +27,7 @@ def setup_bootflow_image(u_boot_console):
     mkdir_cond(mnt)
 
     u_boot_utils.run_and_log(cons, 'qemu-img create %s 20M' % fname)
-    u_boot_utils.run_and_log(cons, 'sudo sfdisk %s' % fname,
+    u_boot_utils.run_and_log(cons, sudo + ' sfdisk %s' % fname,
                              stdin=b'type=c')
 
     loop = None
@@ -33,12 +35,12 @@ def setup_bootflow_image(u_boot_console):
     complete = False
     try:
         out = u_boot_utils.run_and_log(cons,
-                                       'sudo losetup --show -f -P %s' % fname)
+                                       sudo + ' losetup --show -f -P %s' % fname)
         loop = out.strip()
         fatpart = '%sp1' % loop
-        u_boot_utils.run_and_log(cons, 'sudo mkfs.vfat %s' % fatpart)
+        u_boot_utils.run_and_log(cons, sudo + ' mkfs.vfat %s' % fatpart)
         u_boot_utils.run_and_log(
-            cons, 'sudo mount -o loop %s %s -o uid=%d,gid=%d' %
+            cons, sudo + ' mount -o loop %s %s -o uid=%d,gid=%d' %
             (fatpart, mnt, os.getuid(), os.getgid()))
         mounted = True
 
@@ -84,9 +86,9 @@ label Fedora-Workstation-armhfp-31-1.9 (5.3.7-301.fc31.armv7hl)
               str(exc))
     finally:
         if mounted:
-            u_boot_utils.run_and_log(cons, 'sudo umount %s' % mnt)
+            u_boot_utils.run_and_log(cons, sudo + ' umount %s' % mnt)
         if loop:
-            u_boot_utils.run_and_log(cons, 'sudo losetup -d %s' % loop)
+            u_boot_utils.run_and_log(cons, sudo + ' losetup -d %s' % loop)
 
     if not complete:
         # Use a prepared image since we cannot create one
-- 
2.37.1

Re: [PATCH] tests: Do not hardcode sudo tool

[Simon Glass]

Hi Michal,

On Wed, 24 Aug 2022 at 00:48, Michal Suchanek <msuchanek@suse.de> wrote:
>
> In some situations it may be needed to pass parameters to sudo or to use
> a different tool to gain root access. Add SUDO variable to specify the
> sudo tool.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  test/fs/fat-noncontig-test.sh     |  9 +++++----
>  test/fs/fs-test.sh                | 26 ++++++++++++++------------
>  test/py/tests/test_fs/conftest.py |  8 +++++---
>  test/py/tests/test_ut.py          | 14 ++++++++------
>  4 files changed, 32 insertions(+), 25 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

But please can you update doc/develop/testing.rst ?

Regards,
Simon

Re: [PATCH] patman: do not hardcode coverage tool

[Simon Glass]

Hi Michal,

On Wed, 24 Aug 2022 at 00:43, Michal Suchanek <msuchanek@suse.de> wrote:
>
> The coverage tool name varies across distributions.
>
> Add COVERAGE variable to specify the tool name.
>
> Also there is one place where prefix is prepended to the tool path,
> remove the prefix.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  tools/patman/test_util.py | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

But please update the docs.

Regards,
Simon

[PATCH 1/2] patman: do not hardcode coverage tool

[Michal Suchanek]

The coverage tool name varies across distributions.

Add COVERAGE variable to specify the tool name.

Also there is one place where prefix is prepended to the tool path,
remove the prefix.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 doc/develop/testing.rst   |  3 +++
 tools/patman/test_util.py | 18 ++++++++++--------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst
index 1abe4d7f0f..054fbfc814 100644
--- a/doc/develop/testing.rst
+++ b/doc/develop/testing.rst
@@ -17,6 +17,9 @@ To run most tests on sandbox, type this::
 in the U-Boot directory. Note that only the pytest suite is run using this
 command.
 
+Note: external tool `python3-coverage` is used by tests. The environment
+variable `COVERAGE` can be set to alternative name or location of this tool.
+
 Some tests take ages to run and are marked with @pytest.mark.slow. To run just
 the quick ones, type this::
 
diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index 0f6d1aa902..e11806b626 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -15,6 +15,8 @@ from patman import command
 
 from io import StringIO
 
+coverage = os.environ.get('COVERAGE', 'python3-coverage')
+
 buffer_outputs = True
 use_concurrent = True
 try:
@@ -58,11 +60,11 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
     prefix = ''
     if build_dir:
         prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
-    cmd = ('%spython3-coverage run '
-           '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
+    cmd = ('%s run '
+           '--omit "%s" %s %s %s -P1' % (coverage, ','.join(glob_list),
                                          prog, extra_args or '', test_cmd))
     os.system(cmd)
-    stdout = command.output('python3-coverage', 'report')
+    stdout = command.output(coverage, 'report')
     lines = stdout.splitlines()
     if required:
         # Convert '/path/to/name.py' just the module name 'name'
@@ -76,13 +78,13 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
             print(stdout)
             ok = False
 
-    coverage = lines[-1].split(' ')[-1]
+    cov_result = lines[-1].split(' ')[-1]
     ok = True
-    print(coverage)
-    if coverage != '100%':
+    print(cov_result)
+    if cov_result != '100%':
         print(stdout)
-        print("To get a report in 'htmlcov/index.html', type: python3-coverage html")
-        print('Coverage error: %s, but should be 100%%' % coverage)
+        print("To get a report in 'htmlcov/index.html', type: %s html" % coverage)
+        print('Coverage error: %s, but should be 100%%' % cov_result)
         ok = False
     if not ok:
         raise ValueError('Test coverage failure')
-- 
2.37.1

[PATCH 2/2] tests: Do not hardcode sudo tool

[Michal Suchanek]

In some situations it may be needed to pass parameters to sudo or to use
a different tool to gain root access. Add SUDO variable to specify the
sudo tool.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 doc/develop/testing.rst           |  5 +++--
 test/fs/fat-noncontig-test.sh     |  9 +++++----
 test/fs/fs-test.sh                | 26 ++++++++++++++------------
 test/py/tests/test_fs/conftest.py |  8 +++++---
 test/py/tests/test_ut.py          | 14 ++++++++------
 5 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst
index 054fbfc814..622c2f7924 100644
--- a/doc/develop/testing.rst
+++ b/doc/develop/testing.rst
@@ -17,8 +17,9 @@ To run most tests on sandbox, type this::
 in the U-Boot directory. Note that only the pytest suite is run using this
 command.
 
-Note: external tool `python3-coverage` is used by tests. The environment
-variable `COVERAGE` can be set to alternative name or location of this tool.
+Note: external tools `sudo` and  `python3-coverage` are used by tests. The
+environment variables `SUDO` and `COVERAGE` can be set to alternative name or
+location of the tools or to specify additional parameters.
 
 Some tests take ages to run and are marked with @pytest.mark.slow. To run just
 the quick ones, type this::
diff --git a/test/fs/fat-noncontig-test.sh b/test/fs/fat-noncontig-test.sh
index b02dae765f..7e478c6705 100755
--- a/test/fs/fat-noncontig-test.sh
+++ b/test/fs/fat-noncontig-test.sh
@@ -60,6 +60,7 @@ testfn=noncontig.img
 mnttestfn=${mnt}/${testfn}
 crcaddr=0
 loadaddr=1000
+[ -n "$SUDO" ] || SUDO=sudo
 
 for prereq in fallocate mkfs.fat dd crc32; do
     if [ ! -x "`which $prereq`" ]; then
@@ -87,7 +88,7 @@ if [ ! -f ${img} ]; then
         exit $?
     fi
 
-    sudo mount -o loop,uid=$(id -u) ${img} ${mnt}
+    $SUDO mount -o loop,uid=$(id -u) ${img} ${mnt}
     if [ $? -ne 0 ]; then
         echo Could not mount test filesystem
         exit $?
@@ -106,20 +107,20 @@ if [ ! -f ${img} ]; then
     # sector size (ignoring sizes that are multiples of both).
     dd if=${fill} of=${mnttestfn} bs=511 >/dev/null 2>&1
 
-    sudo umount ${mnt}
+    $SUDO umount ${mnt}
     if [ $? -ne 0 ]; then
         echo Could not unmount test filesystem
         exit $?
     fi
 fi
 
-sudo mount -o ro,loop,uid=$(id -u) ${img} ${mnt}
+$SUDO mount -o ro,loop,uid=$(id -u) ${img} ${mnt}
 if [ $? -ne 0 ]; then
     echo Could not mount test filesystem
     exit $?
 fi
 crc=0x`crc32 ${mnttestfn}`
-sudo umount ${mnt}
+$SUDO umount ${mnt}
 if [ $? -ne 0 ]; then
     echo Could not unmount test filesystem
     exit $?
diff --git a/test/fs/fs-test.sh b/test/fs/fs-test.sh
index b87748106c..bd55ff51b6 100755
--- a/test/fs/fs-test.sh
+++ b/test/fs/fs-test.sh
@@ -55,6 +55,8 @@ OUT="${OUT_DIR}/fs-test"
 MB1="${MOUNT_DIR}/${SMALL_FILE}"
 GB2p5="${MOUNT_DIR}/${BIG_FILE}"
 
+[ -n "$SUDO" ] || SUDO=sudo
+
 # ************************
 # * Functions start here *
 # ************************
@@ -351,34 +353,34 @@ EOF
 function create_files() {
 	# Mount the image so we can populate it.
 	mkdir -p "$MOUNT_DIR"
-	sudo mount -o loop,rw "$1" "$MOUNT_DIR"
+	$SUDO mount -o loop,rw "$1" "$MOUNT_DIR"
 
 	# Create a subdirectory.
-	sudo mkdir -p "$MOUNT_DIR/SUBDIR"
+	$SUDO mkdir -p "$MOUNT_DIR/SUBDIR"
 
 	# Create big file in this image.
 	# Note that we work only on the start 1MB, couple MBs in the 2GB range
 	# and the last 1 MB of the huge 2.5GB file.
 	# So, just put random values only in those areas.
 	if [ ! -f "${GB2p5}" ]; then
-		sudo dd if=/dev/urandom of="${GB2p5}" bs=1M count=1 \
+		$SUDO dd if=/dev/urandom of="${GB2p5}" bs=1M count=1 \
 			&> /dev/null
-		sudo dd if=/dev/urandom of="${GB2p5}" bs=1M count=2 seek=2047 \
+		$SUDO dd if=/dev/urandom of="${GB2p5}" bs=1M count=2 seek=2047 \
 			&> /dev/null
-		sudo dd if=/dev/urandom of="${GB2p5}" bs=1M count=1 seek=2499 \
+		$SUDO dd if=/dev/urandom of="${GB2p5}" bs=1M count=1 seek=2499 \
 			&> /dev/null
 	fi
 
 	# Create a small file in this image.
 	if [ ! -f "${MB1}" ]; then
-		sudo dd if=/dev/urandom of="${MB1}" bs=1M count=1 \
+		$SUDO dd if=/dev/urandom of="${MB1}" bs=1M count=1 \
 			&> /dev/null
 	fi
 
 	# Delete the small file copies which possibly are written as part of a
 	# previous test.
-	sudo rm -f "${MB1}.w"
-	sudo rm -f "${MB1}.w2"
+	$SUDO rm -f "${MB1}.w"
+	$SUDO rm -f "${MB1}.w2"
 
 	# Generate the md5sums of reads that we will test against small file
 	dd if="${MB1}" bs=1M skip=0 count=1 2> /dev/null | md5sum > "$2"
@@ -405,7 +407,7 @@ function create_files() {
 		2> /dev/null | md5sum >> "$2"
 
 	sync
-	sudo umount "$MOUNT_DIR"
+	$SUDO umount "$MOUNT_DIR"
 	rmdir "$MOUNT_DIR"
 }
 
@@ -597,13 +599,13 @@ for fs in ext4 fat16 fat32; do
 		uid=""
 		;;
 	esac
-	sudo mount -o loop,rw,$uid "$IMAGE" "$MOUNT_DIR"
-	sudo chmod 777 "$MOUNT_DIR"
+	$SUDO mount -o loop,rw,$uid "$IMAGE" "$MOUNT_DIR"
+	$SUDO chmod 777 "$MOUNT_DIR"
 
 	OUT_FILE="${OUT}.sb.${fs}.out"
 	test_image $IMAGE $fs $SMALL_FILE $BIG_FILE sb `pwd`/$MOUNT_DIR \
 		> ${OUT_FILE} 2>&1
-	sudo umount "$MOUNT_DIR"
+	$SUDO umount "$MOUNT_DIR"
 	rmdir "$MOUNT_DIR"
 
 	check_results $OUT_FILE $MD5_FILE_FS $SMALL_FILE $BIG_FILE
diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py
index b638284e07..21fc8ce9e2 100644
--- a/test/py/tests/test_fs/conftest.py
+++ b/test/py/tests/test_fs/conftest.py
@@ -10,6 +10,8 @@ from subprocess import call, check_call, check_output, CalledProcessError
 from fstest_defs import *
 import u_boot_utils as util
 
+sudo = os.environ.get('SUDO', 'sudo')
+
 supported_fs_basic = ['fat16', 'fat32', 'ext4']
 supported_fs_ext = ['fat16', 'fat32']
 supported_fs_mkdir = ['fat16', 'fat32']
@@ -222,11 +224,11 @@ def mount_fs(fs_type, device, mount_point):
     if re.match('fat', fs_type):
         mount_opt += ',umask=0000'
 
-    check_call('sudo mount -o %s %s %s'
+    check_call(sudo + ' mount -o %s %s %s'
         % (mount_opt, device, mount_point), shell=True)
 
     # may not be effective for some file systems
-    check_call('sudo chmod a+rw %s' % mount_point, shell=True)
+    check_call(sudo + ' chmod a+rw %s' % mount_point, shell=True)
 
 def umount_fs(mount_point):
     """Unmount a volume.
@@ -251,7 +253,7 @@ def umount_fs(mount_point):
             pass
 
     else:
-        call('sudo umount %s' % mount_point, shell=True)
+        call(sudo + ' umount %s' % mount_point, shell=True)
 
 #
 # Fixture for basic fs test
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index 35fb393c1f..698c3db448 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -8,6 +8,8 @@ import pytest
 
 import u_boot_utils
 
+sudo = os.environ.get('SUDO', 'sudo')
+
 def mkdir_cond(dirname):
     """Create a directory if it doesn't already exist
 
@@ -25,7 +27,7 @@ def setup_bootflow_image(u_boot_console):
     mkdir_cond(mnt)
 
     u_boot_utils.run_and_log(cons, 'qemu-img create %s 20M' % fname)
-    u_boot_utils.run_and_log(cons, 'sudo sfdisk %s' % fname,
+    u_boot_utils.run_and_log(cons, sudo + ' sfdisk %s' % fname,
                              stdin=b'type=c')
 
     loop = None
@@ -33,12 +35,12 @@ def setup_bootflow_image(u_boot_console):
     complete = False
     try:
         out = u_boot_utils.run_and_log(cons,
-                                       'sudo losetup --show -f -P %s' % fname)
+                                       sudo + ' losetup --show -f -P %s' % fname)
         loop = out.strip()
         fatpart = '%sp1' % loop
-        u_boot_utils.run_and_log(cons, 'sudo mkfs.vfat %s' % fatpart)
+        u_boot_utils.run_and_log(cons, sudo + ' mkfs.vfat %s' % fatpart)
         u_boot_utils.run_and_log(
-            cons, 'sudo mount -o loop %s %s -o uid=%d,gid=%d' %
+            cons, sudo + ' mount -o loop %s %s -o uid=%d,gid=%d' %
             (fatpart, mnt, os.getuid(), os.getgid()))
         mounted = True
 
@@ -84,9 +86,9 @@ label Fedora-Workstation-armhfp-31-1.9 (5.3.7-301.fc31.armv7hl)
               str(exc))
     finally:
         if mounted:
-            u_boot_utils.run_and_log(cons, 'sudo umount %s' % mnt)
+            u_boot_utils.run_and_log(cons, sudo + ' umount %s' % mnt)
         if loop:
-            u_boot_utils.run_and_log(cons, 'sudo losetup -d %s' % loop)
+            u_boot_utils.run_and_log(cons, sudo + ' losetup -d %s' % loop)
 
     if not complete:
         # Use a prepared image since we cannot create one
-- 
2.37.1

Re: [PATCH 2/2] tests: Do not hardcode sudo tool

[Simon Glass]

Hi Michal,

On Wed, 24 Aug 2022 at 23:51, Michal Suchanek <msuchanek@suse.de> wrote:
>
> In some situations it may be needed to pass parameters to sudo or to use
> a different tool to gain root access. Add SUDO variable to specify the
> sudo tool.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  doc/develop/testing.rst           |  5 +++--
>  test/fs/fat-noncontig-test.sh     |  9 +++++----
>  test/fs/fs-test.sh                | 26 ++++++++++++++------------
>  test/py/tests/test_fs/conftest.py |  8 +++++---
>  test/py/tests/test_ut.py          | 14 ++++++++------
>  5 files changed, 35 insertions(+), 27 deletions(-)

This is missing a change log and should have 'Series-version: 2' if
you are using patman.

Regards,
SImon

Re: [PATCH 1/2] patman: do not hardcode coverage tool

[Quentin Schulz]

Hi Michal,

On 8/25/22 08:49, Michal Suchanek wrote:
> The coverage tool name varies across distributions.
> 
> Add COVERAGE variable to specify the tool name.
> 
> Also there is one place where prefix is prepended to the tool path,
> remove the prefix.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>   doc/develop/testing.rst   |  3 +++
>   tools/patman/test_util.py | 18 ++++++++++--------
>   2 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst
> index 1abe4d7f0f..054fbfc814 100644
> --- a/doc/develop/testing.rst
> +++ b/doc/develop/testing.rst
> @@ -17,6 +17,9 @@ To run most tests on sandbox, type this::
>   in the U-Boot directory. Note that only the pytest suite is run using this
>   command.
>   
> +Note: external tool `python3-coverage` is used by tests. The environment
> +variable `COVERAGE` can be set to alternative name or location of this tool.
> +
>   Some tests take ages to run and are marked with @pytest.mark.slow. To run just
>   the quick ones, type this::
>   
> diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
> index 0f6d1aa902..e11806b626 100644
> --- a/tools/patman/test_util.py
> +++ b/tools/patman/test_util.py
> @@ -15,6 +15,8 @@ from patman import command
>   
>   from io import StringIO
>   
> +coverage = os.environ.get('COVERAGE', 'python3-coverage')
> +
>   buffer_outputs = True
>   use_concurrent = True
>   try:
> @@ -58,11 +60,11 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
>       prefix = ''
>       if build_dir:
>           prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
> -    cmd = ('%spython3-coverage run '
> -           '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
> +    cmd = ('%s run '
> +           '--omit "%s" %s %s %s -P1' % (coverage, ','.join(glob_list),
>                                            prog, extra_args or '', test_cmd))

What about using
python3 -m coverage run
instead?
This way we wouldn't rely on the binary name the host distribution 
chooses (python3-coverage for Ubuntu, coverage for Fedora).

I'm not sure there is a need to give the user the ability to override 
this value since I expect only coverage.py is supported at the moment?

Cheers,
Quentin

Re: [PATCH 1/2] patman: do not hardcode coverage tool

[Michal Suchánek]

On Tue, Aug 30, 2022 at 12:01:55PM +0200, Quentin Schulz wrote:
> Hi Michal,
> 
> On 8/25/22 08:49, Michal Suchanek wrote:
> > The coverage tool name varies across distributions.
> > 
> > Add COVERAGE variable to specify the tool name.
> > 
> > Also there is one place where prefix is prepended to the tool path,
> > remove the prefix.
> > 
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >   doc/develop/testing.rst   |  3 +++
> >   tools/patman/test_util.py | 18 ++++++++++--------
> >   2 files changed, 13 insertions(+), 8 deletions(-)
> > 
> > diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst
> > index 1abe4d7f0f..054fbfc814 100644
> > --- a/doc/develop/testing.rst
> > +++ b/doc/develop/testing.rst
> > @@ -17,6 +17,9 @@ To run most tests on sandbox, type this::
> >   in the U-Boot directory. Note that only the pytest suite is run using this
> >   command.
> > +Note: external tool `python3-coverage` is used by tests. The environment
> > +variable `COVERAGE` can be set to alternative name or location of this tool.
> > +
> >   Some tests take ages to run and are marked with @pytest.mark.slow. To run just
> >   the quick ones, type this::
> > diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
> > index 0f6d1aa902..e11806b626 100644
> > --- a/tools/patman/test_util.py
> > +++ b/tools/patman/test_util.py
> > @@ -15,6 +15,8 @@ from patman import command
> >   from io import StringIO
> > +coverage = os.environ.get('COVERAGE', 'python3-coverage')
> > +
> >   buffer_outputs = True
> >   use_concurrent = True
> >   try:
> > @@ -58,11 +60,11 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
> >       prefix = ''
> >       if build_dir:
> >           prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
> > -    cmd = ('%spython3-coverage run '
> > -           '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
> > +    cmd = ('%s run '
> > +           '--omit "%s" %s %s %s -P1' % (coverage, ','.join(glob_list),
> >                                            prog, extra_args or '', test_cmd))
> 
> What about using
> python3 -m coverage run
> instead?
> This way we wouldn't rely on the binary name the host distribution chooses
> (python3-coverage for Ubuntu, coverage for Fedora).
> 
> I'm not sure there is a need to give the user the ability to override this
> value since I expect only coverage.py is supported at the moment?

Then you run into the problems that you do not have coverage on
python3.4 but only python3.6 or whatever is the relevant version for
your distribution ATM.

In other words the only general solution is to specify the tool AFAICT.

Thanks

Michal

Re: [PATCH 2/2] tests: Do not hardcode sudo tool

[Michal Suchánek]

On Thu, Aug 25, 2022 at 09:01:08AM -0600, Simon Glass wrote:
> Hi Michal,
> 
> On Wed, 24 Aug 2022 at 23:51, Michal Suchanek <msuchanek@suse.de> wrote:
> >
> > In some situations it may be needed to pass parameters to sudo or to use
> > a different tool to gain root access. Add SUDO variable to specify the
> > sudo tool.
> >
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > ---
> >  doc/develop/testing.rst           |  5 +++--
> >  test/fs/fat-noncontig-test.sh     |  9 +++++----
> >  test/fs/fs-test.sh                | 26 ++++++++++++++------------
> >  test/py/tests/test_fs/conftest.py |  8 +++++---
> >  test/py/tests/test_ut.py          | 14 ++++++++------
> >  5 files changed, 35 insertions(+), 27 deletions(-)
> 
> This is missing a change log and should have 'Series-version: 2' if
> you are using patman.

Hello,

sorry for the confusion. This is v2 for

https://lists.denx.de/pipermail/u-boot/2022-August/492636.html
https://lists.denx.de/pipermail/u-boot/2022-August/492637.html

and adds the documentation changes. With that the patches are no longer
independent and need to be applied in order.

I do not use patman, the coverage results (or errors about missing
coverage tool) are displayed during test runs.

Thanks

Michal

Re: [PATCH 1/2] patman: do not hardcode coverage tool

[Quentin Schulz]

Hi Michal,

On 8/30/22 12:11, Michal Suchánek wrote:
> On Tue, Aug 30, 2022 at 12:01:55PM +0200, Quentin Schulz wrote:
>> Hi Michal,
>>
>> On 8/25/22 08:49, Michal Suchanek wrote:
>>> The coverage tool name varies across distributions.
>>>
>>> Add COVERAGE variable to specify the tool name.
>>>
>>> Also there is one place where prefix is prepended to the tool path,
>>> remove the prefix.
>>>
>>> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
>>> ---
>>>    doc/develop/testing.rst   |  3 +++
>>>    tools/patman/test_util.py | 18 ++++++++++--------
>>>    2 files changed, 13 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst
>>> index 1abe4d7f0f..054fbfc814 100644
>>> --- a/doc/develop/testing.rst
>>> +++ b/doc/develop/testing.rst
>>> @@ -17,6 +17,9 @@ To run most tests on sandbox, type this::
>>>    in the U-Boot directory. Note that only the pytest suite is run using this
>>>    command.
>>> +Note: external tool `python3-coverage` is used by tests. The environment
>>> +variable `COVERAGE` can be set to alternative name or location of this tool.
>>> +
>>>    Some tests take ages to run and are marked with @pytest.mark.slow. To run just
>>>    the quick ones, type this::
>>> diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
>>> index 0f6d1aa902..e11806b626 100644
>>> --- a/tools/patman/test_util.py
>>> +++ b/tools/patman/test_util.py
>>> @@ -15,6 +15,8 @@ from patman import command
>>>    from io import StringIO
>>> +coverage = os.environ.get('COVERAGE', 'python3-coverage')
>>> +
>>>    buffer_outputs = True
>>>    use_concurrent = True
>>>    try:
>>> @@ -58,11 +60,11 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
>>>        prefix = ''
>>>        if build_dir:
>>>            prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
>>> -    cmd = ('%spython3-coverage run '
>>> -           '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
>>> +    cmd = ('%s run '
>>> +           '--omit "%s" %s %s %s -P1' % (coverage, ','.join(glob_list),
>>>                                             prog, extra_args or '', test_cmd))
>>
>> What about using
>> python3 -m coverage run
>> instead?
>> This way we wouldn't rely on the binary name the host distribution chooses
>> (python3-coverage for Ubuntu, coverage for Fedora).
>>
>> I'm not sure there is a need to give the user the ability to override this
>> value since I expect only coverage.py is supported at the moment?
> 
> Then you run into the problems that you do not have coverage on
> python3.4 but only python3.6 or whatever is the relevant version for
> your distribution ATM.
> 

I don't understand this point? If coverage cannot run with the python 
from the host, then it won't just be possible to run it... so... we 
don't care? Is there something we can do about this I'm missing?

I would like to advocate for COVERAGE to default to "coverage" or 
"python3 -m coverage" since this is more likely to be a sane default for 
everybody, using pip or distro installs.

Cheers,
Quentin

Re: [PATCH] patman: do not hardcode coverage tool

[Quentin Schulz]

Hi Michal,

On 8/24/22 09:43, Michal Suchanek wrote:
> The coverage tool name varies across distributions.
> 
> Add COVERAGE variable to specify the tool name.
> 
> Also there is one place where prefix is prepended to the tool path,
> remove the prefix.
> 
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>   tools/patman/test_util.py | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
> index 0f6d1aa902..e11806b626 100644
> --- a/tools/patman/test_util.py
> +++ b/tools/patman/test_util.py
> @@ -15,6 +15,8 @@ from patman import command
>   
>   from io import StringIO
>   
> +coverage = os.environ.get('COVERAGE', 'python3-coverage')
> +
>   buffer_outputs = True
>   use_concurrent = True
>   try:
> @@ -58,11 +60,11 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
>       prefix = ''
>       if build_dir:
>           prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
> -    cmd = ('%spython3-coverage run '
> -           '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
> +    cmd = ('%s run '
> +           '--omit "%s" %s %s %s -P1' % (coverage, ','.join(glob_list),
>                                            prog, extra_args or '', test_cmd))
>       os.system(cmd)
> -    stdout = command.output('python3-coverage', 'report')
> +    stdout = command.output(coverage, 'report')

Please use:
command.run_pipe((coverage + ' report').split(), capture=True, 
raise_on_error=True)
instead, so that COVERAGE can contain "python3 -m coverage". (or if you 
know a way of unpacking a list, pass (coverage + ' report') unpacked to 
command.output()).

Cheers,
Quentin

Re: [PATCH 1/2] patman: do not hardcode coverage tool

[Michal Suchánek]

Hello,

On Wed, Aug 31, 2022 at 12:08:32PM +0200, Quentin Schulz wrote:
> Hi Michal,
> 
> On 8/30/22 12:11, Michal Suchánek wrote:
> > On Tue, Aug 30, 2022 at 12:01:55PM +0200, Quentin Schulz wrote:
> > > Hi Michal,
> > > 
> > > On 8/25/22 08:49, Michal Suchanek wrote:
> > > > The coverage tool name varies across distributions.
> > > > 
> > > > Add COVERAGE variable to specify the tool name.
> > > > 
> > > > Also there is one place where prefix is prepended to the tool path,
> > > > remove the prefix.
> > > > 
> > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > > ---
> > > >    doc/develop/testing.rst   |  3 +++
> > > >    tools/patman/test_util.py | 18 ++++++++++--------
> > > >    2 files changed, 13 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst
> > > > index 1abe4d7f0f..054fbfc814 100644
> > > > --- a/doc/develop/testing.rst
> > > > +++ b/doc/develop/testing.rst
> > > > @@ -17,6 +17,9 @@ To run most tests on sandbox, type this::
> > > >    in the U-Boot directory. Note that only the pytest suite is run using this
> > > >    command.
> > > > +Note: external tool `python3-coverage` is used by tests. The environment
> > > > +variable `COVERAGE` can be set to alternative name or location of this tool.
> > > > +
> > > >    Some tests take ages to run and are marked with @pytest.mark.slow. To run just
> > > >    the quick ones, type this::
> > > > diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
> > > > index 0f6d1aa902..e11806b626 100644
> > > > --- a/tools/patman/test_util.py
> > > > +++ b/tools/patman/test_util.py
> > > > @@ -15,6 +15,8 @@ from patman import command
> > > >    from io import StringIO
> > > > +coverage = os.environ.get('COVERAGE', 'python3-coverage')
> > > > +
> > > >    buffer_outputs = True
> > > >    use_concurrent = True
> > > >    try:
> > > > @@ -58,11 +60,11 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
> > > >        prefix = ''
> > > >        if build_dir:
> > > >            prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
> > > > -    cmd = ('%spython3-coverage run '
> > > > -           '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
> > > > +    cmd = ('%s run '
> > > > +           '--omit "%s" %s %s %s -P1' % (coverage, ','.join(glob_list),
> > > >                                             prog, extra_args or '', test_cmd))
> > > 
> > > What about using
> > > python3 -m coverage run
> > > instead?
> > > This way we wouldn't rely on the binary name the host distribution chooses
> > > (python3-coverage for Ubuntu, coverage for Fedora).
> > > 
> > > I'm not sure there is a need to give the user the ability to override this
> > > value since I expect only coverage.py is supported at the moment?
> > 
> > Then you run into the problems that you do not have coverage on
> > python3.4 but only python3.6 or whatever is the relevant version for
> > your distribution ATM.
> > 
> 
> I don't understand this point? If coverage cannot run with the python from
> the host, then it won't just be possible to run it... so... we don't care?
> Is there something we can do about this I'm missing?

Host can have multiple python versions with varying tools installed.

> 
> I would like to advocate for COVERAGE to default to "coverage" or "python3
> -m coverage" since this is more likely to be a sane default for everybody,
> using pip or distro installs.

"coverage" will likely break current test environment but "python3 -m
coverage" may just work, and also test that expanding the arguments
works correctly.

Thanks

Michal