Re: [Qemu-devel] [PATCH 1/2] iotests: add script_initialize

[Eduardo Habkost]

On Fri, Jul 26, 2019 at 06:52:00PM -0400, John Snow wrote:
> Like script_main, but doesn't require a single point of entry.
> Replace all existing initialization sections with this drop-in replacement.
> 
> This brings debug support to all existing script-style iotests.
> 
> Note: supported_oses=['linux'] was omitted, as it is a default argument.
> Signed-off-by: John Snow <address@hidden>
[...]

Looks good overall, I just have one comment:


> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 727730422f..5e9b2989dd 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -891,9 +891,8 @@ def execute_unittest(output, verbosity, debug):
>              sys.stderr.write(re.sub(r'Ran (\d+) tests? in [\d.]+s',
>                                      r'Ran \1 tests', output.getvalue()))
>  
> -def execute_test(test_function=None,
> -                 supported_fmts=[], supported_oses=['linux'],
> -                 supported_cache_modes=[], unsupported_fmts=[]):
> +def execute_setup_common(supported_fmts=[], supported_oses=['linux'],
> +                         supported_cache_modes=[], unsupported_fmts=[]):
>      """Run either unittest or script-style tests."""
>  
>      # We are using TEST_DIR and QEMU_DEFAULT_MACHINE as proxies to
> @@ -925,16 +924,28 @@ def execute_test(test_function=None,
>              output = io.BytesIO()
>  
>      logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN))
> +    return output, verbosity, debug

Can't we make this simpler?

What about just returning `debug`, and letting execute_unittest()
take care of `verbosity` and `output`?

>  
> +def execute_test(test_function=None, *args, **kwargs):
> +    """Run either unittest or script-style tests."""
> +
> +    unittest_args = execute_setup_common(*args, **kwargs)
>      if not test_function:
> -        execute_unittest(output, verbosity, debug)
> +        execute_unittest(*unittest_args)
>      else:
>          test_function()
>  
> +# This is called from script-style iotests without a single point of entry
> +def script_initialize(*args, **kwargs):
> +    """Initialize script-style tests without running any tests."""
> +    execute_setup_common(*args, **kwargs)
> +
> +# This is called from script-style iotests with a single point of entry
>  def script_main(test_function, *args, **kwargs):
>      """Run script-style tests outside of the unittest framework"""
>      execute_test(test_function, *args, **kwargs)
>  
> +# This is called from unittest style iotests
>  def main(*args, **kwargs):
>      """Run tests using the unittest framework"""
>      execute_test(None, *args, **kwargs)
> -- 
> 2.21.0
> 

-- 
Eduardo