diff mbox series

iotests: Write test output to TEST_DIR

Message ID 20220221132908.608461-1-hreitz@redhat.com
State New
Headers show
Series iotests: Write test output to TEST_DIR | expand

Commit Message

Hanna Czenczek Feb. 21, 2022, 1:29 p.m. UTC
Drop the use of OUTPUT_DIR (test/qemu-iotests under the build
directory), and instead write test output files (.out.bad, .notrun, and
.casenotrun) to TEST_DIR.

With this, the same test can be run concurrently without the separate
instances interfering, because they will need separate TEST_DIRs anyway.
Running the same test separately is useful when running the iotests with
various format/protocol combinations in parallel, or when you just want
to aggressively exercise a single test (e.g. when it fails only
sporadically).

Putting this output into TEST_DIR means that it will stick around for
inspection after the test run is done (though running the same test in
the same TEST_DIR will overwrite it, just as it used to be); but given
that TEST_DIR is a scratch directory, it should be clear that users can
delete all of its content at any point.  (And if TEST_DIR is on tmpfs,
it will just disappear on shutdown.)  Contrarily, alternative approaches
that would put these output files into OUTPUT_DIR with some prefix to
differentiate between separate test runs might easily lead to cluttering
OUTPUT_DIR.

(This change means OUTPUT_DIR is no longer written to by the iotests, so
we can drop its usage altogether.)

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
I used to have a draft patch to allow parallel test runs on my private
test branch, and since Thomas needs something like this for his "Improve
integration of iotests in the meson test harness", he took up the idea
and sent such a patch as part of that series's v2.

The approach taken there is to just prefix the .out.bad file's name with
the current PID, which tends to clutter the output directory after a
while.  Technically, this output directory is a scratch directory, but
at least I tend to delete/clean it only very rarely, so this is a bit
annoying.  Also, finding the .out.bad you are looking for is kind of
difficult when there are so many to choose from.

That's why this patch takes Vladimir's suggestion to put the test
output into TEST_DIR.  This way, these output files are in pure scratch
directories, and finding the one you are looking for should be easy (as
long as you know the respective TEST_DIR).
---
 tests/qemu-iotests/common.rc     |  6 +++---
 tests/qemu-iotests/iotests.py    |  5 ++---
 tests/qemu-iotests/testenv.py    |  5 +----
 tests/qemu-iotests/testrunner.py | 12 ++++++------
 4 files changed, 12 insertions(+), 16 deletions(-)
diff mbox series

Patch

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 9885030b43..5bde2415dc 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -726,7 +726,7 @@  _img_info()
 #
 _notrun()
 {
-    echo "$*" >"$OUTPUT_DIR/$seq.notrun"
+    echo "$*" >"$TEST_DIR/$seq.notrun"
     echo "$seq not run: $*"
     status=0
     exit
@@ -738,14 +738,14 @@  _notrun()
 #
 _casenotrun()
 {
-    echo "    [case not run] $*" >>"$OUTPUT_DIR/$seq.casenotrun"
+    echo "    [case not run] $*" >>"$TEST_DIR/$seq.casenotrun"
 }
 
 # just plain bail out
 #
 _fail()
 {
-    echo "$*" | tee -a "$OUTPUT_DIR/$seq.full"
+    echo "$*" | tee -a "$TEST_DIR/$seq.full"
     echo "(see $seq.full for details)"
     status=1
     exit 1
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 6ba65eb1ff..1d157d1325 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -84,7 +84,6 @@ 
 
 imgfmt = os.environ.get('IMGFMT', 'raw')
 imgproto = os.environ.get('IMGPROTO', 'file')
-output_dir = os.environ.get('OUTPUT_DIR', '.')
 
 try:
     test_dir = os.environ['TEST_DIR']
@@ -1209,7 +1208,7 @@  def notrun(reason):
     # Each test in qemu-iotests has a number ("seq")
     seq = os.path.basename(sys.argv[0])
 
-    with open('%s/%s.notrun' % (output_dir, seq), 'w', encoding='utf-8') \
+    with open('%s/%s.notrun' % (test_dir, seq), 'w', encoding='utf-8') \
             as outfile:
         outfile.write(reason + '\n')
     logger.warning("%s not run: %s", seq, reason)
@@ -1224,7 +1223,7 @@  def case_notrun(reason):
     # Each test in qemu-iotests has a number ("seq")
     seq = os.path.basename(sys.argv[0])
 
-    with open('%s/%s.casenotrun' % (output_dir, seq), 'a', encoding='utf-8') \
+    with open('%s/%s.casenotrun' % (test_dir, seq), 'a', encoding='utf-8') \
             as outfile:
         outfile.write('    [case not run] ' + reason + '\n')
 
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 0f32897fe8..b11e943c8a 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -66,7 +66,7 @@  class TestEnv(ContextManager['TestEnv']):
     # pylint: disable=too-many-instance-attributes
 
     env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
-                     'OUTPUT_DIR', 'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
+                     'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
                      'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
                      'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
                      'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT',
@@ -106,7 +106,6 @@  def init_directories(self) -> None:
              TEST_DIR
              SOCK_DIR
              SAMPLE_IMG_DIR
-             OUTPUT_DIR
         """
 
         # Path where qemu goodies live in this source tree.
@@ -134,8 +133,6 @@  def init_directories(self) -> None:
                                         os.path.join(self.source_iotests,
                                                      'sample_images'))
 
-        self.output_dir = os.getcwd()  # OUTPUT_DIR
-
     def init_binaries(self) -> None:
         """Init binary path variables:
              PYTHON (for bash tests)
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 0eace147b8..3d2e6231af 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -259,9 +259,6 @@  def do_run_test(self, test: str, mp: bool) -> TestResult:
         """
 
         f_test = Path(test)
-        f_bad = Path(f_test.name + '.out.bad')
-        f_notrun = Path(f_test.name + '.notrun')
-        f_casenotrun = Path(f_test.name + '.casenotrun')
         f_reference = Path(self.find_reference(test))
 
         if not f_test.exists():
@@ -276,9 +273,6 @@  def do_run_test(self, test: str, mp: bool) -> TestResult:
                               description='No qualified output '
                                           f'(expected {f_reference})')
 
-        for p in (f_bad, f_notrun, f_casenotrun):
-            silent_unlink(p)
-
         args = [str(f_test.resolve())]
         env = self.env.prepare_subprocess(args)
         if mp:
@@ -288,6 +282,12 @@  def do_run_test(self, test: str, mp: bool) -> TestResult:
                 env[d] = os.path.join(env[d], f_test.name)
                 Path(env[d]).mkdir(parents=True, exist_ok=True)
 
+        test_dir = env['TEST_DIR']
+        f_bad = Path(os.path.join(test_dir, f_test.name + '.out.bad'))
+        f_notrun = Path(os.path.join(test_dir, f_test.name + '.notrun'))
+        f_casenotrun = Path(os.path.join(test_dir,
+                                         f_test.name + '.casenotrun'))
+
         t0 = time.time()
         with f_bad.open('w', encoding="utf-8") as f:
             with subprocess.Popen(args, cwd=str(f_test.parent), env=env,