Re: [LTP] [PATCH] shell: Add support for runtime

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril, Li,

LGTM
Reviewed-by: Petr Vorel <pvorel@suse.cz>

> > +++ b/testcases/lib/tests/shell_loader_filesystems.sh
> > @@ -6,6 +6,7 @@
> >  # env
> >  # {
> >  #  "mount_device": true,
> > +#  "runtime": 10,
> >  #  "mntpoint": "ltp_mntpoint",
> >  #  "filesystems": [
> >  #   {
> > @@ -38,6 +39,14 @@ tst_test()
> >         else
> >                 tst_res TFAIL "Device not mounted!"
> >         fi
> > +
> > +       RUNTIME=$(tst_remaining_runtime)
> > +
> > +       if [ "$RUNTIME" -ge 9 ]; then
> > +               tst_res TPASS "Remaining runtime $RUNTIME"
> > +       else
> > +               tst_res TFAIL "Remaning runtime $RUNTIME"

> Typo ->  s/Remaning/Remaining

+1

> > +       fi
> >  }

..
> > +++ b/testcases/lib/tst_run_shell.c
> > @@ -60,6 +60,7 @@ enum test_attr_ids {
> >         MIN_CPUS,
> >         MIN_MEM_AVAIL,
> >         MIN_KVER,
> > +       MIN_RUNTIME,
> >         MIN_SWAP_AVAIL,
> >         MNTPOINT,
> >         MOUNT_DEVICE,
> > @@ -74,6 +75,7 @@ enum test_attr_ids {
> >         NEEDS_ROOT,
> >         NEEDS_TMPDIR,
> >         RESTORE_WALLCLOCK,
> > +       RUNTIME,


> Do we need to add a flexible TIMEOUT in shell support as well?

> The default TST_TIMEOUT in SHELL is 300, which is not consistent with C

300 s (5 min) is for tst_test.sh API. C API has 120 s (2 min), therefore that's
the default also for the shell loader API (the newest shell API).
And this specific test has 2 min 10 sec (due runtime 10).

The longer timeout is due Tumbleweed having CONFIG_FAULT_INJECTION, otherwise
it'd be 30 s (or 40 s for the test). I'm still not convinced that
CONFIG_FAULT_INJECTION itself should give 4x longer timeout for all
Tumbleweed/SLES testing. But that's something else, not relevant to the patch.

Kind regards,
Petr

> method (DEFAULT_TIMEOUT + tst_test->timeout).

> Anyway, this patch itself looks good:
> Reviewed-by: Li Wang <liwang@redhat.com>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp