[elbe-devel] [PATCH 5/5] tests: test_pylint: Parametize pylint

[Bastian Germann]

Typo in the subject: Parametrize (missing r)

Am 23.07.20 um 04:51 schrieb Olivier Dion:
> Also skip some file listed in 'failure_set'.  These are the remaining

file -> files

> lint errors.  Hopefully, we can make this set empty and remove it.
> 
> File that are listed in the set are simply skip with reason that we're

File -> Files

skip with reason -> skipped with the reason

> expecting them to fail.
> 
> It would be nice to be able to use 'unittest.expectedFailure', but
> this function doesn't work on a 'TestCase' object.  Indeed, it marks
> the test method to be expected to fail.  And since this is done before
> the parametization of the test case object, there's no possible way to

parametization -> parametrization

> use this decorator at our advantage.  Thus, using 'self.skip' is the
> closest thing that mimic what we want.

mimic -> mimics

> 
> Signed-off-by: Olivier Dion <dion at linutronix.de>
> ---
>  elbepack/tests/test_pylint.py | 70 +++++++++++++++++++++++++++--------
>  1 file changed, 54 insertions(+), 16 deletions(-)
> 
> diff --git a/elbepack/tests/test_pylint.py b/elbepack/tests/test_pylint.py
> index 80475e38..f45aaade 100644
> --- a/elbepack/tests/test_pylint.py
> +++ b/elbepack/tests/test_pylint.py
> @@ -4,26 +4,64 @@
>  # SPDX-License-Identifier: GPL-3.0-or-later
>  
>  import os
> -import unittest
>  
> -from elbepack.shellhelper import system, CommandError
> +from elbepack.commands.test import ElbeTestCase
> +from elbepack.shellhelper import command_out, system_out
> +from elbepack.directories import elbe_dir
>  
> -class TestPylint(unittest.TestCase):
> +class TestPylint(ElbeTestCase):
>  
> -    this_dir = os.path.dirname(os.path.realpath(__file__))
> -    top_dir  = os.path.abspath(os.path.join(this_dir, "..", ".."))
>      pylint_opts = ["--reports=n",
>                     "--score=n",
> -                   "--rcfile=%s" % os.path.join(top_dir, ".pylintrc"),
> -                   "--disable=W0511,R0801",
> -                   "elbe", "elbepack"]
> +                   "--rcfile=%s" % os.path.join(elbe_dir, ".pylintrc"),
> +                   "--disable=W0511,R0801"]
> +
> +    failure_set = {os.path.join(elbe_dir, path)
> +                   for path
> +                   in [
> +                       "docs/conf.py",
> +                       "elbepack/daemons/soap/esoap.py",
> +
> +                       # These are not need to be fix since debianize

need -> needed

fix -> fixed

> +                       # is going to be rewritten
> +                       "elbepack/debianize/base/tui.py",
> +                       "elbepack/debianize/panels/base.py",
> +                       "elbepack/debianize/panels/kernel.py",
> +                       "elbepack/debianize/widgets/button.py",
> +                       "elbepack/debianize/widgets/edit.py",
> +                       "elbepack/debianize/widgets/form.py",
> +                       "elbepack/debianize/widgets/grid.py",
> +                       "elbepack/debianize/widgets/radio.py",
> +
> +                       "elbepack/elbeproject.py",
> +                       "elbepack/elbexml.py",
> +
> +                       # This one is an actual bug to be fix

fix -> fixed

Please prepend the comment with "FIXME: "

> +                       # 274:30: W0631: Using possibly undefined loop variable 'entry' (undefined-loop-variable)
> +                       # 276:26: W0631: Using possibly undefined loop variable 'entry' (undefined-loop-variable)
> +                       "elbepack/hdimg.py",
> +
> +                       "elbepack/initvmaction.py",
> +                       "elbepack/log.py",
> +                       "elbepack/pbuilderaction.py",
> +                       "elbepack/repomanager.py",
> +                       "elbepack/rfs.py",
> +                       "elbepack/rpcaptcache.py",
> +                       "test/updated.py",
> +                   ]}
> +
> +    @staticmethod
> +    def params():
> +        files = system_out("find %s -iname '*.py'" % elbe_dir).splitlines()
> +        files.append("elbe")
> +        return files
>  
>      def test_lint(self):
> -        ret = True
> -        try:
> -            system("pylint3 %s" % " ".join(self.pylint_opts))
> -        except CommandError as E:
> -            print(E)
> -            ret = False
> -
> -        self.assertTrue(ret)
> +
> +        ret, out = command_out("pylint3 %s %s" % (' '.join(self.pylint_opts), self.param))
> +
> +        if ret:
> +            if self.param in TestPylint.failure_set:
> +                self.skipTest("Pylint test for %s is expected to fail\n%s" % (self.param, out))
> +            else:
> +                self.fail(msg=out)

With all the suggested changes (incl. subject):
Reviewed-by: Bastian Germann <bage at linutronix.de>