diff mbox series

iotests/testrunner.py: refactor test_field_width

Message ID 20211210201450.101576-1-vsementsov@virtuozzo.com
State New
Headers show
Series iotests/testrunner.py: refactor test_field_width | expand

Commit Message

Vladimir Sementsov-Ogievskiy Dec. 10, 2021, 8:14 p.m. UTC 
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(-)

Comments

John Snow Dec. 17, 2021, 12:02 a.m. UTC | #1 
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>
Kevin Wolf Jan. 13, 2022, 5:11 p.m. UTC | #2 
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 mbox series

Patch

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)