[PATCH 1/1] test: fix pylint warning for capsule tests

[Heinrich Schuchardt]

On 4/19/23 03:45, Simon Glass wrote:
> Hi Heinrich,
> 
> On Fri, 14 Apr 2023 at 02:34, Heinrich Schuchardt
> <heinrich.schuchardt at canonical.com> wrote:
>>
>> Fix pylint warnings like:
>>
>> * Class inherits from object
>> * Missing module description
>> * Missing class description
>> * First line of comment blank
>> * Superfluous imports
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>> ---
>>   test/py/tests/test_efi_capsule/conftest.py    | 27 ++++--------
>>   .../test_capsule_firmware_fit.py              | 35 ++++++++--------
>>   .../test_capsule_firmware_signed_fit.py       | 41 ++++++++++---------
>>   .../test_capsule_firmware_signed_raw.py       | 38 ++++++++---------
>>   4 files changed, 65 insertions(+), 76 deletions(-)
>>
>> diff --git a/test/py/tests/test_efi_capsule/conftest.py b/test/py/tests/test_efi_capsule/conftest.py
>> index 4879f2b5c2..0e5137de60 100644
>> --- a/test/py/tests/test_efi_capsule/conftest.py
>> +++ b/test/py/tests/test_efi_capsule/conftest.py
>> @@ -2,30 +2,21 @@
>>   # Copyright (c) 2020, Linaro Limited
>>   # Author: AKASHI Takahiro <takahiro.akashi at linaro.org>
>>
>> -import os
>> -import os.path
>> -import re
>> -from subprocess import call, check_call, check_output, CalledProcessError
>> -import pytest
>> -from capsule_defs import *
>> +"""Fixture for UEFI capsule test
>> +"""
>>
>> -#
>> -# Fixture for UEFI capsule test
>> -#
>> +from subprocess import call, check_call, CalledProcessError
>> +import pytest
>> +from capsule_defs import CAPSULE_DATA_DIR, CAPSULE_INSTALL_DIR, EFITOOLS_PATH
>>
>>   @pytest.fixture(scope='session')
>>   def efi_capsule_data(request, u_boot_config):
>> -    """Set up a file system to be used in UEFI capsule and
>> -       authentication test.
>> -
>> -    Args:
>> -        request: Pytest request object.
>> -        u_boot_config: U-boot configuration.
>> +    """Set up a file system to be used in UEFI capsule and authentication test
>> +    and return a ath to disk image to be used for testing
> 
> Thanks for cleaning this up. I suppose with all the rounds of review
> we got tired of worrying about the style. Also this probably predates
> the pylint check.
> 
> Can we please follow the style in the rest of the code? This should
> have a heading line, with further notes after a blank line.
> 
>>
>> -    Return:
>> -        A path to disk image to be used for testing
>> +    request -- Pytest request object.
>> +    u_boot_config -- U-boot configuration.
> 
> Again the style is:
> 
> Return:
>     request (pytest.Request): Request to be processed
>     u_boot_config (type): U-Boot configuration

This seems to be some Google internal code style. We should stick to 
PEP257 (https://peps.python.org/pep-0257/).

Best regards

Heinrich

> 
> (so avoid periods at the end and add the type in brackets)
> 
> The u_boot_config thing is pretty annoying since it creates a
> strangely named class - see confest.py where is has:
> 
>      ubconfig = ArbitraryAttributeContainer()
> 
> Really that should be a class with a sensible name and documented
> properties. The internals of this are a little too arcane for my
> liking and discoverability is not great.
> 
> [..]
> 
> Also, this should be added to the checker - try 'make pylint' to see that.
> 
> Regards,
> Simon