| Message ID | 20220209101530.3442837-2-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Improve integration of iotests in the meson test harness | expand |
Am 09.02.2022 um 11:15 hat Thomas Huth geschrieben: > If multiple tests run in parallel, they must use unique file > names for the test output. > > Suggested-by: Hanna Reitz <hreitz@redhat.com> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > tests/qemu-iotests/testrunner.py | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py > index 0eace147b8..9d20f51bb1 100644 > --- a/tests/qemu-iotests/testrunner.py > +++ b/tests/qemu-iotests/testrunner.py > @@ -259,7 +259,7 @@ def do_run_test(self, test: str, mp: bool) -> TestResult: > """ > > f_test = Path(test) > - f_bad = Path(f_test.name + '.out.bad') > + f_bad = Path(f'{os.getpid()}-{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)) Hmm... It does make sense, but nobody ever cleans those files up. Currently, the next run of the test will just overwrite the existing file or delete it when the test succeeds. So after running the test suite, you have .out.bad files for every failed test, but not for those that succeeded. After this change, won't the test directory accumulate tons of .out.bad files over time? Kevin
11.02.2022 12:29, Kevin Wolf wrote: > Am 09.02.2022 um 11:15 hat Thomas Huth geschrieben: >> If multiple tests run in parallel, they must use unique file >> names for the test output. >> >> Suggested-by: Hanna Reitz <hreitz@redhat.com> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> tests/qemu-iotests/testrunner.py | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py >> index 0eace147b8..9d20f51bb1 100644 >> --- a/tests/qemu-iotests/testrunner.py >> +++ b/tests/qemu-iotests/testrunner.py >> @@ -259,7 +259,7 @@ def do_run_test(self, test: str, mp: bool) -> TestResult: >> """ >> >> f_test = Path(test) >> - f_bad = Path(f_test.name + '.out.bad') >> + f_bad = Path(f'{os.getpid()}-{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)) > > Hmm... It does make sense, but nobody ever cleans those files up. > Currently, the next run of the test will just overwrite the existing > file or delete it when the test succeeds. So after running the test > suite, you have .out.bad files for every failed test, but not for those > that succeeded. > > After this change, won't the test directory accumulate tons of .out.bad > files over time? > Actually, .out.bad files are put not to TEST_DIR but to build/tests/qemu-iotests.. If we want to do several runs in parallel, I think all files that test-run produces should be in TEST_DIR and SOCK_DIR. And we should care to keep TEST_DIR/*.out.bad after test-run, so user can examine them.
On 11/02/2022 10.29, Kevin Wolf wrote: > Am 09.02.2022 um 11:15 hat Thomas Huth geschrieben: >> If multiple tests run in parallel, they must use unique file >> names for the test output. >> >> Suggested-by: Hanna Reitz <hreitz@redhat.com> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> tests/qemu-iotests/testrunner.py | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py >> index 0eace147b8..9d20f51bb1 100644 >> --- a/tests/qemu-iotests/testrunner.py >> +++ b/tests/qemu-iotests/testrunner.py >> @@ -259,7 +259,7 @@ def do_run_test(self, test: str, mp: bool) -> TestResult: >> """ >> >> f_test = Path(test) >> - f_bad = Path(f_test.name + '.out.bad') >> + f_bad = Path(f'{os.getpid()}-{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)) > > Hmm... It does make sense, but nobody ever cleans those files up. > Currently, the next run of the test will just overwrite the existing > file or delete it when the test succeeds. So after running the test > suite, you have .out.bad files for every failed test, but not for those > that succeeded. > > After this change, won't the test directory accumulate tons of .out.bad > files over time? True ... but we certainly want to keep the file for failed tests for further analysis instead of immediately deleting them ... maybe it would be enough to encode the image format (qcow2, qed, vmdk, ...) into the output name, instead of using the PID, so that "make check SPEED=thorough" works as expected here? Thomas
Am 11.02.2022 um 14:53 hat Thomas Huth geschrieben: > On 11/02/2022 10.29, Kevin Wolf wrote: > > Am 09.02.2022 um 11:15 hat Thomas Huth geschrieben: > > > If multiple tests run in parallel, they must use unique file > > > names for the test output. > > > > > > Suggested-by: Hanna Reitz <hreitz@redhat.com> > > > Signed-off-by: Thomas Huth <thuth@redhat.com> > > > --- > > > tests/qemu-iotests/testrunner.py | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py > > > index 0eace147b8..9d20f51bb1 100644 > > > --- a/tests/qemu-iotests/testrunner.py > > > +++ b/tests/qemu-iotests/testrunner.py > > > @@ -259,7 +259,7 @@ def do_run_test(self, test: str, mp: bool) -> TestResult: > > > """ > > > f_test = Path(test) > > > - f_bad = Path(f_test.name + '.out.bad') > > > + f_bad = Path(f'{os.getpid()}-{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)) > > > > Hmm... It does make sense, but nobody ever cleans those files up. > > Currently, the next run of the test will just overwrite the existing > > file or delete it when the test succeeds. So after running the test > > suite, you have .out.bad files for every failed test, but not for those > > that succeeded. > > > > After this change, won't the test directory accumulate tons of .out.bad > > files over time? > > True ... but we certainly want to keep the file for failed tests for further > analysis instead of immediately deleting them ... maybe it would be enough > to encode the image format (qcow2, qed, vmdk, ...) into the output name, > instead of using the PID, so that "make check SPEED=thorough" works as > expected here? It depends on what the supported use case for test suites running in parallel is. If it's just for testing multiple formats at the same time, then this would work, yes. I could think of more test runs that you might want to do in parallel, like different protocols, different image format options, maybe even different host file system. I'm not sure if all (or any) of these are relevant, though. Supporting only things that "make check" uses might be a good compromise. Kevin
On 11.02.22 17:00, Kevin Wolf wrote: > Am 11.02.2022 um 14:53 hat Thomas Huth geschrieben: >> On 11/02/2022 10.29, Kevin Wolf wrote: >>> Am 09.02.2022 um 11:15 hat Thomas Huth geschrieben: >>>> If multiple tests run in parallel, they must use unique file >>>> names for the test output. >>>> >>>> Suggested-by: Hanna Reitz <hreitz@redhat.com> >>>> Signed-off-by: Thomas Huth <thuth@redhat.com> >>>> --- >>>> tests/qemu-iotests/testrunner.py | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py >>>> index 0eace147b8..9d20f51bb1 100644 >>>> --- a/tests/qemu-iotests/testrunner.py >>>> +++ b/tests/qemu-iotests/testrunner.py >>>> @@ -259,7 +259,7 @@ def do_run_test(self, test: str, mp: bool) -> TestResult: >>>> """ >>>> f_test = Path(test) >>>> - f_bad = Path(f_test.name + '.out.bad') >>>> + f_bad = Path(f'{os.getpid()}-{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)) >>> Hmm... It does make sense, but nobody ever cleans those files up. >>> Currently, the next run of the test will just overwrite the existing >>> file or delete it when the test succeeds. So after running the test >>> suite, you have .out.bad files for every failed test, but not for those >>> that succeeded. >>> >>> After this change, won't the test directory accumulate tons of .out.bad >>> files over time? >> True ... but we certainly want to keep the file for failed tests for further >> analysis instead of immediately deleting them ... maybe it would be enough >> to encode the image format (qcow2, qed, vmdk, ...) into the output name, >> instead of using the PID, so that "make check SPEED=thorough" works as >> expected here? > It depends on what the supported use case for test suites running in > parallel is. If it's just for testing multiple formats at the same time, > then this would work, yes. > > I could think of more test runs that you might want to do in parallel, > like different protocols, different image format options, maybe even > different host file system. I'm not sure if all (or any) of these are > relevant, though. > > Supporting only things that "make check" uses might be a good > compromise. Personally and originally, I wrote that diff to allow me to actually run the very same test many times in parallel. If an error occurs only very rarely, then I like running like 24 loops of the same test with exactly the same configuration (just different TEST_DIRs, of course) in parallel. The fact that the .out.bad files tend to accumulate is why I haven’t sent it upstream so far. Personally, I like Vladimir’s idea to put them into TEST_DIR, but probably just because this works so well for my usual case where TEST_DIR is on tmpfs and I thus don’t have to clean it up. Hanna
Am 11.02.2022 um 17:14 hat Hanna Reitz geschrieben: > On 11.02.22 17:00, Kevin Wolf wrote: > > Am 11.02.2022 um 14:53 hat Thomas Huth geschrieben: > > > On 11/02/2022 10.29, Kevin Wolf wrote: > > > > Am 09.02.2022 um 11:15 hat Thomas Huth geschrieben: > > > > > If multiple tests run in parallel, they must use unique file > > > > > names for the test output. > > > > > > > > > > Suggested-by: Hanna Reitz <hreitz@redhat.com> > > > > > Signed-off-by: Thomas Huth <thuth@redhat.com> > > > > > --- > > > > > tests/qemu-iotests/testrunner.py | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py > > > > > index 0eace147b8..9d20f51bb1 100644 > > > > > --- a/tests/qemu-iotests/testrunner.py > > > > > +++ b/tests/qemu-iotests/testrunner.py > > > > > @@ -259,7 +259,7 @@ def do_run_test(self, test: str, mp: bool) -> TestResult: > > > > > """ > > > > > f_test = Path(test) > > > > > - f_bad = Path(f_test.name + '.out.bad') > > > > > + f_bad = Path(f'{os.getpid()}-{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)) > > > > Hmm... It does make sense, but nobody ever cleans those files up. > > > > Currently, the next run of the test will just overwrite the existing > > > > file or delete it when the test succeeds. So after running the test > > > > suite, you have .out.bad files for every failed test, but not for those > > > > that succeeded. > > > > > > > > After this change, won't the test directory accumulate tons of .out.bad > > > > files over time? > > > True ... but we certainly want to keep the file for failed tests for further > > > analysis instead of immediately deleting them ... maybe it would be enough > > > to encode the image format (qcow2, qed, vmdk, ...) into the output name, > > > instead of using the PID, so that "make check SPEED=thorough" works as > > > expected here? > > It depends on what the supported use case for test suites running in > > parallel is. If it's just for testing multiple formats at the same time, > > then this would work, yes. > > > > I could think of more test runs that you might want to do in parallel, > > like different protocols, different image format options, maybe even > > different host file system. I'm not sure if all (or any) of these are > > relevant, though. > > > > Supporting only things that "make check" uses might be a good > > compromise. > > Personally and originally, I wrote that diff to allow me to actually run the > very same test many times in parallel. If an error occurs only very rarely, > then I like running like 24 loops of the same test with exactly the same > configuration (just different TEST_DIRs, of course) in parallel. > > The fact that the .out.bad files tend to accumulate is why I haven’t sent it > upstream so far. Personally, I like Vladimir’s idea to put them into > TEST_DIR, but probably just because this works so well for my usual case > where TEST_DIR is on tmpfs and I thus don’t have to clean it up. I think it could actually work fine because if you don't override TEST_DIR, it's the same every time, and then you get the old behaviour, just with the .out.bad files moved into scratch/. Kevin
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py index 0eace147b8..9d20f51bb1 100644 --- a/tests/qemu-iotests/testrunner.py +++ b/tests/qemu-iotests/testrunner.py @@ -259,7 +259,7 @@ def do_run_test(self, test: str, mp: bool) -> TestResult: """ f_test = Path(test) - f_bad = Path(f_test.name + '.out.bad') + f_bad = Path(f'{os.getpid()}-{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 multiple tests run in parallel, they must use unique file names for the test output. Suggested-by: Hanna Reitz <hreitz@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> --- tests/qemu-iotests/testrunner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)