Re: [PATCH for-5.2] tests/9pfs: Mark "local" tests as "slow"

[Greg Kurz <groug@kaod.org>]

On Mon, 23 Nov 2020 19:08:18 +0100
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Montag, 23. November 2020 18:41:51 CET Greg Kurz wrote:
> > The "local" tests can fail on some automated build systems as
> > reported here:
> > 
> > https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg05510.html
> > 
> > This will need to be investigated and addressed later. Let's go for a
> > workaround in the meantime : mark the "local" tests as "slow" so that
> > they aren't executed with a simple "make check" like in the case above.
> > 
> > Reported-by: Cole Robinson <crobinso@redhat.com>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> > 
> > Unless there's a strong argument against merging this, I'll post
> > a PR with this fix for RC3.
> > ---
> >  tests/qtest/virtio-9p-test.c |   35 +++++++++++++++++++++--------------
> >  1 file changed, 21 insertions(+), 14 deletions(-)
> > 
> > diff --git a/tests/qtest/virtio-9p-test.c b/tests/qtest/virtio-9p-test.c
> > index 21e340fa5f43..dad37ace9772 100644
> > --- a/tests/qtest/virtio-9p-test.c
> > +++ b/tests/qtest/virtio-9p-test.c
> > @@ -1454,20 +1454,27 @@ static void register_virtio_9p_test(void)
> >      qos_add_test("synth/readdir/split_128", "virtio-9p",
> >                   fs_readdir_split_128,  &opts);
> > 
> > -
> > -    /* 9pfs test cases using the 'local' filesystem driver */
> > -    opts.before = assign_9p_local_driver;
> > -    qos_add_test("local/config", "virtio-9p", pci_config,  &opts);
> > -    qos_add_test("local/create_dir", "virtio-9p", fs_create_dir, &opts);
> > -    qos_add_test("local/unlinkat_dir", "virtio-9p", fs_unlinkat_dir,
> > &opts); -    qos_add_test("local/create_file", "virtio-9p", fs_create_file,
> > &opts); -    qos_add_test("local/unlinkat_file", "virtio-9p",
> > fs_unlinkat_file, &opts); -    qos_add_test("local/symlink_file",
> > "virtio-9p", fs_symlink_file, &opts); -   
> > qos_add_test("local/unlinkat_symlink", "virtio-9p", fs_unlinkat_symlink, - 
> >                &opts);
> > -    qos_add_test("local/hardlink_file", "virtio-9p", fs_hardlink_file,
> > &opts); -    qos_add_test("local/unlinkat_hardlink", "virtio-9p",
> > fs_unlinkat_hardlink, -                 &opts);
> > +    if (g_test_slow()) {
> 
> If you don't mind, I would suggest simply using
> 
> if (!g_test_slow()) {
> 	return;
> }
> 
> here to keep the diff noise low.
> 

Sure, I'll do that.

> > +        /* 9pfs test cases using the 'local' filesystem driver */
> > +        /*
> > +         * XXX: Until we are sure that these tests can run everywhere,
> > +         * keep them as "slow" so that they aren't run with "make check"
> > +         */
> > +        opts.before = assign_9p_local_driver;
> > +        qos_add_test("local/config", "virtio-9p", pci_config,  &opts);
> > +        qos_add_test("local/create_dir", "virtio-9p", fs_create_dir,
> > &opts); +        qos_add_test("local/unlinkat_dir", "virtio-9p",
> > fs_unlinkat_dir, &opts); +        qos_add_test("local/create_file",
> > "virtio-9p", fs_create_file, &opts); +       
> > qos_add_test("local/unlinkat_file", "virtio-9p", fs_unlinkat_file, +       
> >              &opts);
> > +        qos_add_test("local/symlink_file", "virtio-9p", fs_symlink_file,
> > &opts); +        qos_add_test("local/unlinkat_symlink", "virtio-9p",
> > fs_unlinkat_symlink, +                     &opts);
> > +        qos_add_test("local/hardlink_file", "virtio-9p", fs_hardlink_file,
> > +                     &opts);
> > +        qos_add_test("local/unlinkat_hardlink", "virtio-9p",
> > +                     fs_unlinkat_hardlink, &opts);
> > +    }
> >  }
> > 
> >  libqos_init(register_virtio_9p_test);
> 
> I agree that this is okay for now to avoid inconveniences with nearby 5.2 
> release, but do we agree that this is going to be reverted when 6.0 
> development phase starts?
> 
> In 6.0 I would rather handle this by introducing a previously mentioned 
> 'loglevel' option for 9p, and asking for the required log data if the local 
> tests fail for somebody. I can take care about the loglevel option when 6.0 
> phase starts.
> 
> Best regards,
> Christian Schoenebeck
> 
>