Re: [PATCH v2 2/5] Support format or cache specific out file
On Tue, Dec 13, 2022 at 8:09 PM Hanna Reitz wrote:
>
> On 13.12.22 16:56, Nir Soffer wrote:
> > On Mon, Dec 12, 2022 at 12:38 PM Hanna Reitz wrote:
> >> On 28.11.22 15:15, Nir Soffer wrote:
> >>> Extend the test finder to find tests with format (*.out.qcow2) or cache
> >>> specific (*.out.nocache) out file. This worked before only for the
> >>> numbered tests.
> >>> ---
> >>>tests/qemu-iotests/findtests.py | 10 --
> >>>1 file changed, 8 insertions(+), 2 deletions(-)
> >> This patch lacks an S-o-b, too.
> >>
> >>> diff --git a/tests/qemu-iotests/findtests.py
> >>> b/tests/qemu-iotests/findtests.py
> >>> index dd77b453b8..f4344ce78c 100644
> >>> --- a/tests/qemu-iotests/findtests.py
> >>> +++ b/tests/qemu-iotests/findtests.py
> >>> @@ -38,31 +38,37 @@ def chdir(path: Optional[str] = None) ->
> >>> Iterator[None]:
> >>>os.chdir(saved_dir)
> >>>
> >>>
> >>>class TestFinder:
> >>>def __init__(self, test_dir: Optional[str] = None) -> None:
> >>>self.groups = defaultdict(set)
> >>>
> >>>with chdir(test_dir):
> >>>self.all_tests = glob.glob('[0-9][0-9][0-9]')
> >>>self.all_tests += [f for f in glob.iglob('tests/*')
> >>> - if not f.endswith('.out') and
> >>> - os.path.isfile(f + '.out')]
> >>> + if self.is_test(f)]
> >> So previously a file was only considered a test file if there was a
> >> corresponding reference output file (`f + '.out'`), so files without
> >> such a reference output aren’t considered test files...
> >>
> >>>for t in self.all_tests:
> >>>with open(t, encoding="utf-8") as f:
> >>>for line in f:
> >>>if line.startswith('# group: '):
> >>>for g in line.split()[2:]:
> >>>self.groups[g].add(t)
> >>>break
> >>>
> >>> +def is_test(self, fname: str) -> bool:
> >>> +"""
> >>> +The tests directory contains tests (no extension) and out files
> >>> +(*.out, *.out.{format}, *.out.{option}).
> >>> +"""
> >>> +return re.search(r'.+\.out(\.\w+)?$', fname) is None
> >> ...but this new function doesn’t check that. I think we should check it
> >> (just whether there’s any variant of `/{fname}\.out(\.\w+)?/` to go with
> >> `fname`) so that behavior isn’t changed.
> > This means that you cannot add a test without a *.out* file, which may
> > be useful when you don't use the out file for validation, but we can
> > add this later if needed.
>
> I don’t think tests work without a reference output, do they? At least
> a couple of years ago, the ./check script would refuse to run tests
> without a corresponding .out file.
This may be true, but most tests do not really need an out file and better be
verified by asserting. There are some python tests that have pointless out
file with the output of python unittest:
$ cat tests/qemu-iotests/tests/nbd-multiconn.out
...
--
Ran 3 tests
OK
This is not only unhelpful (update the output when adding a 4th test)
but fragile.
if unitests changes the output, maybe adding info about skipped tests, or
changing "---" to "", the test will break.
But for now I agree the test framework should keep the current behavior.
Nir
Re: [PATCH v2 2/5] Support format or cache specific out file
On 13.12.22 16:56, Nir Soffer wrote:
On Mon, Dec 12, 2022 at 12:38 PM Hanna Reitz wrote:
On 28.11.22 15:15, Nir Soffer wrote:
Extend the test finder to find tests with format (*.out.qcow2) or cache
specific (*.out.nocache) out file. This worked before only for the
numbered tests.
---
tests/qemu-iotests/findtests.py | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
This patch lacks an S-o-b, too.
diff --git a/tests/qemu-iotests/findtests.py b/tests/qemu-iotests/findtests.py
index dd77b453b8..f4344ce78c 100644
--- a/tests/qemu-iotests/findtests.py
+++ b/tests/qemu-iotests/findtests.py
@@ -38,31 +38,37 @@ def chdir(path: Optional[str] = None) -> Iterator[None]:
os.chdir(saved_dir)
class TestFinder:
def __init__(self, test_dir: Optional[str] = None) -> None:
self.groups = defaultdict(set)
with chdir(test_dir):
self.all_tests = glob.glob('[0-9][0-9][0-9]')
self.all_tests += [f for f in glob.iglob('tests/*')
- if not f.endswith('.out') and
- os.path.isfile(f + '.out')]
+ if self.is_test(f)]
So previously a file was only considered a test file if there was a
corresponding reference output file (`f + '.out'`), so files without
such a reference output aren’t considered test files...
for t in self.all_tests:
with open(t, encoding="utf-8") as f:
for line in f:
if line.startswith('# group: '):
for g in line.split()[2:]:
self.groups[g].add(t)
break
+def is_test(self, fname: str) -> bool:
+"""
+The tests directory contains tests (no extension) and out files
+(*.out, *.out.{format}, *.out.{option}).
+"""
+return re.search(r'.+\.out(\.\w+)?$', fname) is None
...but this new function doesn’t check that. I think we should check it
(just whether there’s any variant of `/{fname}\.out(\.\w+)?/` to go with
`fname`) so that behavior isn’t changed.
This means that you cannot add a test without a *.out* file, which may
be useful when you don't use the out file for validation, but we can
add this later if needed.
I don’t think tests work without a reference output, do they? At least
a couple of years ago, the ./check script would refuse to run tests
without a corresponding .out file.
Hanna
Re: [PATCH v2 2/5] Support format or cache specific out file
On Mon, Dec 12, 2022 at 12:38 PM Hanna Reitz wrote:
>
> On 28.11.22 15:15, Nir Soffer wrote:
> > Extend the test finder to find tests with format (*.out.qcow2) or cache
> > specific (*.out.nocache) out file. This worked before only for the
> > numbered tests.
> > ---
> > tests/qemu-iotests/findtests.py | 10 --
> > 1 file changed, 8 insertions(+), 2 deletions(-)
>
> This patch lacks an S-o-b, too.
>
> > diff --git a/tests/qemu-iotests/findtests.py
> > b/tests/qemu-iotests/findtests.py
> > index dd77b453b8..f4344ce78c 100644
> > --- a/tests/qemu-iotests/findtests.py
> > +++ b/tests/qemu-iotests/findtests.py
> > @@ -38,31 +38,37 @@ def chdir(path: Optional[str] = None) -> Iterator[None]:
> > os.chdir(saved_dir)
> >
> >
> > class TestFinder:
> > def __init__(self, test_dir: Optional[str] = None) -> None:
> > self.groups = defaultdict(set)
> >
> > with chdir(test_dir):
> > self.all_tests = glob.glob('[0-9][0-9][0-9]')
> > self.all_tests += [f for f in glob.iglob('tests/*')
> > - if not f.endswith('.out') and
> > - os.path.isfile(f + '.out')]
> > + if self.is_test(f)]
>
> So previously a file was only considered a test file if there was a
> corresponding reference output file (`f + '.out'`), so files without
> such a reference output aren’t considered test files...
>
> > for t in self.all_tests:
> > with open(t, encoding="utf-8") as f:
> > for line in f:
> > if line.startswith('# group: '):
> > for g in line.split()[2:]:
> > self.groups[g].add(t)
> > break
> >
> > +def is_test(self, fname: str) -> bool:
> > +"""
> > +The tests directory contains tests (no extension) and out files
> > +(*.out, *.out.{format}, *.out.{option}).
> > +"""
> > +return re.search(r'.+\.out(\.\w+)?$', fname) is None
>
> ...but this new function doesn’t check that. I think we should check it
> (just whether there’s any variant of `/{fname}\.out(\.\w+)?/` to go with
> `fname`) so that behavior isn’t changed.
This means that you cannot add a test without a *.out* file, which may
be useful when you don't use the out file for validation, but we can
add this later if needed.
I'll change the code to check both conditions.
Re: [PATCH v2 2/5] Support format or cache specific out file
On 28.11.22 15:15, Nir Soffer wrote:
Extend the test finder to find tests with format (*.out.qcow2) or cache
specific (*.out.nocache) out file. This worked before only for the
numbered tests.
---
tests/qemu-iotests/findtests.py | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
This patch lacks an S-o-b, too.
diff --git a/tests/qemu-iotests/findtests.py b/tests/qemu-iotests/findtests.py
index dd77b453b8..f4344ce78c 100644
--- a/tests/qemu-iotests/findtests.py
+++ b/tests/qemu-iotests/findtests.py
@@ -38,31 +38,37 @@ def chdir(path: Optional[str] = None) -> Iterator[None]:
os.chdir(saved_dir)
class TestFinder:
def __init__(self, test_dir: Optional[str] = None) -> None:
self.groups = defaultdict(set)
with chdir(test_dir):
self.all_tests = glob.glob('[0-9][0-9][0-9]')
self.all_tests += [f for f in glob.iglob('tests/*')
- if not f.endswith('.out') and
- os.path.isfile(f + '.out')]
+ if self.is_test(f)]
So previously a file was only considered a test file if there was a
corresponding reference output file (`f + '.out'`), so files without
such a reference output aren’t considered test files...
for t in self.all_tests:
with open(t, encoding="utf-8") as f:
for line in f:
if line.startswith('# group: '):
for g in line.split()[2:]:
self.groups[g].add(t)
break
+def is_test(self, fname: str) -> bool:
+"""
+The tests directory contains tests (no extension) and out files
+(*.out, *.out.{format}, *.out.{option}).
+"""
+return re.search(r'.+\.out(\.\w+)?$', fname) is None
...but this new function doesn’t check that. I think we should check it
(just whether there’s any variant of `/{fname}\.out(\.\w+)?/` to go with
`fname`) so that behavior isn’t changed.
Hanna
