[LTP] [PATCH 0/4] ci: run shell loader tests

[LTP] [PATCH 0/4] ci: run shell loader tests

[Petr Vorel]

Tested: https://github.com/pevik/ltp/actions/runs/12196430765

Petr Vorel (4):
  testcases/lib/run_tests.sh: Check expected results
  Makefile: Add target to run shell loader
  build.sh: Allow to run shell loader tests
  ci: Add shell loader tests

 .github/workflows/ci-docker-build.yml |   5 ++
 Makefile                              |   7 ++
 build.sh                              |  19 +++--
 testcases/lib/run_tests.sh            | 115 +++++++++++++++++++++-----
 4 files changed, 117 insertions(+), 29 deletions(-)

-- 
2.45.2


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

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

[Petr Vorel]

This verification helps 1) see if anything broke 2) be able to run in CI.

Also:
1) Allow to run tests outside of the test directory (call just by
relative PATH).
2) Allow to pass build directory (useful for out of tree build).
3) Allow to skip tests (useful for github CI).

shell_loader_all_filesystems.sh shell_loader_supported_archs.sh
shell_loader_filesystems.sh fails on Github Actions due broken loop
device therefore skip them:

tst_tmpdir.c:317: TINFO: Using /tmp/LTP_sheHtNv5R as tmpdir (overlayfs filesystem)
tst_device.c:147: TINFO: No free devices found
tst_device.c:360: TBROK: Failed to acquire device

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
NOTE: it's not perfect (we could finally add check which compares whole
output), but checking exit code is better than nothing.

lib/newlib_tests/runtest.sh uses a different approach, but I did not
bother trying to unify them. But it would be worth to add support for
tests which TBROK on GitHub also to lib/newlib_tests/runtest.sh. That
allows us to easily run 'make test' locally to get higher coverage (e.g.
before the release).

 testcases/lib/run_tests.sh | 115 ++++++++++++++++++++++++++++++-------
 1 file changed, 95 insertions(+), 20 deletions(-)

diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
index 40d415e6c4..380870ae55 100755
--- a/testcases/lib/run_tests.sh
+++ b/testcases/lib/run_tests.sh
@@ -1,32 +1,107 @@
 #!/bin/sh
 
-testdir=$(realpath $(dirname $0))
-export PATH="$PATH:$testdir:$testdir/tests/"
-
-for i in `seq -w 01 06`; do
-	echo
-	echo "*** Running shell_test$i ***"
-	echo
-	./tests/shell_test$i
-done
+TESTS_PASS="shell_test01 shell_test02 shell_test03 shell_test04 shell_test05
+shell_loader.sh shell_loader_tcnt.sh shell_loader_c_child.sh"
+
+TESTS_PASS_GITHUB_TBROK="shell_loader_all_filesystems.sh shell_loader_supported_archs.sh shell_loader_filesystems.sh shell_loader_kconfigs.sh"
+
+TESTS_FAIL="shell_loader_tags.sh"
+
+TESTS_TBROK="shell_loader_wrong_metadata.sh shell_loader_no_metadata.sh
+shell_loader_invalid_metadata.sh shell_loader_invalid_block.sh"
+
+TESTS_TCONF="shell_test06"
 
-for i in shell_loader.sh shell_loader_all_filesystems.sh shell_loader_no_metadata.sh \
-	 shell_loader_wrong_metadata.sh shell_loader_invalid_metadata.sh\
-	 shell_loader_supported_archs.sh shell_loader_filesystems.sh\
-	 shell_loader_tcnt.sh shell_loader_kconfigs.sh shell_loader_tags.sh \
-	 shell_loader_invalid_block.sh shell_loader_c_child.sh; do
-	echo
-	echo "*** Running $i ***"
-	echo
-	$i
+SKIP_GITHUB=
+FAIL=
+
+srcdir="$(realpath $(dirname $0))"
+builddir="$srcdir"
+
+usage()
+{
+	cat << EOF
+Usage: $0 [-b DIR ] [-s TESTS]
+-b DIR   build directory (required for out-of-tree build)
+-h       print this help
+EOF
+}
+
+while getopts b:h opt; do
+	case $opt in
+		'h') usage; exit 0;;
+		'b')
+			builddir="$OPTARG/testcases/lib/"
+			if [ ! -d "$builddir" ]; then
+				echo "directory '$builddir' does not exist!" >&2
+				exit 1
+			fi
+			;;
+		*) usage; runtest_brk TBROK "Error: invalid option";;
+	esac
 done
 
