[LTP] [PATCH v4 0/7] Run tests in CI

[LTP] [PATCH v4 0/7] Run tests in CI

[Petr Vorel]

Hi,

changes v3->v4:
* don't run on cross compile build
* add 2 commits to fix tests on (tst_net.sh on dash and tst_strstatus.c
  on alpine)
* partly rewrite runtest.sh

tested
https://github.com/pevik/ltp/actions/runs/1026124767

Kind regards,
Petr

Petr Vorel (7):
  tst_device: Require root
  tst_net.sh: Declare prefix variable as empty
  tst_strstatus.c: Use musl compatible status number
  lib: Add script for running tests
  make: Add make test{, -c, -shell} targets
  build.sh: Add support for make test
  CI: Run also make test

 .github/workflows/ci.yml         |   5 ++
 Makefile                         |  23 +++++
 build.sh                         |  22 ++++-
 lib/newlib_tests/runtest.sh      | 148 +++++++++++++++++++++++++++++++
 lib/newlib_tests/tst_device.c    |   1 +
 lib/newlib_tests/tst_strstatus.c |   3 +-
 testcases/lib/tst_net.sh         |   2 +-
 7 files changed, 201 insertions(+), 3 deletions(-)
 create mode 100755 lib/newlib_tests/runtest.sh

-- 
2.32.0

[LTP] [PATCH v4 1/7] tst_device: Require root

[Petr Vorel]

as it's actually required for successful run:
tst_device.c:100: TINFO: Not allowed to open /dev/loop-control. Are you root?: EACCES (13)
tst_device.c:139: TINFO: No free devices found
tst_device.c:335: TBROK: Failed to acquire device

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
The same as in v3.

 lib/newlib_tests/tst_device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/newlib_tests/tst_device.c b/lib/newlib_tests/tst_device.c
