| Message ID | 20190620010356.19164-9-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | bitmaps: introduce 'bitmap' sync mode | expand |
On 20.06.19 03:03, John Snow wrote: > Because the new-style python tests don't use the iotests.main() test > launcher, we don't turn on the debugger logging for these scripts > when invoked via ./check -d. > > Refactor the launcher shim into new and old style shims so that they > share environmental configuration. > > Two cleanup notes: debug was not actually used as a global, and there > was no reason to create a class in an inner scope just to achieve > default variables; we can simply create an instance of the runner with > the values we want instead. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > tests/qemu-iotests/iotests.py | 40 +++++++++++++++++++++++------------ > 1 file changed, 26 insertions(+), 14 deletions(-) I don’t quite get how script_main() works (yes, both my Pythonfu and my Googlefu are that bad), but it works and looks good, so have a Reviewed-by: Max Reitz <mreitz@redhat.com>
On 20.06.19 19:09, Max Reitz wrote: > On 20.06.19 03:03, John Snow wrote: >> Because the new-style python tests don't use the iotests.main() test >> launcher, we don't turn on the debugger logging for these scripts >> when invoked via ./check -d. >> >> Refactor the launcher shim into new and old style shims so that they >> share environmental configuration. >> >> Two cleanup notes: debug was not actually used as a global, and there >> was no reason to create a class in an inner scope just to achieve >> default variables; we can simply create an instance of the runner with >> the values we want instead. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> tests/qemu-iotests/iotests.py | 40 +++++++++++++++++++++++------------ >> 1 file changed, 26 insertions(+), 14 deletions(-) > > I don’t quite get how script_main() works (yes, both my Pythonfu and my > Googlefu are that bad), but it works and looks good, so have a Oh, it doesn’t work (well, not automagically). I just assumed seeing the log output means it’s working. Seeing that the test needs to call iotests.script_main() explicitly does clear up my confusion. All OK with me. Max
On 6/20/19 1:26 PM, Max Reitz wrote: > On 20.06.19 19:09, Max Reitz wrote: >> On 20.06.19 03:03, John Snow wrote: >>> Because the new-style python tests don't use the iotests.main() test >>> launcher, we don't turn on the debugger logging for these scripts >>> when invoked via ./check -d. >>> >>> Refactor the launcher shim into new and old style shims so that they >>> share environmental configuration. >>> >>> Two cleanup notes: debug was not actually used as a global, and there >>> was no reason to create a class in an inner scope just to achieve >>> default variables; we can simply create an instance of the runner with >>> the values we want instead. >>> >>> Signed-off-by: John Snow <jsnow@redhat.com> >>> --- >>> tests/qemu-iotests/iotests.py | 40 +++++++++++++++++++++++------------ >>> 1 file changed, 26 insertions(+), 14 deletions(-) >> >> I don’t quite get how script_main() works (yes, both my Pythonfu and my >> Googlefu are that bad), but it works and looks good, so have a > > Oh, it doesn’t work (well, not automagically). I just assumed seeing > the log output means it’s working. Seeing that the test needs to call > iotests.script_main() explicitly does clear up my confusion. > > All OK with me. > > Max > Yes. I should convert the others to opt-in to the new format so that copy-paste in the future will get us the right paradigm. Tests just need to be refactored to have a single point of entry so it can be passed as a closure to the test runner. If this seems like a good change I will do that as a follow-up series with only the churn. --js
On 20.06.19 20:47, John Snow wrote: > > > On 6/20/19 1:26 PM, Max Reitz wrote: >> On 20.06.19 19:09, Max Reitz wrote: >>> On 20.06.19 03:03, John Snow wrote: >>>> Because the new-style python tests don't use the iotests.main() test >>>> launcher, we don't turn on the debugger logging for these scripts >>>> when invoked via ./check -d. >>>> >>>> Refactor the launcher shim into new and old style shims so that they >>>> share environmental configuration. >>>> >>>> Two cleanup notes: debug was not actually used as a global, and there >>>> was no reason to create a class in an inner scope just to achieve >>>> default variables; we can simply create an instance of the runner with >>>> the values we want instead. >>>> >>>> Signed-off-by: John Snow <jsnow@redhat.com> >>>> --- >>>> tests/qemu-iotests/iotests.py | 40 +++++++++++++++++++++++------------ >>>> 1 file changed, 26 insertions(+), 14 deletions(-) >>> >>> I don’t quite get how script_main() works (yes, both my Pythonfu and my >>> Googlefu are that bad), but it works and looks good, so have a >> >> Oh, it doesn’t work (well, not automagically). I just assumed seeing >> the log output means it’s working. Seeing that the test needs to call >> iotests.script_main() explicitly does clear up my confusion. >> >> All OK with me. >> >> Max >> > > Yes. I should convert the others to opt-in to the new format so that > copy-paste in the future will get us the right paradigm. > > Tests just need to be refactored to have a single point of entry so it > can be passed as a closure to the test runner. > > If this seems like a good change I will do that as a follow-up series > with only the churn. It does seem good to me. Not even because of the test runner, but maybe even more so because it seems like better style to split the tests into one function per case. Max
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 3ecef5bc90..fcad957d63 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -61,7 +61,6 @@ cachemode = os.environ.get('CACHEMODE') qemu_default_machine = os.environ.get('QEMU_DEFAULT_MACHINE') socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper') -debug = False luks_default_secret_object = 'secret,id=keysec0,data=' + \ os.environ.get('IMGKEYSECRET', '') @@ -834,11 +833,22 @@ def skip_if_unsupported(required_formats=[], read_only=False): return func_wrapper return skip_test_decorator -def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[], - unsupported_fmts=[]): - '''Run tests''' +def execute_unittest(output, verbosity, debug): + runner = unittest.TextTestRunner(stream=output, descriptions=True, + verbosity=verbosity) + try: + # unittest.main() will use sys.exit(); so expect a SystemExit + # exception + unittest.main(testRunner=runner) + finally: + if not debug: + sys.stderr.write(re.sub(r'Ran (\d+) tests? in [\d.]+s', + r'Ran \1 tests', output.getvalue())) - global debug +def execute_test(test_function=None, + 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 # indicate that we're not being run via "check". There may be @@ -870,13 +880,15 @@ def main(supported_fmts=[], supported_oses=['linux'], supported_cache_modes=[], logging.basicConfig(level=(logging.DEBUG if debug else logging.WARN)) - class MyTestRunner(unittest.TextTestRunner): - def __init__(self, stream=output, descriptions=True, verbosity=verbosity): - unittest.TextTestRunner.__init__(self, stream, descriptions, verbosity) + if not test_function: + execute_unittest(output, verbosity, debug) + else: + test_function() - # unittest.main() will use sys.exit() so expect a SystemExit exception - try: - unittest.main(testRunner=MyTestRunner) - finally: - if not debug: - sys.stderr.write(re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', output.getvalue())) +def script_main(test_function, *args, **kwargs): + """Run script-style tests outside of the unittest framework""" + execute_test(test_function, *args, **kwargs) + +def main(*args, **kwargs): + """Run tests using the unittest framework""" + execute_test(None, *args, **kwargs)
Because the new-style python tests don't use the iotests.main() test launcher, we don't turn on the debugger logging for these scripts when invoked via ./check -d. Refactor the launcher shim into new and old style shims so that they share environmental configuration. Two cleanup notes: debug was not actually used as a global, and there was no reason to create a class in an inner scope just to achieve default variables; we can simply create an instance of the runner with the values we want instead. Signed-off-by: John Snow <jsnow@redhat.com> --- tests/qemu-iotests/iotests.py | 40 +++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 14 deletions(-)