[LTP] [PATCH 1/4] testcases/lib/run_tests.sh: Check expected results

[Li Wang]

On Tue, Dec 10, 2024 at 3:00 PM Petr Vorel <pvorel@suse.cz> wrote:

> > On Mon, Dec 9, 2024 at 6:14 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> > > Hi Li, all,
>
> > > ...
> > > > We could add a blank line print here to make the output better
> readable.
>
> > > >   echo ""
>
> > > > +               echo "*** Running '$test' (exp: $(tst_mask2flag
> $exp))
> > > ***"
>
> > > +1
>
>
> > > > > +               $test
> > > > > +               rc=$?
> > > > > +               if [ $rc = 127 ]; then
> > > > > +                       echo "Test '$test' not found, maybe
> out-of-tree
> > > > > build and unset builddir?" >&2
> > > > > +                       exit 1
> > > > > +               elif [ $rc = 2 -a $WHITELIST_GITHUB = 1 -a
> > > > > "$GITHUB_ACTIONS" ]; then
>
>
> > > > If one or more variables used in the conditional test are
> > > > either unset or empty, that will lead to invalid syntax.
>
> > > > So I would suggest using [ ... ] and &&:
>
> > > >   elif [ $rc = 2 ] && [ $WHITELIST_GITHUB = 1 ] && [ -n
> "$GITHUB_ACTIONS"
> > > > ]; then
>
> > > Good point. Or maybe just quote?
>
> > > elif [ "$rc" = 2 -a "$WHITELIST_GITHUB" = 1 -a "$GITHUB_ACTIONS" ];
> then
>
>
> > This can work,  but using -a can lead to ambiguous or hard-to-diagnose
> > behavior. A better approach would be to replace -a with &&.
>
> > Maybe the best way is:
>
> > elif [ "$rc" = 2 ] && [ "$WHITELIST_GITHUB" = 1 ] && [ -n
> "$GITHUB_ACTIONS"
> > ]; then
>
> Thank you for pointing this, I'll use this.
>
> I thought -a takes precedence to -o like in C, i.e. [ foo -a bar -o baz ]
> is the
> equivalent of [ foo ] && [ bar ] || baz.
>
> But now I see in man test(1):
>
>         Binary -a and -o are ambiguous.  Use 'test EXPR1 && test EXPR2' or
> 'test EXPR1 || test EXPR2' instead.
>
> I also wonder if any -a or -o is ambiguous. Or just combination of both.
> Because
> we have some "-a" and "-o" usage in tst_test.sh. Should we transform them?
>
>         if [ $TST_BROK -gt 0 -o $TST_FAIL -gt 0 -o $TST_WARN -gt 0 ]; then
>                 _tst_check_security_modules
>         fi
>

Usually this way is not recommended because if $TST_BROK is unset,
this becomes:
    if [ -gt 0 -o $TST_FAIL -gt 0 ... ]; then
which leads to an invalid error.

But in the tst_test.sh, TST_BROK has been set to 0 at the beginning so it
could be worked correctly each of the time. So far nobody complains there
is problem with running it, so we can keep it no change.



>
>         if [ "$TST_NEEDS_TMPDIR" = 1 -a -n "$TST_TMPDIR" ]; then
>

This is safer than above, quoting variables will be helpful.


                rm -r "$TST_TMPDIR"
>                 [ "$TST_TMPDIR_RHOST" = 1 ] && tst_cleanup_rhost
>         fi


>         NOTE: this one I'm going to change in upcoming fix of the TBROK =>
> TWARN evaluation
>         # TBROK => TWARN on cleanup or exit
>         if [ "$res" = TBROK ] && [ "$TST_DO_EXIT" = 1 -o -z
> "$TST_DO_CLEANUP" -a -n "$TST_CLEANUP" ]; then
>                 tst_res TWARN "$@"
>                 TST_DO_CLEANUP=
>                 return
>         fi
>
> Kind regards,
> Petr
>
> > > Kind regards,
> > > Petr
>
>

-- 
Regards,
Li Wang