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

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

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