| Message ID | 20241206094938.92895-2-pvorel@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ci: run shell loader tests | expand |
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>
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
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 > >
> 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
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 > >
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"
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(-)