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

[Simon Glass]

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 be able to try that and see what you get.

Regards,
Simon

[1] https://sphinxcontrib-napoleon.readthedocs.io/en/latest/index.html