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

[Heinrich Schuchardt]

On 4/20/23 00:40, Simon Glass wrote:
> Hi Heinrich,
> 
> On Wed, 19 Apr 2023 at 18:30, Heinrich Schuchardt
> <heinrich.schuchardt at canonical.com> wrote:
>>
>>
>>
>> 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/).
> 
> That doesn't really tell you much though. It is just the idea of a
> docstring from 20 years ago. We need a convention for what is in the
> strings, not just the strings.
> 
> The one we currently use is at least based on [1] or something like
> it. I would not describe it as internal to Google. What makes you
> think that? It is widely used.
> 
> For me at least, pylint asks for type information:
> 
> tools/binman/control.py:146:0: W9016: "modules, test_missing" missing
> in parameter type documentation (missing-type-doc)

You should add type hints (PEP484) and use snake case (PEP8):

-def WriteEntryDocs(modules, test_missing=None):
+def write_entry_docs(modules: List[str], test_missing: str=None):

-def ListEntries(image_fname, entry_paths):
+def list_entries(image_fname: str, entry_paths: List[str]):

Best regards

Heinrich

> 
> You should be able to try that and see what you get.
> 
> Regards,
> Simon
> 
> [1] https://sphinxcontrib-napoleon.readthedocs.io/en/latest/index.html