+# srcdir is for *.sh, builddir for *.c
+export PATH="$PATH:$srcdir:$builddir:$srcdir/tests/:$builddir/tests/"
+
+
+tst_mask2flag()
+{
+	case "$1" in
+	0) echo TPASS;;
+	1) echo TFAIL;;
+	2) echo TBROK;;
+	4) echo TWARN;;
+	16) echo TINFO;;
+	32) echo TCONF;;
+	esac
+}
+
+run_tests()
+{
+	local exp="$1"
+	local test rc
+	shift
+
+	for test in "$@"; do
+		echo "*** Running '$test' (exp: $(tst_mask2flag $exp)) ***"
+		$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
+			SKIP_GITHUB="$SKIP_GITHUB\n*$test"
+		elif [ $rc != $exp ]; then
+			FAIL="$FAIL\n* $test ($(tst_mask2flag $rc), exp: $(tst_mask2flag $exp))"
+		fi
+	done
+}
+
+run_tests 0 $TESTS_PASS
+WHITELIST_GITHUB=1 run_tests 0 $TESTS_PASS_GITHUB_TBROK
+run_tests 32 $TESTS_TCONF
+
 echo
 echo "*** Testing LTP test -h option ***"
 echo
-shell_loader.sh -h
+run_tests 0 "shell_loader.sh -h"
 
 echo
 echo "*** Testing LTP test -i option ***"
 echo
-shell_loader.sh -i 2
+run_tests 0 "shell_loader.sh -i 2"
+
+echo
+echo "***** RESULTS *****"
+
+if [ "$SKIP_GITHUB" ]; then
+	printf "Test skipped on GitHub Actions:$SKIP_GITHUB\n"
+fi
+
+if [ "$FAIL" ]; then
+	printf "Failed tests:$FAIL\n"
+	exit 1
+fi
+
+echo "All tests passed"
-- 
2.45.2


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

[LTP] [PATCH 2/4] Makefile: Add target to run shell loader

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 Makefile | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Makefile b/Makefile
index ae0247761c..5066789349 100644
--- a/Makefile
+++ b/Makefile
@@ -188,6 +188,7 @@ ifneq ($(build),$(host))
 	$(error running tests on cross-compile build not supported)
 endif
 	$(call _test)
+	$(MAKE) test-shell-loader
 	$(MAKE) test-metadata
 
 test-c: lib-all
@@ -202,6 +203,12 @@ ifneq ($(build),$(host))
 endif
 	$(call _test,-s)
 
+test-shell-loader: lib-all
+ifneq ($(build),$(host))
+	$(error running tests on cross-compile build not supported)
+endif
+	$(top_srcdir)/testcases/lib/run_tests.sh -b $(abs_builddir)
+
 test-metadata: metadata-all
 	$(MAKE) -C $(abs_srcdir)/metadata test
 
-- 
2.45.2


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

[LTP] [PATCH 3/4] build.sh: Allow to run shell loader tests

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 build.sh | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/build.sh b/build.sh
index 7bd0d38592..47a5a7b050 100755
--- a/build.sh
+++ b/build.sh
@@ -173,13 +173,14 @@ cross    cross-compile build (requires set compiler via -c switch)
 native   native build
 
 RUN:
-autotools   run only 'make autotools'
-configure   run only 'configure'
-build       run only 'make'
-test        run only 'make test' (not supported for cross-compile build)
-test-c      run only 'make test-c' (not supported for cross-compile build)
-test-shell  run only 'make test-shell' (not supported for cross-compile build)
-install     run only 'make install'
+autotools          run only 'make autotools'
+configure          run only 'configure'
+build              run only 'make'
+test               run only 'make test' (not supported for cross-compile build)
+test-c             run only 'make test-c' (not supported for cross-compile build)
+test-shell         run only 'make test-shell' (not supported for cross-compile build)
+test-shell-loader  run only 'make test-shell-loader' (not supported for cross-compile build)
+install            run only 'make install'
 
 Default configure options:
 in-tree:    $CONFIGURE_OPTS_IN_TREE
