| Message ID | 20211210201450.101576-1-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iotests/testrunner.py: refactor test_field_width | expand |
On Fri, Dec 10, 2021 at 3:15 PM Vladimir Sementsov-Ogievskiy < vsementsov@virtuozzo.com> wrote: > A lot of Optional[] types doesn't make code beautiful. > test_field_width defaults to 8, but that is never used in the code. > > More over, if we want some default behavior for single call of > test_run(), it should just print the whole test name, not limiting or > expanding its width, so 8 is bad default. > > So, just drop the default as unused for now. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > > This is a follow-up for "[PATCH 0/3] iotests: multiprocessing!!" and > based on Hanna's block-next branch. > > Based-on: <20211203122223.2780098-1-vsementsov@virtuozzo.com> > > tests/qemu-iotests/testrunner.py | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/tests/qemu-iotests/testrunner.py > b/tests/qemu-iotests/testrunner.py > index 0feaa396d0..15788f919e 100644 > --- a/tests/qemu-iotests/testrunner.py > +++ b/tests/qemu-iotests/testrunner.py > @@ -174,19 +174,17 @@ def __enter__(self) -> 'TestRunner': > def __exit__(self, exc_type: Any, exc_value: Any, traceback: Any) -> > None: > self._stack.close() > > - def test_print_one_line(self, test: str, starttime: str, > + def test_print_one_line(self, test: str, > + test_field_width: int, > + starttime: str, > endtime: Optional[str] = None, status: str = > '...', > lasttime: Optional[float] = None, > thistime: Optional[float] = None, > description: str = '', > - test_field_width: Optional[int] = None, > end: str = '\n') -> None: > """ Print short test info before/after test run """ > test = os.path.basename(test) > > - if test_field_width is None: > - test_field_width = 8 > - > if self.makecheck and status != '...': > if status and status != 'pass': > status = f' [{status}]' > @@ -328,7 +326,7 @@ def do_run_test(self, test: str, mp: bool) -> > TestResult: > casenotrun=casenotrun) > > def run_test(self, test: str, > - test_field_width: Optional[int] = None, > + test_field_width: int, > mp: bool = False) -> TestResult: > """ > Run one test and print short status > @@ -347,20 +345,21 @@ def run_test(self, test: str, > > if not self.makecheck: > self.test_print_one_line(test=test, > + test_field_width=test_field_width, > status = 'started' if mp else '...', > starttime=start, > lasttime=last_el, > - end = '\n' if mp else '\r', > - test_field_width=test_field_width) > + end = '\n' if mp else '\r') > > res = self.do_run_test(test, mp) > > end = datetime.datetime.now().strftime('%H:%M:%S') > - self.test_print_one_line(test=test, status=res.status, > + self.test_print_one_line(test=test, > + test_field_width=test_field_width, > + status=res.status, > starttime=start, endtime=end, > lasttime=last_el, thistime=res.elapsed, > - description=res.description, > - test_field_width=test_field_width) > + description=res.description) > > if res.casenotrun: > print(res.casenotrun) > -- > 2.31.1 > > "LGTM" Reviewed-by: John Snow <jsnow@redhat.com>
Am 10.12.2021 um 21:14 hat Vladimir Sementsov-Ogievskiy geschrieben: > A lot of Optional[] types doesn't make code beautiful. > test_field_width defaults to 8, but that is never used in the code. > > More over, if we want some default behavior for single call of > test_run(), it should just print the whole test name, not limiting or > expanding its width, so 8 is bad default. > > So, just drop the default as unused for now. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Thanks, applied to the block branch. Kevin
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py index 0feaa396d0..15788f919e 100644 --- a/tests/qemu-iotests/testrunner.py +++ b/tests/qemu-iotests/testrunner.py @@ -174,19 +174,17 @@ def __enter__(self) -> 'TestRunner': def __exit__(self, exc_type: Any, exc_value: Any, traceback: Any) -> None: self._stack.close() - def test_print_one_line(self, test: str, starttime: str, + def test_print_one_line(self, test: str, + test_field_width: int, + starttime: str, endtime: Optional[str] = None, status: str = '...', lasttime: Optional[float] = None, thistime: Optional[float] = None, description: str = '', - test_field_width: Optional[int] = None, end: str = '\n') -> None: """ Print short test info before/after test run """ test = os.path.basename(test) - if test_field_width is None: - test_field_width = 8 - if self.makecheck and status != '...': if status and status != 'pass': status = f' [{status}]' @@ -328,7 +326,7 @@ def do_run_test(self, test: str, mp: bool) -> TestResult: casenotrun=casenotrun) def run_test(self, test: str, - test_field_width: Optional[int] = None, + test_field_width: int, mp: bool = False) -> TestResult: """ Run one test and print short status @@ -347,20 +345,21 @@ def run_test(self, test: str, if not self.makecheck: self.test_print_one_line(test=test, + test_field_width=test_field_width, status = 'started' if mp else '...', starttime=start, lasttime=last_el, - end = '\n' if mp else '\r', - test_field_width=test_field_width) + end = '\n' if mp else '\r') res = self.do_run_test(test, mp) end = datetime.datetime.now().strftime('%H:%M:%S') - self.test_print_one_line(test=test, status=res.status, + self.test_print_one_line(test=test, + test_field_width=test_field_width, + status=res.status, starttime=start, endtime=end, lasttime=last_el, thistime=res.elapsed, - description=res.description, - test_field_width=test_field_width) + description=res.description) if res.casenotrun: print(res.casenotrun)
A lot of Optional[] types doesn't make code beautiful. test_field_width defaults to 8, but that is never used in the code. More over, if we want some default behavior for single call of test_run(), it should just print the whole test name, not limiting or expanding its width, so 8 is bad default. So, just drop the default as unused for now. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- This is a follow-up for "[PATCH 0/3] iotests: multiprocessing!!" and based on Hanna's block-next branch. Based-on: <20211203122223.2780098-1-vsementsov@virtuozzo.com> tests/qemu-iotests/testrunner.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)