All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-devel@nongnu.org
Cc: eesposit@redhat.com, kwolf@redhat.com, qemu-block@nongnu.org,
	mreitz@redhat.com
Subject: Re: [PATCH 1/4] qemu-iotests: allow passing unittest.main arguments to the test scripts
Date: Tue, 23 Mar 2021 18:04:39 +0100	[thread overview]
Message-ID: <04724b6a-3be2-54ff-c769-35fed34e9135@redhat.com> (raw)
In-Reply-To: <a51c2d8d-cc9f-705f-2527-857a791e54de@virtuozzo.com>

On 23/03/21 17:56, Vladimir Sementsov-Ogievskiy wrote:
> hmm, just use a separate call of unittest.main, or honestly define a 
> varaible as Union[] or just Any ?

All the ugliness goes away if the implementation is done properly. :)

> For me normal try-finally is a lot more clean than calling atexit() in a 
> class method. It's just a strange interface. Prior to the patch user can 
> call execute_unittest several times and expect that output will be 
> printed during the call. After the patch outputs of all calls to 
> execute_unittest() will be printed at program exit..

Yeah, I agree.  I didn't like the finally, but I really didn't like it
because of the *behavior* of buffering the whole output.  I have changed
it to drop the buffering altogether, that's much better code and more usable:

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 90d0b62523..a74f4f9488 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -32,7 +32,7 @@
  import sys
  import time
  from typing import (Any, Callable, Dict, Iterable,
-                    List, Optional, Sequence, Tuple, TypeVar)
+                    List, Optional, Sequence, Tuple, Type, TypeVar)
  import unittest
  
  from contextlib import contextmanager
@@ -1271,37 +1271,49 @@ def func_wrapper(*args, **kwargs):
              return func(*args, **kwargs)
      return func_wrapper
  
+# We need to filter out the time taken from the output so that
+# qemu-iotest can reliably diff the results against master output,
+# and hide skipped tests from the reference output.
+
+class ReproducibleTestResult(unittest.TextTestResult):
+    def addSkip(self, test, reason):
+        # Same as TextTestResult, but print dot instead of "s"
+        unittest.TestResult.addSkip(self, test, reason)
+        if self.showAll:
+            self.stream.writeln("skipped {0!r}".format(reason))
+        elif self.dots:
+            self.stream.write(".")
+            self.stream.flush()
+
+class ReproducibleStreamWrapper(object):
+    def __init__(self, stream):
+        self.stream = stream
+
+    def __getattr__(self, attr):
+        if attr in ('stream', '__getstate__'):
+            raise AttributeError(attr)
+        return getattr(self.stream,attr)
+
+    def write(self, arg=None):
+        arg = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', arg)
+        arg = re.sub(r' \(skipped=\d+\)', r'', arg)
+        self.stream.write(arg)
+
+class ReproducibleTestRunner(unittest.TextTestRunner):
+    def __init__(self, stream: Optional[io.TextIOBase] = None,
+                 resultclass: Type = ReproducibleTestResult, *args, **kwargs):
+        rstream = ReproducibleStreamWrapper(stream or sys.stdout)
+        super().__init__(stream=rstream,           # type: ignore
+                         descriptions=True,
+                         resultclass=resultclass,
+                         *args, **kwargs)
+
  def execute_unittest(debug=False):
      """Executes unittests within the calling module."""
  
      verbosity = 2 if debug else 1
-
-    if debug:
-        output = sys.stdout
-    else:
-        # We need to filter out the time taken from the output so that
-        # qemu-iotest can reliably diff the results against master output.
-        output = io.StringIO()
-
-    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:
-        # We need to filter out the time taken from the output so that
-        # qemu-iotest can reliably diff the results against master output.
-        if not debug:
-            out = output.getvalue()
-            out = re.sub(r'Ran (\d+) tests? in [\d.]+s', r'Ran \1 tests', out)
-
-            # Hide skipped tests from the reference output
-            out = re.sub(r'OK \(skipped=\d+\)', 'OK', out)
-            out_first_line, out_rest = out.split('\n', 1)
-            out = out_first_line.replace('s', '.') + '\n' + out_rest
-
-            sys.stderr.write(out)
+    runner = unittest.ReproducibleTestRunner(verbosity=verbosity)
+    unittest.main(testRunner=ReproducibleTestRunner)
  
  def execute_setup_common(supported_fmts: Sequence[str] = (),
                           supported_platforms: Sequence[str] = (),



  reply	other threads:[~2021-03-23 18:25 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-23 13:06 [PATCH 0/4] qemu-iotests: quality of life improvements Paolo Bonzini
2021-03-23 13:06 ` [PATCH 1/4] qemu-iotests: allow passing unittest.main arguments to the test scripts Paolo Bonzini
2021-03-23 14:34   ` Vladimir Sementsov-Ogievskiy
2021-03-23 15:29     ` Paolo Bonzini
2021-03-23 16:04       ` Vladimir Sementsov-Ogievskiy
2021-03-23 16:56       ` Vladimir Sementsov-Ogievskiy
2021-03-23 17:04         ` Paolo Bonzini [this message]
2021-03-23 17:27           ` Vladimir Sementsov-Ogievskiy
2021-03-23 17:35             ` Paolo Bonzini
2021-03-23 13:06 ` [PATCH 2/4] qemu-iotests: move command line and environment handling from TestRunner to TestEnv Paolo Bonzini
2021-03-23 16:22   ` Vladimir Sementsov-Ogievskiy
2021-03-23 13:06 ` [PATCH 3/4] qemu-iotests: let "check" spawn an arbitrary test command Paolo Bonzini
2021-03-23 16:43   ` Vladimir Sementsov-Ogievskiy
2021-03-23 17:00     ` Paolo Bonzini
2021-03-23 17:11       ` Vladimir Sementsov-Ogievskiy
2021-03-23 17:22         ` Paolo Bonzini
2021-03-23 17:39           ` Vladimir Sementsov-Ogievskiy
2021-03-23 13:06 ` [PATCH 4/4] qemu-iotests: fix case of SOCK_DIR already in the environment Paolo Bonzini
2021-03-23 16:45   ` Vladimir Sementsov-Ogievskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=04724b6a-3be2-54ff-c769-35fed34e9135@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=eesposit@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.