index ad077affd..0bee0a939 100644
--- a/lib/newlib_tests/tst_device.c
+++ b/lib/newlib_tests/tst_device.c
@@ -40,6 +40,7 @@ static void do_test(void)
 }
 
 static struct tst_test test = {
+	.needs_root = 1,
 	.needs_device = 1,
 	.dev_min_size = 300,
 	.test_all = do_test,
-- 
2.32.0

[LTP] [PATCH v4 2/7] tst_net.sh: Declare prefix variable as empty

[Petr Vorel]

in tst_ipaddr_un(). This is required to fix problem on dash, which
(unlike bash and busybox ash implementation) takes value of previously
defined variable, i.e.  guarding with local does not work:

$ foo=ee; bar() { local foo; echo "foo: '$foo'"; }; bar
foo: 'ee'

It requires declare it as empty:
$ foo=ee; bar() { local foo=; echo "foo: '$foo'"; }; bar
foo: ''

This problem seems to be on various dash versions from 0.5.8 to
0.5.11+git20200708+dd9ef66-5 (on Debian oldstable, stable, testing).
It'd require to set all variables, but now fix just prefix, which is
used in build.sh and it's quite common name.

Reviewed-by: Li Wang <liwang@redhat.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v4.

 testcases/lib/tst_net.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
index db1db4790..511fb7eb1 100644
--- a/testcases/lib/tst_net.sh
+++ b/testcases/lib/tst_net.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 # SPDX-License-Identifier: GPL-2.0-or-later
 # Copyright (c) 2014-2017 Oracle and/or its affiliates. All Rights Reserved.
-# Copyright (c) 2016-2019 Petr Vorel <pvorel@suse.cz>
+# Copyright (c) 2016-2021 Petr Vorel <pvorel@suse.cz>
 # Author: Alexey Kodanev <alexey.kodanev@oracle.com>
 
 [ -n "$TST_LIB_NET_LOADED" ] && return 0
@@ -408,7 +408,7 @@ tst_ipaddr_un()
 	local max_net_id=$default_max
 	local min_net_id=0
 
-	local counter host_id host_range is_counter max_host_id min_host_id net_id prefix tmp type
+	local counter host_id host_range is_counter max_host_id min_host_id net_id prefix= tmp type
 
 	local OPTIND
 	while getopts "c:h:n:p" opt; do
-- 
2.32.0

[LTP] [PATCH v4 3/7] tst_strstatus.c: Use musl compatible status number

[Petr Vorel]

musl since commit 41c632824c08 ("fix definitions of WIFSTOPPED and
WIFSIGNALED to support up to signal 127") returns 1 on
WIFSIGNALED(0xff). Thus test with WIFSIGNALED(0x1ff), which is
compatible for both glibc and musl.

Suggested-by: Alexey Kodanev <aleksei.kodanev@bell-sw.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v4.

 lib/newlib_tests/tst_strstatus.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/newlib_tests/tst_strstatus.c b/lib/newlib_tests/tst_strstatus.c
index aeeeb77ed..f8655fe82 100644
--- a/lib/newlib_tests/tst_strstatus.c
+++ b/lib/newlib_tests/tst_strstatus.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2017 Cyril Hrubis <chrubis@suse.cz>
+ * Copyright (c) 2019-2021 Petr Vorel <pvorel@suse.cz>
  */
 
 /*
@@ -18,7 +19,7 @@ static struct tcase {
 	{0x0001, "killed by SIGHUP"},
 	{0x137f, "is stopped"},
 	{0xffff, "is resumed"},
-	{0xff, "invalid status 0xff"},
+	{0x1ff, "invalid status 0x1ff"},
 };
 
 static void do_test(unsigned int n)
-- 
2.32.0

[LTP] [PATCH v4 4/7] lib: Add script for running tests

[Petr Vorel]

For now run only tests which TPASS or TCONF.

Disabled also problematic tests:

* tst_bool_expr: for some reason killed after testing:

/__w/ltp/ltp/lib/newlib_tests/tst_bool_expr.c:41: TINFO: Parsing 'A ( B )'
A ( B )

Summary:
passed   24
failed   0
broken   0
skipped  0
warnings 0
PATH: '/__w/ltp/ltp/../ltp-build/testcases/lib:/__w/ltp/ltp/lib/newlib_tests/../../testcases/lib/:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin'
DEBUG: 0
/__w/ltp/ltp/lib/tst_test.c:1363: TBROK: Test killed by SIGSEGV!

* tst_fuzzy_sync01: sporadically fails:
../../include/tst_fuzzy_sync.h:685: TINFO: Exceeded execution loops, requesting exit
tst_fuzzy_sync01.c:227: TFAIL: acs:3  act:1  art:1  | =:23   -:46   +:2999931
...
Summary:
passed   21
failed   3

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Partly rewritten since v3.

 lib/newlib_tests/runtest.sh | 148 ++++++++++++++++++++++++++++++++++++
 1 file changed, 148 insertions(+)
 create mode 100755 lib/newlib_tests/runtest.sh

diff --git a/lib/newlib_tests/runtest.sh b/lib/newlib_tests/runtest.sh
new file mode 100755
index 000000000..80bba00e9
--- /dev/null
+++ b/lib/newlib_tests/runtest.sh
@@ -0,0 +1,148 @@
+#!/bin/sh
+# Copyright (c) 2021 Petr Vorel <pvorel@suse.cz>
+
+LTP_C_API_TESTS="${LTP_C_API_TESTS:-test05 test07 test09 test12 test15 test18
+test_exec test_timer tst_res_hexd tst_strstatus tst_fuzzy_sync02 tst_fuzzy_sync03}"
+
+LTP_SHELL_API_TESTS="${LTP_SHELL_API_TESTS:-shell/tst_check_driver.sh shell/net/*.sh}"
+
+cd $(dirname $0)
+PATH="$PWD/../../testcases/lib/:$PATH"
+
+. tst_ansi_color.sh
+
+usage()
+{
+	cat << EOF
+Usage: $0 [-b DIR ] [-c|-s]
+-b DIR  build directory (required for out-of-tree build)
+-c      run C API tests only
+-s      run shell API tests only
+-h      print this help
+EOF
+}
+
+# custom version
+tst_flag2mask()
+{
+	case "$1" in
+	TPASS) return 0;;
+	TFAIL) return 1;;
+	TBROK) return 2;;
+	TWARN) return 4;;
+	TINFO) return 16;;
+	TCONF) return 32;;
+	esac
+}
+
+# custom version
+tst_res()
+{
+	if [ $# -eq 0 ]; then
+		echo >&2
+		return
+	fi
+
+	local res="$1"
+	shift
+
+	tst_color_enabled
+	local color=$?
+
+	printf "runtest " >&2
+	tst_print_colored $res "$res: " >&2
+	echo "$@" >&2
+
+}
+
+# custom version
+tst_brk()
+{
+	local res="$1"
+	shift
+
+	tst_flag2mask "$res"
+	local mask=$?
+
+	tst_res
+	tst_res $res $@
+
+	exit $mask
+}
+
+run_tests()
+{
+	local target="$1"
+	local i ret tconf tpass vars
+
+	eval vars="\$LTP_${target}_API_TESTS"
+
+	tst_res TINFO "=== Run $target tests ==="
+
+	for i in $vars; do
+		tst_res TINFO "* $i"
+		./$i
+		ret=$?
+
+		case $ret in
+			0) tpass="$tpass $i";;
+			1) tst_brk TFAIL "$i failed with TFAIL";;
+			2) tst_brk TFAIL "$i failed with TBROK";;
+			4) tst_brk TFAIL "$i failed with TWARN";;
+			32) tconf="$tconf $i";;
+			127) tst_brk TBROK "Error: file not found (wrong PATH? out-of-tree build without -b?), exit code: $ret";;
+			*) tst_brk TBROK "Error: unknown failure, exit code: $ret";;
+		esac
+		tst_res
+	done
+
+	[ -z "$tpass" ] && tpass=" none"
+	[ -z "$tconf" ] && tconf=" none"
+
+	tst_res TINFO "=== $target TEST RESULTS ==="
+	tst_res TINFO "Tests exited with TPASS:$tpass"
+	tst_res TINFO "Tests exited with TCONF:$tconf"
+	tst_res
+}
+
+run_c_tests()
+{
+	if [ "$builddir" ]; then
+		cd $builddir/lib/newlib_tests
+	fi
+
+	run_tests "C"
+
+	if [ "$builddir" ]; then
+		cd -
+	fi
+}
+
+run_shell_tests()
+{
+	run_tests "SHELL"
+}
+
+builddir=
+run=
+while getopts b:chs opt; do
+	case $opt in
+		'h') usage; exit 0;;
+		'b') builddir=$OPTARG; PATH="$builddir/testcases/lib:$PATH";;
+		'c') run="c";;
+		's') run="s";;
+		*) usage; tst_brk TBROK "Error: invalid option";;
+	esac
+done
+
+tst_res TINFO "PATH='$PATH'"
+
+if [ -z "$run" -o "$run" = "c" ]; then
+	run_c_tests
+fi
+
+if [ -z "$run" -o "$run" = "s" ]; then
+	run_shell_tests
+fi
+
+tst_res TPASS "No test failed"
-- 
2.32.0

[LTP] [PATCH v4 5/7] make: Add make test{, -c, -shell} targets

[Petr Vorel]

For testing C and shell API.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
changes v3->v4:
Don't run on cross-compile.

 Makefile | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/Makefile b/Makefile
index 56812d77b..ff4a70eec 100644
--- a/Makefile
+++ b/Makefile
@@ -192,6 +192,29 @@ $(INSTALL_TARGETS): $(INSTALL_DIR) $(DESTDIR)/$(bindir)
 ## Install
 install: $(INSTALL_TARGETS)
 
+## Test
+define _test
+	@set -e; $(top_srcdir)/lib/newlib_tests/runtest.sh -b $(abs_builddir) $(1)
+endef
+
+test: lib-all
+ifneq ($(build),$(host))
+	$(error running tests on cross-compile build not supported)
+endif
+	$(call _test)
+
+test-c: lib-all
+ifneq ($(build),$(host))
+	$(error running tests on cross-compile build not supported)
+endif
+	$(call _test,-c)
+
+test-shell: lib-all
+ifneq ($(build),$(host))
+	$(error running tests on cross-compile build not supported)
+endif
+	$(call _test,-s)
+
 ## Help
 .PHONY: help
 help:
-- 
2.32.0

[LTP] [PATCH v4 6/7] build.sh: Add support for make test

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
changes v3->v4:
Don't run on cross-compile.

 build.sh | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/build.sh b/build.sh
index 240ce8e68..9cb26e899 100755
--- a/build.sh
+++ b/build.sh
@@ -119,6 +119,17 @@ build_out_tree()
 	make $MAKE_OPTS_OUT_TREE
 }
 
+test_in_tree()
+{
+	make test
+}
+
+test_out_tree()
+{
+	cd $BUILD_DIR
+	make $MAKE_OPTS_OUT_TREE test
+}
+
 install_in_tree()
 {
 	make $MAKE_OPTS install
@@ -165,6 +176,7 @@ 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)
 install     run only 'make install'
 
 Default configure options:
@@ -192,7 +204,7 @@ while getopts "c:hio:p:r:t:" opt; do
 		esac;;
 	p) prefix="$OPTARG";;
 	r) case "$OPTARG" in
-		autotools|configure|build|install) run="$OPTARG";;
+		autotools|configure|build|test|install) run="$OPTARG";;
 		*) echo "Wrong run type '$OPTARG'" >&2; usage; exit 1;;
 		esac;;
 	t) case "$OPTARG" in
@@ -218,6 +230,14 @@ if [ -z "$run" -o "$run" = "build" ]; then
 	eval build_${tree}_tree
 fi
 
+if [ -z "$run" -o "$run" = "test" ]; then
+	if [ "$build" = "cross" ]; then
+		echo "cross-compile build, skipping running tests" >&2
+	else
+		eval test_${tree}_tree
+	fi
+fi
+
 if [ -z "$run" -o "$run" = "install" ]; then
 	if [ "$install" = 1 ]; then
 		eval install_${tree}_tree
-- 
2.32.0

[LTP] [PATCH v4 7/7] CI: Run also make test

[Petr Vorel]

on all targets

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
changes v3->v4:
Add $BUILD to not run on cross-compile.

 .github/workflows/ci.yml | 5 +++++
 1 file changed, 5 insertions(+)

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

[LTP] [PATCH v4 1/7] tst_device: Require root

[Cyril Hrubis]

Hi!
Obviously correct.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v4 2/7] tst_net.sh: Declare prefix variable as empty

[Cyril Hrubis]

Hi!
It starts to look like it would be easier to write a shell just for LTP
and stick to it :-).

Anyways:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v4 3/7] tst_strstatus.c: Use musl compatible status number

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v4 4/7] lib: Add script for running tests

[Cyril Hrubis]

Hi!
> For now run only tests which TPASS or TCONF.
> 
> Disabled also problematic tests:
> 
> * tst_bool_expr: for some reason killed after testing:
> 
> /__w/ltp/ltp/lib/newlib_tests/tst_bool_expr.c:41: TINFO: Parsing 'A ( B )'
> A ( B )
> 
> Summary:
> passed   24
> failed   0
> broken   0
> skipped  0
> warnings 0
> PATH: '/__w/ltp/ltp/../ltp-build/testcases/lib:/__w/ltp/ltp/lib/newlib_tests/../../testcases/lib/:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin'
> DEBUG: 0
> /__w/ltp/ltp/lib/tst_test.c:1363: TBROK: Test killed by SIGSEGV!

I've executed the test under valgrind and found some "conditional jump
depends on uninitialized value", which is because we do not clear the
priv pointer for newly added tokens. Does this patch fix it for you?

diff --git a/lib/tst_bool_expr.c b/lib/tst_bool_expr.c
index 387c38b91..15825e364 100644
--- a/lib/tst_bool_expr.c
+++ b/lib/tst_bool_expr.c
@@ -55,6 +55,7 @@ static int new_tok(struct tst_expr_tok **last, const char *tok, size_t tok_len)
        (*last)->tok = tok;
        (*last)->tok_len = tok_len;
        (*last)->op = char_to_op(tok[0]);
+       (*last)->priv = NULL;
        (*last)++;

        return 1;

> * tst_fuzzy_sync01: sporadically fails:
> ../../include/tst_fuzzy_sync.h:685: TINFO: Exceeded execution loops, requesting exit
> tst_fuzzy_sync01.c:227: TFAIL: acs:3  act:1  art:1  | =:23   -:46   +:2999931
> ...
> Summary:
> passed   21
> failed   3

Not sure what we can do here, I guess that timings would be hard to fix
on VMs that run the tests.

> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Partly rewritten since v3.
> 
>  lib/newlib_tests/runtest.sh | 148 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 148 insertions(+)
>  create mode 100755 lib/newlib_tests/runtest.sh
> 
> diff --git a/lib/newlib_tests/runtest.sh b/lib/newlib_tests/runtest.sh
> new file mode 100755
> index 000000000..80bba00e9
> --- /dev/null
> +++ b/lib/newlib_tests/runtest.sh
> @@ -0,0 +1,148 @@
> +#!/bin/sh
> +# Copyright (c) 2021 Petr Vorel <pvorel@suse.cz>
> +
> +LTP_C_API_TESTS="${LTP_C_API_TESTS:-test05 test07 test09 test12 test15 test18
> +test_exec test_timer tst_res_hexd tst_strstatus tst_fuzzy_sync02 tst_fuzzy_sync03}"
> +
> +LTP_SHELL_API_TESTS="${LTP_SHELL_API_TESTS:-shell/tst_check_driver.sh shell/net/*.sh}"
> +
> +cd $(dirname $0)
> +PATH="$PWD/../../testcases/lib/:$PATH"
> +
> +. tst_ansi_color.sh
> +
> +usage()
> +{
> +	cat << EOF
> +Usage: $0 [-b DIR ] [-c|-s]
> +-b DIR  build directory (required for out-of-tree build)
> +-c      run C API tests only
> +-s      run shell API tests only
> +-h      print this help
> +EOF
> +}
> +
> +# custom version
> +tst_flag2mask()
> +{
> +	case "$1" in
> +	TPASS) return 0;;
> +	TFAIL) return 1;;
> +	TBROK) return 2;;
> +	TWARN) return 4;;
> +	TINFO) return 16;;
> +	TCONF) return 32;;
> +	esac
> +}
> +
> +# custom version
> +tst_res()
> +{
> +	if [ $# -eq 0 ]; then
> +		echo >&2
> +		return
> +	fi
> +
> +	local res="$1"
> +	shift
> +
> +	tst_color_enabled
> +	local color=$?
> +
> +	printf "runtest " >&2
> +	tst_print_colored $res "$res: " >&2
> +	echo "$@" >&2
> +
> +}
> +
> +# custom version
> +tst_brk()
> +{
> +	local res="$1"
> +	shift
> +
> +	tst_flag2mask "$res"
> +	local mask=$?
> +
> +	tst_res
> +	tst_res $res $@
> +
> +	exit $mask
> +}

I'm not sure that we should call these function tst_res and tst_brk it
only confuses everything since these are different from the ones in the
test library.

> +run_tests()
> +{
> +	local target="$1"
> +	local i ret tconf tpass vars
> +
> +	eval vars="\$LTP_${target}_API_TESTS"
> +
> +	tst_res TINFO "=== Run $target tests ==="
> +
> +	for i in $vars; do
> +		tst_res TINFO "* $i"
> +		./$i
> +		ret=$?
> +
> +		case $ret in
> +			0) tpass="$tpass $i";;
> +			1) tst_brk TFAIL "$i failed with TFAIL";;
> +			2) tst_brk TFAIL "$i failed with TBROK";;
> +			4) tst_brk TFAIL "$i failed with TWARN";;
> +			32) tconf="$tconf $i";;
> +			127) tst_brk TBROK "Error: file not found (wrong PATH? out-of-tree build without -b?), exit code: $ret";;
> +			*) tst_brk TBROK "Error: unknown failure, exit code: $ret";;

Why do we exit on failure here?

We should just increase the fail counters and go ahead with next test.

> +		esac
> +		tst_res
> +	done
> +
> +	[ -z "$tpass" ] && tpass=" none"
> +	[ -z "$tconf" ] && tconf=" none"
> +
> +	tst_res TINFO "=== $target TEST RESULTS ==="
> +	tst_res TINFO "Tests exited with TPASS:$tpass"
> +	tst_res TINFO "Tests exited with TCONF:$tconf"
> +	tst_res
> +}
> +
> +run_c_tests()
> +{
> +	if [ "$builddir" ]; then
> +		cd $builddir/lib/newlib_tests
> +	fi
> +
> +	run_tests "C"
> +
> +	if [ "$builddir" ]; then
> +		cd -
> +	fi
> +}
> +
> +run_shell_tests()
> +{
> +	run_tests "SHELL"
> +}
> +
> +builddir=
> +run=
> +while getopts b:chs opt; do
> +	case $opt in
> +		'h') usage; exit 0;;
> +		'b') builddir=$OPTARG; PATH="$builddir/testcases/lib:$PATH";;
> +		'c') run="c";;
> +		's') run="s";;
> +		*) usage; tst_brk TBROK "Error: invalid option";;
> +	esac
> +done
> +
> +tst_res TINFO "PATH='$PATH'"
> +
> +if [ -z "$run" -o "$run" = "c" ]; then
> +	run_c_tests
> +fi
> +
> +if [ -z "$run" -o "$run" = "s" ]; then
> +	run_shell_tests
> +fi
> +
> +tst_res TPASS "No test failed"
> -- 
> 2.32.0
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v4 4/7] lib: Add script for running tests

[Cyril Hrubis]

Hi!
> Thanks! It looks like it helped (but few jobs haven't finished yet).
> https://github.com/pevik/ltp/actions/runs/1026771350
> Will you merge this fix yourself please?

It was obvious bug in the code, so I pushed it with yours Reported-by and
Tested-by.

> > Not sure what we can do here, I guess that timings would be hard to fix
> > on VMs that run the tests.
> 
> If I remember correctly Richie suggested that FAIL is also OK. He said only
> TBROK and TCONF is a problem. I'd prefer to fuzzy sync tests which always pass,
> but after this effort I can work on API tests metadata, which would allow also
> this.

Another possibility would be relaxing the timings on VMs. I guess that
we could change the treshold for reaching the criticall section to 10 on
VMs.

> > > +# custom version
> > > +tst_res()
> > > +{
> > > +	if [ $# -eq 0 ]; then
> > > +		echo >&2
> > > +		return
> > > +	fi
> > > +
> > > +	local res="$1"
> > > +	shift
> > > +
> > > +	tst_color_enabled
> > > +	local color=$?
> > > +
> > > +	printf "runtest " >&2
> > > +	tst_print_colored $res "$res: " >&2
> > > +	echo "$@" >&2
> > > +
> > > +}
> > > +
> > > +# custom version
> > > +tst_brk()
> > > +{
> > > +	local res="$1"
> > > +	shift
> > > +
> > > +	tst_flag2mask "$res"
> > > +	local mask=$?
> > > +
> > > +	tst_res
> > > +	tst_res $res $@
> > > +
> > > +	exit $mask
> > > +}
> 
> > I'm not sure that we should call these function tst_res and tst_brk it
> > only confuses everything since these are different from the ones in the
> > test library.
> OK, I'll rename it (runtest_res() and runtest_brk()).
> 
> > > +run_tests()
> > > +{
> > > +	local target="$1"
> > > +	local i ret tconf tpass vars
> > > +
> > > +	eval vars="\$LTP_${target}_API_TESTS"
> > > +
> > > +	tst_res TINFO "=== Run $target tests ==="
> > > +
> > > +	for i in $vars; do
> > > +		tst_res TINFO "* $i"
> > > +		./$i
> > > +		ret=$?
> > > +
> > > +		case $ret in
> > > +			0) tpass="$tpass $i";;
> > > +			1) tst_brk TFAIL "$i failed with TFAIL";;
> > > +			2) tst_brk TFAIL "$i failed with TBROK";;
> > > +			4) tst_brk TFAIL "$i failed with TWARN";;
> > > +			32) tconf="$tconf $i";;
> > > +			127) tst_brk TBROK "Error: file not found (wrong PATH? out-of-tree build without -b?), exit code: $ret";;
> > > +			*) tst_brk TBROK "Error: unknown failure, exit code: $ret";;
> 
> > Why do we exit on failure here?
> 
> > We should just increase the fail counters and go ahead with next test.
> 
> I quit here because you know how hard is to find error in very long log
> file. Also why to waste developer time when some test failed? Similar approach
> make has. But sure, I can continue here and print summary at the end.

As long as we run it on CI we really should run all tests.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v4 4/7] lib: Add script for running tests

[Petr Vorel]

> Hi!
> > For now run only tests which TPASS or TCONF.

> > Disabled also problematic tests:

> > * tst_bool_expr: for some reason killed after testing:

> > /__w/ltp/ltp/lib/newlib_tests/tst_bool_expr.c:41: TINFO: Parsing 'A ( B )'
> > A ( B )

> > Summary:
> > passed   24
> > failed   0
> > broken   0
> > skipped  0
> > warnings 0
> > PATH: '/__w/ltp/ltp/../ltp-build/testcases/lib:/__w/ltp/ltp/lib/newlib_tests/../../testcases/lib/:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin'
> > DEBUG: 0
> > /__w/ltp/ltp/lib/tst_test.c:1363: TBROK: Test killed by SIGSEGV!

> I've executed the test under valgrind and found some "conditional jump
> depends on uninitialized value", which is because we do not clear the
> priv pointer for newly added tokens. Does this patch fix it for you?

> diff --git a/lib/tst_bool_expr.c b/lib/tst_bool_expr.c
> index 387c38b91..15825e364 100644
> --- a/lib/tst_bool_expr.c
> +++ b/lib/tst_bool_expr.c
> @@ -55,6 +55,7 @@ static int new_tok(struct tst_expr_tok **last, const char *tok, size_t tok_len)
>         (*last)->tok = tok;
>         (*last)->tok_len = tok_len;
>         (*last)->op = char_to_op(tok[0]);
> +       (*last)->priv = NULL;
>         (*last)++;

Thanks! It looks like it helped (but few jobs haven't finished yet).
https://github.com/pevik/ltp/actions/runs/1026771350
Will you merge this fix yourself please?

>         return 1;

> > * tst_fuzzy_sync01: sporadically fails:
> > ../../include/tst_fuzzy_sync.h:685: TINFO: Exceeded execution loops, requesting exit
> > tst_fuzzy_sync01.c:227: TFAIL: acs:3  act:1  art:1  | =:23   -:46   +:2999931
> > ...
> > Summary:
> > passed   21
> > failed   3

> Not sure what we can do here, I guess that timings would be hard to fix
> on VMs that run the tests.

If I remember correctly Richie suggested that FAIL is also OK. He said only
TBROK and TCONF is a problem. I'd prefer to fuzzy sync tests which always pass,
but after this effort I can work on API tests metadata, which would allow also
this.

...
> > +# custom version
> > +tst_res()
> > +{
> > +	if [ $# -eq 0 ]; then
> > +		echo >&2
> > +		return
> > +	fi
> > +
> > +	local res="$1"
> > +	shift
> > +
> > +	tst_color_enabled
> > +	local color=$?
> > +
> > +	printf "runtest " >&2
> > +	tst_print_colored $res "$res: " >&2
> > +	echo "$@" >&2
> > +
> > +}
> > +
> > +# custom version
> > +tst_brk()
> > +{
> > +	local res="$1"
> > +	shift
> > +
> > +	tst_flag2mask "$res"
> > +	local mask=$?
> > +
> > +	tst_res
> > +	tst_res $res $@
> > +
> > +	exit $mask
> > +}

> I'm not sure that we should call these function tst_res and tst_brk it
> only confuses everything since these are different from the ones in the
> test library.
OK, I'll rename it (runtest_res() and runtest_brk()).

> > +run_tests()
> > +{
> > +	local target="$1"
> > +	local i ret tconf tpass vars
> > +
> > +	eval vars="\$LTP_${target}_API_TESTS"
> > +
> > +	tst_res TINFO "=== Run $target tests ==="
> > +
> > +	for i in $vars; do
> > +		tst_res TINFO "* $i"
> > +		./$i
> > +		ret=$?
> > +
> > +		case $ret in
> > +			0) tpass="$tpass $i";;
> > +			1) tst_brk TFAIL "$i failed with TFAIL";;
> > +			2) tst_brk TFAIL "$i failed with TBROK";;
> > +			4) tst_brk TFAIL "$i failed with TWARN";;
> > +			32) tconf="$tconf $i";;
> > +			127) tst_brk TBROK "Error: file not found (wrong PATH? out-of-tree build without -b?), exit code: $ret";;
> > +			*) tst_brk TBROK "Error: unknown failure, exit code: $ret";;

> Why do we exit on failure here?

> We should just increase the fail counters and go ahead with next test.

I quit here because you know how hard is to find error in very long log
file. Also why to waste developer time when some test failed? Similar approach
make has. But sure, I can continue here and print summary at the end.

Thanks for your review!
I merged the first 3 commits.

Kind regards,
Petr

[LTP] [PATCH v4 5/7] make: Add make test{, -c, -shell} targets

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v4 6/7] build.sh: Add support for make test

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v4 4/7] lib: Add script for running tests

[Petr Vorel]

Hi all,

...
> > > Not sure what we can do here, I guess that timings would be hard to fix
> > > on VMs that run the tests.

> > If I remember correctly Richie suggested that FAIL is also OK. He said only
> > TBROK and TCONF is a problem. I'd prefer to fuzzy sync tests which always pass,
> > but after this effort I can work on API tests metadata, which would allow also
> > this.

> Another possibility would be relaxing the timings on VMs. I guess that
> we could change the treshold for reaching the criticall section to 10 on
> VMs.
I'd be for it, we have checks for VM. @Richie, any opinion about it?

Kind regards,
Petr