Re: [PATCH v2 2/5] Support format or cache specific out file

[Nir Soffer]

On Tue, Dec 13, 2022 at 8:09 PM Hanna Reitz <hreitz@redhat.com> wrote:
>
> On 13.12.22 16:56, Nir Soffer wrote:
> > On Mon, Dec 12, 2022 at 12:38 PM Hanna Reitz <hreitz@redhat.com> 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