Re: [PATCH v7 09/11] iotests: add testrunner.py

[Vladimir Sementsov-Ogievskiy]

22.01.2021 17:11, Kevin Wolf wrote:
> Am 16.01.2021 um 14:44 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > Add TestRunner class, which will run tests in a new python iotests
> > running framework.
> >
> > There are some differences with current ./check behavior, most
> > significant are:
> > - Consider all tests self-executable, just run them, don't run python
> >    by hand.
> > - Elapsed time is cached in json file
> > - Elapsed time precision increased a bit
> > - use python difflib instead of "diff -w", to ignore spaces at line
> >    ends strip lines by hand. Do not ignore other spaces.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > ---
> >   tests/qemu-iotests/testrunner.py | 344 +++++++++++++++++++++++++++++++
> >   1 file changed, 344 insertions(+)
> >   create mode 100644 tests/qemu-iotests/testrunner.py
>
> > +TestResult = collections.namedtuple(
> > +    'TestResult',
> > +    ['status', 'description', 'elapsed', 'diff', 'casenotrun'],
> > +    defaults=('', '', '', ''))
>
> defaults was only introduced in Python 3.7, it seems.

hmm, yes

> Why not use a normal class
OK

> > +
> > +class TestRunner(AbstractContextManager['TestRunner']):
> > +    def __init__(self, env: TestEnv, makecheck: bool = False) -> None:
> > +        self.env = env
> > +        self.test_run_env = self.env.get_env()
> > +        if 'MALLOC_PERTURB_' not in os.environ and \
> > +                'MALLOC_PERTURB_' not in self.test_run_env:
>
> 'MALLOC_PERTURB_' is not in TestEnv.env_variables, so it will never be
> in self.test_run_env here.

Hmm, right..

> > +            x = random.randrange(1, 255)
> > +            self.test_run_env['MALLOC_PERTURB_'] = str(x)

I don't remember why this is in TestRunner and not in TestEnv. May be better to 
move this logic to TestEnv.

> > +
> > +        self.makecheck = makecheck
> > +
> > +        self.last_elapsed = LastElapsedTime('.last-elapsed-cache', env)
> > +
> > +    def __enter__(self) -> 'TestRunner':
> > +        # pylint: disable=attribute-defined-outside-init
>
> You can avoid this by declaring the attribute in __init__ without
> initialising it yet:
>
>      self._stack: contextlib.ExitStack

OK

> > +        self._stack = contextlib.ExitStack()
> > +        self._stack.enter_context(self.env)
> > +        self._stack.enter_context(self.last_elapsed)
> > +        self._stack.enter_context(savetty())
> > +        return self
>
> Kevin


--
Best regards,
Vladimir