@@ -206,7 +207,7 @@ while getopts "c:hio:p:r:t:" opt; do
 		esac;;
 	p) prefix="$OPTARG";;
 	r) case "$OPTARG" in
-		autotools|configure|build|test|test-c|test-shell|install) run="$OPTARG";;
+		autotools|configure|build|test|test-c|test-shell|test-shell-loader|install) run="$OPTARG";;
 		*) echo "Wrong run type '$OPTARG'" >&2; usage; exit 1;;
 		esac;;
 	t) case "$OPTARG" in
@@ -232,7 +233,7 @@ if [ -z "$run" -o "$run" = "build" ]; then
 	eval build_${tree}_tree
 fi
 
-if [ -z "$run" -o "$run" = "test" -o "$run" = "test-c" -o "$run" = "test-shell" ]; then
+if [ -z "$run" -o "$run" = "test" -o "$run" = "test-c" -o "$run" = "test-shell" -o "$run" = "test-shell-loader" ]; then
 	if [ "$build" = "cross" ]; then
 		echo "cross-compile build, skipping running tests" >&2
 	else
-- 
2.45.2


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

[LTP] [PATCH 4/4] ci: Add shell loader tests

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 .github/workflows/ci-docker-build.yml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/.github/workflows/ci-docker-build.yml b/.github/workflows/ci-docker-build.yml
index 0445a35384..6b3bc2cef0 100644
--- a/.github/workflows/ci-docker-build.yml
+++ b/.github/workflows/ci-docker-build.yml
@@ -161,6 +161,11 @@ jobs:
         case "$VARIANT" in cross-compile*) BUILD="cross";; i386) BUILD="32";; *) BUILD="native";; esac
         ./build.sh -r test-shell -o ${TREE:-in} -t $BUILD
 
+    - name: Test shell loader
+      run: |
+        case "$VARIANT" in cross-compile*) BUILD="cross";; i386) BUILD="32";; *) BUILD="native";; esac
+        ./build.sh -r test-shell-loader -o ${TREE:-in} -t $BUILD
+
     - name: Install
       run: |
         if [ "$MAKE_INSTALL" = 1 ]; then INSTALL_OPT="-i"; fi
-- 
2.45.2


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

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

[Li Wang]

On Fri, Dec 6, 2024 at 5:50 PM Petr Vorel <pvorel@suse.cz> wrote:

