| Message ID | 20220221172909.762858-1-hreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] iotests: Write test output to TEST_DIR | expand |
21.02.2022 20:29, Hanna Reitz wrote: > 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> > --- > v1: https://lists.nongnu.org/archive/html/qemu-block/2022-02/msg00675.html > > v2: > - Delete .casenotrun before running a test: Writes to this file only > append data, so if we do not delete it before a test run, it may still > contain stale data from a previous run > - While at it, we might as well delete .notrun, because before this > patch, all of .out.bad, .notrun, and .casenotrun are deleted. (Really > no need to delete .out.bad, though, given it is immediately > overwritten after where we delete .notrun and .casenotrun.) > --- > tests/qemu-iotests/common.rc | 6 +++--- > tests/qemu-iotests/iotests.py | 5 ++--- > tests/qemu-iotests/testenv.py | 5 +---- > tests/qemu-iotests/testrunner.py | 15 +++++++++------ > 4 files changed, 15 insertions(+), 16 deletions(-) > > 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..262b13004d 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,15 @@ 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')) You don't need os.path.join inside Path(), simple Path(test_dir, f_test.name + '...') should work. > + > + for p in (f_notrun, f_casenotrun): > + silent_unlink(p) Why don't you want to remove old f_bad too, like pre-patch? > + > t0 = time.time() > with f_bad.open('w', encoding="utf-8") as f: > with subprocess.Popen(args, cwd=str(f_test.parent), env=env,
On 22.02.22 15:39, Vladimir Sementsov-Ogievskiy wrote: > 21.02.2022 20:29, Hanna Reitz wrote: >> 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> >> --- >> v1: >> https://lists.nongnu.org/archive/html/qemu-block/2022-02/msg00675.html >> >> v2: >> - Delete .casenotrun before running a test: Writes to this file only >> append data, so if we do not delete it before a test run, it may >> still >> contain stale data from a previous run >> - While at it, we might as well delete .notrun, because before this >> patch, all of .out.bad, .notrun, and .casenotrun are deleted. >> (Really >> no need to delete .out.bad, though, given it is immediately >> overwritten after where we delete .notrun and .casenotrun.) >> --- >> tests/qemu-iotests/common.rc | 6 +++--- >> tests/qemu-iotests/iotests.py | 5 ++--- >> tests/qemu-iotests/testenv.py | 5 +---- >> tests/qemu-iotests/testrunner.py | 15 +++++++++------ >> 4 files changed, 15 insertions(+), 16 deletions(-) >> >> 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..262b13004d 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,15 @@ 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')) > > You don't need os.path.join inside Path(), simple > > Path(test_dir, f_test.name + '...') > > should work. Oh, good! >> + >> + for p in (f_notrun, f_casenotrun): >> + silent_unlink(p) > > Why don't you want to remove old f_bad too, like pre-patch? Mainly because... >> + >> t0 = time.time() >> with f_bad.open('w', encoding="utf-8") as f: I found it’d look just a bit silly when we immediately overwrite it here anyway. But if you prefer to keep the pre-patch list for completeness’ sake, I won’t mind. >> with subprocess.Popen(args, cwd=str(f_test.parent), >> env=env, > >
22.02.2022 17:44, Hanna Reitz wrote: > On 22.02.22 15:39, Vladimir Sementsov-Ogievskiy wrote: >> 21.02.2022 20:29, Hanna Reitz wrote: >>> 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> >>> --- >>> v1: https://lists.nongnu.org/archive/html/qemu-block/2022-02/msg00675.html >>> >>> v2: >>> - Delete .casenotrun before running a test: Writes to this file only >>> append data, so if we do not delete it before a test run, it may still >>> contain stale data from a previous run >>> - While at it, we might as well delete .notrun, because before this >>> patch, all of .out.bad, .notrun, and .casenotrun are deleted. (Really >>> no need to delete .out.bad, though, given it is immediately >>> overwritten after where we delete .notrun and .casenotrun.) >>> --- >>> tests/qemu-iotests/common.rc | 6 +++--- >>> tests/qemu-iotests/iotests.py | 5 ++--- >>> tests/qemu-iotests/testenv.py | 5 +---- >>> tests/qemu-iotests/testrunner.py | 15 +++++++++------ >>> 4 files changed, 15 insertions(+), 16 deletions(-) >>> >>> 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..262b13004d 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,15 @@ 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')) >> >> You don't need os.path.join inside Path(), simple >> >> Path(test_dir, f_test.name + '...') >> >> should work. > > Oh, good! > >>> + >>> + for p in (f_notrun, f_casenotrun): >>> + silent_unlink(p) >> >> Why don't you want to remove old f_bad too, like pre-patch? > > Mainly because... > >>> + >>> t0 = time.time() >>> with f_bad.open('w', encoding="utf-8") as f: > > I found it’d look just a bit silly when we immediately overwrite it here anyway. But if you prefer to keep the pre-patch list for completeness’ sake, I won’t mind. Ah, right, we rewrite it immediately, OK for me. With simplified Path(..) constructors: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > >>> with subprocess.Popen(args, cwd=str(f_test.parent), env=env, >> >> >
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..262b13004d 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,15 @@ 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')) + + for p in (f_notrun, f_casenotrun): + silent_unlink(p) + t0 = time.time() with f_bad.open('w', encoding="utf-8") as f: with subprocess.Popen(args, cwd=str(f_test.parent), env=env,
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> --- v1: https://lists.nongnu.org/archive/html/qemu-block/2022-02/msg00675.html v2: - Delete .casenotrun before running a test: Writes to this file only append data, so if we do not delete it before a test run, it may still contain stale data from a previous run - While at it, we might as well delete .notrun, because before this patch, all of .out.bad, .notrun, and .casenotrun are deleted. (Really no need to delete .out.bad, though, given it is immediately overwritten after where we delete .notrun and .casenotrun.) --- tests/qemu-iotests/common.rc | 6 +++--- tests/qemu-iotests/iotests.py | 5 ++--- tests/qemu-iotests/testenv.py | 5 +---- tests/qemu-iotests/testrunner.py | 15 +++++++++------ 4 files changed, 15 insertions(+), 16 deletions(-)