[PATCH 2/2] test/py: fix test_efi_secboot/conftest.py

[Heinrich Schuchardt]

On 23.04.20 02:15, AKASHI Takahiro wrote:
> Heinrich,
>
> On Wed, Apr 22, 2020 at 05:52:55PM +0200, Heinrich Schuchardt wrote:
>> If udisksctl is present
>> test/py/tests/test_efi_secboot/conftest.py
>> fails because the disk image is never mounted.
>>
>> Normal users can only mount fuse file systems. Unfortunately fusefat is
>> still in an experimental state and seems not to work here correctly.
>
> I haven't confirmed that fuse/fat is not stable, but
>
>> So as we have to be root or use the sudo command anyway delete all coding
>> referring to udisksctl.
>
> I don't mind non-root path being deleted as it was used when Travis CI
> didn't work with "sudo".
>
>> --
>>
>> We should not use mount point /mnt as this directory or one of its
>> sub-directories might already be in use as active mount points. Instead
>> create a new directory in the build root as mount point.
>>
>> --
>>
>> Remove debug print statements that have been commented out. print without
>> parentheses is anyway invalid in Python 3. And pytest anyway filters out
>> the output if there is no exception reported.
>
> 'printing a mount point' was mostly useful for debugging
> non-root path.

The code in your comments was invalid: We are using Python 3. In Python
3 print() is a function.

>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>>  test/py/tests/test_efi_secboot/conftest.py | 30 +++++-----------------
>>  1 file changed, 6 insertions(+), 24 deletions(-)
>>
>> diff --git a/test/py/tests/test_efi_secboot/conftest.py b/test/py/tests/test_efi_secboot/conftest.py
>> index 40cdf15bf2..93d308cf0d 100644
>> --- a/test/py/tests/test_efi_secboot/conftest.py
>> +++ b/test/py/tests/test_efi_secboot/conftest.py
>> @@ -43,7 +43,8 @@ def efi_boot_env(request, u_boot_config):
>>          HELLO_PATH = u_boot_config.build_dir + '/lib/efi_loader/helloworld.efi'
>>
>>      try:
>> -        non_root = tool_is_in_path('udisksctl')
>> +        mnt_point = u_boot_config.persistent_data_dir + '/mnt_efisecure'
>> +        check_call('mkdir -p {}'.format(mnt_point), shell=True)
>
> For consistency, it would be better to use "%" formatting as elsewhere
> in the file.

https://docs.python.org/release/3.0/library/stdtypes.html#old-string-formatting-operations

explicitly says:

"The formatting operations described here are obsolete and may go away
in future versions of Python. Use the new String Formatting in new code."

As using % was deprecated 12 years ago it would be great if you could
rework your code to use current Python 3.

Best regards

Heinrich

>
> Thanks,
> -Takahiro Akashi
>
>>
>>          # create a disk/partition
>>          check_call('dd if=/dev/zero of=%s bs=1MiB count=%d'
>> @@ -57,25 +58,11 @@ def efi_boot_env(request, u_boot_config):
>>          check_call('dd if=%s.tmp of=%s bs=1MiB seek=1 count=%d conv=notrunc'
>>                              % (image_path, image_path, 1), shell=True)
>>          check_call('rm %s.tmp' % image_path, shell=True)
>> -        if non_root:
>> -            out_data = check_output('udisksctl loop-setup -f %s -o %d'
>> -                                % (image_path, 1048576), shell=True).decode()
>> -            m = re.search('(?<= as )(.*)\.', out_data)
>> -            loop_dev = m.group(1)
>> -            # print 'loop device is: %s' % loop_dev
>> -            out_data = check_output('udisksctl info -b %s'
>> -                                % loop_dev, shell=True).decode()
>> -            m = re.search('MountPoints:[ \t]+(.*)', out_data)
>> -            mnt_point = m.group(1)
>> -        else:
>> -            loop_dev = check_output('sudo losetup -o 1MiB --sizelimit %dMiB --show -f %s | tr -d "\n"'
>> +        loop_dev = check_output('sudo losetup -o 1MiB --sizelimit %dMiB --show -f %s | tr -d "\n"'
>>                                  % (part_size, image_path), shell=True).decode()
>> -            mnt_point = '/mnt'
>> -            check_output('sudo mount -t %s -o umask=000 %s %s'
>> +        check_output('sudo mount -t %s -o umask=000 %s %s'
>>                                  % (fs_type, loop_dev, mnt_point), shell=True)
>>
>> -        # print 'mount point is: %s' % mnt_point
>> -
>>          # suffix
>>          # *.key: RSA private key in PEM
>>          # *.crt: X509 certificate (self-signed) in PEM
>> @@ -134,13 +121,8 @@ def efi_boot_env(request, u_boot_config):
>>                              % (mnt_point, EFITOOLS_PATH, EFITOOLS_PATH),
>>                              shell=True)
>>
>> -        if non_root:
>> -            check_call('udisksctl unmount -b %s' % loop_dev, shell=True)
>> -            # not needed
>> -            # check_call('udisksctl loop-delete -b %s' % loop_dev, shell=True)
>> -        else:
>> -            check_call('sudo umount %s' % loop_dev, shell=True)
>> -            check_call('sudo losetup -d %s' % loop_dev, shell=True)
>> +        check_call('sudo umount %s' % loop_dev, shell=True)
>> +        check_call('sudo losetup -d %s' % loop_dev, shell=True)
>>
>>      except CalledProcessError as e:
>>          pytest.skip('Setup failed: %s' % e.cmd)
>> --
>> 2.26.1
>>