> This verification helps 1) see if anything broke 2) be able to run in CI.
>
> Also:
> 1) Allow to run tests outside of the test directory (call just by
> relative PATH).
> 2) Allow to pass build directory (useful for out of tree build).
> 3) Allow to skip tests (useful for github CI).
>
> shell_loader_all_filesystems.sh shell_loader_supported_archs.sh
> shell_loader_filesystems.sh fails on Github Actions due broken loop
> device therefore skip them:
>
> tst_tmpdir.c:317: TINFO: Using /tmp/LTP_sheHtNv5R as tmpdir (overlayfs
> filesystem)
> tst_device.c:147: TINFO: No free devices found
> tst_device.c:360: TBROK: Failed to acquire device
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> NOTE: it's not perfect (we could finally add check which compares whole
> output), but checking exit code is better than nothing.
>
> lib/newlib_tests/runtest.sh uses a different approach, but I did not
> bother trying to unify them. But it would be worth to add support for
> tests which TBROK on GitHub also to lib/newlib_tests/runtest.sh. That
> allows us to easily run 'make test' locally to get higher coverage (e.g.
> before the release).
>
>  testcases/lib/run_tests.sh | 115 ++++++++++++++++++++++++++++++-------
>  1 file changed, 95 insertions(+), 20 deletions(-)
>
> diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
> index 40d415e6c4..380870ae55 100755
> --- a/testcases/lib/run_tests.sh
> +++ b/testcases/lib/run_tests.sh
> @@ -1,32 +1,107 @@
>  #!/bin/sh
>
> -testdir=$(realpath $(dirname $0))
> -export PATH="$PATH:$testdir:$testdir/tests/"
> -
> -for i in `seq -w 01 06`; do
> -       echo
> -       echo "*** Running shell_test$i ***"
> -       echo
> -       ./tests/shell_test$i
> -done
> +TESTS_PASS="shell_test01 shell_test02 shell_test03 shell_test04
> shell_test05
> +shell_loader.sh shell_loader_tcnt.sh shell_loader_c_child.sh"
> +
> +TESTS_PASS_GITHUB_TBROK="shell_loader_all_filesystems.sh
> shell_loader_supported_archs.sh shell_loader_filesystems.sh
> shell_loader_kconfigs.sh"
> +
> +TESTS_FAIL="shell_loader_tags.sh"
> +
> +TESTS_TBROK="shell_loader_wrong_metadata.sh shell_loader_no_metadata.sh
> +shell_loader_invalid_metadata.sh shell_loader_invalid_block.sh"
> +
> +TESTS_TCONF="shell_test06"
>
> -for i in shell_loader.sh shell_loader_all_filesystems.sh
> shell_loader_no_metadata.sh \
> -        shell_loader_wrong_metadata.sh shell_loader_invalid_metadata.sh\
> -        shell_loader_supported_archs.sh shell_loader_filesystems.sh\
> -        shell_loader_tcnt.sh shell_loader_kconfigs.sh
> shell_loader_tags.sh \
> -        shell_loader_invalid_block.sh shell_loader_c_child.sh; do
> -       echo
> -       echo "*** Running $i ***"
> -       echo
> -       $i
> +SKIP_GITHUB=
> +FAIL=
> +
> +srcdir="$(realpath $(dirname $0))"
> +builddir="$srcdir"
> +
> +usage()
> +{
> +       cat << EOF
> +Usage: $0 [-b DIR ] [-s TESTS]
> +-b DIR   build directory (required for out-of-tree build)
> +-h       print this help
> +EOF
> +}
> +
> +while getopts b:h opt; do
> +       case $opt in
> +               'h') usage; exit 0;;
> +               'b')
> +                       builddir="$OPTARG/testcases/lib/"
> +                       if [ ! -d "$builddir" ]; then
> +                               echo "directory '$builddir' does not
> exist!" >&2
> +                               exit 1
> +                       fi
> +                       ;;
> +               *) usage; runtest_brk TBROK "Error: invalid option";;
> +       esac
>  done
>
> +# srcdir is for *.sh, builddir for *.c
> +export PATH="$PATH:$srcdir:$builddir:$srcdir/tests/:$builddir/tests/"
> +
> +
> +tst_mask2flag()
> +{
> +       case "$1" in
> +       0) echo TPASS;;
> +       1) echo TFAIL;;
> +       2) echo TBROK;;
> +       4) echo TWARN;;
> +       16) echo TINFO;;
> +       32) echo TCONF;;
> +       esac
> +}
> +
> +run_tests()
> +{
> +       local exp="$1"
> +       local test rc
> +       shift
> +
> +       for test in "$@"; do
>

We could add a blank line print here to make the output better readable.

  echo ""

+               echo "*** Running '$test' (exp: $(tst_mask2flag $exp)) ***"
>


> +               $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


The whole patchset and CI job all look good.
CI:
https://github.com/wangli5665/ltp/actions/runs/12232764805/job/34118483369

Reviewed-by: Li Wang <liwang@redhat.com>


-- 
Regards,
Li Wang

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

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

[Petr Vorel]

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

Kind regards,
Petr

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

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

[Li Wang]

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



>
> Kind regards,
> Petr
>
>

-- 
Regards,
Li Wang

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

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

[Petr Vorel]

> 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

	if [ "$TST_NEEDS_TMPDIR" = 1 -a -n "$TST_TMPDIR" ]; then
		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

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

Re: [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

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