Re: [PATCH 1/2] test/py: Use loop mounts if guestmount fails in filesystem tests

[Alper Nebi Yasak <alpernebiyasak@gmail.com>]

On 26/06/2021 21:29, Simon Glass wrote:
> On Fri, 4 Jun 2021 at 13:05, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
>>
>> If guestmount isn't available on the system, filesystem test setup falls
>> back to using loop mounts to prepare its disk images. If guestmount is
>> available but fails to work, the tests are immediately skipped. Instead
>> of giving up on a guestmount failure, try using loop mounts as an
>> attempt to keep tests running.
>>
>> Also stop checking if guestmount is in PATH, as trying to run a missing
>> guestmount can now follow the same failure codepath and fall back to
>> loop mounts anyway.
>>
>> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
>> ---
>>
>>  test/py/tests/test_fs/conftest.py | 31 +++++++++++++++----------------
>>  1 file changed, 15 insertions(+), 16 deletions(-)
> 
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/282788
> 
> thoughts below

That CI run doesn't have this patch/series applied.

>>
>> diff --git a/test/py/tests/test_fs/conftest.py b/test/py/tests/test_fs/conftest.py
>> index 410a675b9714..e3c461635f8e 100644
>> --- a/test/py/tests/test_fs/conftest.py
>> +++ b/test/py/tests/test_fs/conftest.py
>> @@ -205,24 +205,23 @@ def mount_fs(fs_type, device, mount_point):
>>      """
>>      global fuse_mounted
>>
>> -    fuse_mounted = False
>>      try:
>> -        if tool_is_in_path('guestmount'):
>> -            fuse_mounted = True
>> -            check_call('guestmount -a %s -m /dev/sda %s'
>> -                % (device, mount_point), shell=True)
>> -        else:
>> -            mount_opt = 'loop,rw'
>> -            if re.match('fat', fs_type):
>> -                mount_opt += ',umask=0000'
>> -
>> -            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('guestmount -a %s -m /dev/sda %s'
>> +            % (device, mount_point), shell=True)
>> +        fuse_mounted = True
>> +        return
>>      except CalledProcessError:
>> -        raise
>> +        fuse_mounted = False
>> +
>> +    mount_opt = 'loop,rw'
>> +    if re.match('fat', fs_type):
> 
> This seems ambiguous. I think you should use:
> 
>    if 'fat' in fs_type:
> 
> or
> 
>    if fs_type.startswith('fat'):
> 
> or
> 
>   if fs_type == 'fat':
> 
> so it is clear what you mean.

This just unindents existing code. I can send v2 with startswith('fat')
though.

(I think I should rewrite the tests with guestfish and remove these
mount functions instead; but I'm busy with other things for a while.)

>> +        mount_opt += ',umask=0000'
>> +
>> +    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)
>>
>>  def umount_fs(mount_point):
>>      """Unmount a volume.
>> --
>> 2.32.0.rc2
>>
> 
> Regards,
> Simon
>