Re: [LTP] [PATCH 2/6] tst_env.sh: Backport common functions from tst_test.sh

[Petr Vorel <pvorel@suse.cz>]

> Hi!
...

> > +ROD_SILENT()
> > +{
> > +	local tst_out
> > +
> > +	tst_out=$(tst_rod "$@" 2>&1)
> > +	if [ $? -ne 0 ]; then
> > +		echo "$tst_out"
> > +		tst_brk TBROK "$@ failed"
> > +	fi
> > +}
> > +
> > +ROD()
> > +{
> > +	tst_rod "$@"
> > +	if [ $? -ne 0 ]; then
> > +		tst_brk TBROK "$@ failed"
> > +	fi
> > +}

> Since we are starting from a scratch I wonder if we should call this
> SAFE instead so that the name is closer to the SAFE_XXX macros in C.

Makes sense. I was even thinking on SAFE_CMD, but we use tst_rod.c, not
tst_cmd.c. OTOH it was obvious that ROD uses tst_rod.c, should it be also
renamed? What ROD means anyway? run or dye?

> > +_tst_expect_pass()
> > +{
> > +	local fnc="$1"
> > +	shift
> > +
> > +	tst_rod "$@"

> If I remmeber correctly the whole reason why we introduced tst_rod.c was
> that passing the $@ like this causes the $@ to be evaluated twice and
> produces unexpected results.

FYI code is copy pasted from tst_test.sh. What do you want me to change?

> > +	if [ $? -eq 0 ]; then
> > +		tst_res TPASS "$@ passed as expected"
> > +		return 0
> > +	else
> > +		$fnc TFAIL "$@ failed unexpectedly"
> > +		return 1
> > +	fi
> > +}
> > +
> > +_tst_expect_fail()
> > +{
> > +	local fnc="$1"
> > +	shift
> > +
> > +	# redirect stderr since we expect the command to fail
> > +	tst_rod "$@" 2> /dev/null
> > +	if [ $? -ne 0 ]; then
> > +		tst_res TPASS "$@ failed as expected"
> > +		return 0
> > +	else
> > +		$fnc TFAIL "$@ passed unexpectedly"
> > +		return 1
> > +	fi
> > +}
> > +
> > +EXPECT_PASS()
> > +{
> > +	_tst_expect_pass tst_res "$@"
> > +}
> > +
> > +EXPECT_PASS_BRK()
> > +{
> > +	_tst_expect_pass tst_brk "$@"
> > +}

> I'm not sure that adding the PASS_BRK and FAIL_BRK is a good idea. I
> would stick to simple EXPECT_PASS and EXPECT_FAIL. And maybe we can
> export TST_PASS variable as we do in C to match the API. I think that
> the closer the C and shell API are the better.

C API usually returns on if (!TST_PASS). I don't like it either, but it cannot
be shortened much as we don't use functions but macros.

But changing the code in the shell API does not look to me an improvement:

-EXPECT_PASS_BRK ping$TST_IPV6 -c1 -I $lhost $rhost \>/dev/null
+EXPECT_PASS_BRK ping$TST_IPV6 -c1 -I $lhost $rhost \>/dev/null
+[ "$TST_PASS" = 1 ] || tst_brk "Quit due the above error"

But OTOH maybe route-change-*.sh tests even don't need to quit.
And it's a question if isofs.sh does (and whether we should even keep isofs.sh).
=> I'm ok with it.

Kind regards,
Petr

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