Linux Test Project
 help / color / mirror / Atom feed
* [LTP] [RFC PATCH 0/5] shell loader rewrite to support TST_SETUP
@ 2025-02-28 17:24 Petr Vorel
  2025-02-28 17:24 ` [LTP] [RFC PATCH 1/5] shell lib: Add support for test cleanup Petr Vorel
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Petr Vorel @ 2025-02-28 17:24 UTC (permalink / raw)
  To: ltp

Hi,

attempt to implement Cyril's suggestions:

https://lore.kernel.org/ltp/Z69OLsDLMzNw6RGt@yuki.lan/
https://lore.kernel.org/ltp/Z69QpMlrGVDwpz8w@yuki.lan/

Kind regards,
Petr

Cyril Hrubis (1):
  shell lib: Add support for test cleanup

Petr Vorel (4):
  lib: Allow test to have positional args
  shell: Move shell code into functions
  shell lib: Add basic support for test cleanup
  shell: Add shell_loader_setup_cleanup.sh test

 doc/developers/writing_tests.rst              |  2 +-
 include/tst_test.h                            |  5 +-
 lib/tst_test.c                                |  9 +++-
 testcases/kernel/mem/vma/vma05.sh             | 47 ++++++++++---------
 testcases/lib/run_tests.sh                    |  5 +-
 testcases/lib/tests/shell_loader.sh           | 19 ++++----
 .../lib/tests/shell_loader_all_filesystems.sh | 26 +++++-----
 .../lib/tests/shell_loader_brk_cleanup.sh     | 23 +++++++++
 testcases/lib/tests/shell_loader_c_child.sh   | 15 +++---
 testcases/lib/tests/shell_loader_cleanup.sh   | 23 +++++++++
 .../lib/tests/shell_loader_filesystems.sh     | 23 +++++----
 .../lib/tests/shell_loader_invalid_block.sh   |  7 ++-
 .../tests/shell_loader_invalid_metadata.sh    |  7 ++-
 testcases/lib/tests/shell_loader_kconfigs.sh  |  7 ++-
 .../lib/tests/shell_loader_no_metadata.sh     |  7 ++-
 .../lib/tests/shell_loader_setup_cleanup.sh   | 29 ++++++++++++
 .../lib/tests/shell_loader_supported_archs.sh |  7 ++-
 testcases/lib/tests/shell_loader_tags.sh      |  7 ++-
 testcases/lib/tests/shell_loader_tcnt.sh      |  7 ++-
 .../lib/tests/shell_loader_wrong_metadata.sh  |  7 ++-
 testcases/lib/tst_exec.sh                     | 19 ++++++++
 testcases/lib/tst_loader.sh                   |  5 +-
 testcases/lib/tst_run_shell.c                 | 16 +++++--
 23 files changed, 235 insertions(+), 87 deletions(-)
 create mode 100755 testcases/lib/tests/shell_loader_brk_cleanup.sh
 create mode 100755 testcases/lib/tests/shell_loader_cleanup.sh
 create mode 100755 testcases/lib/tests/shell_loader_setup_cleanup.sh
 create mode 100755 testcases/lib/tst_exec.sh

-- 
2.47.2


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [LTP] [RFC PATCH 1/5] shell lib: Add support for test cleanup
  2025-02-28 17:24 [LTP] [RFC PATCH 0/5] shell loader rewrite to support TST_SETUP Petr Vorel
@ 2025-02-28 17:24 ` Petr Vorel
  2025-03-04 12:57   ` Li Wang
  2025-02-28 17:24 ` [LTP] [RFC PATCH 2/5] lib: Allow test to have positional args Petr Vorel
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Petr Vorel @ 2025-02-28 17:24 UTC (permalink / raw)
  To: ltp

From: Cyril Hrubis <chrubis@suse.cz>

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
The same as in:
https://patchwork.ozlabs.org/project/ltp/patch/20250214112135.18947-3-chrubis@suse.cz/

 testcases/lib/run_tests.sh                    |  4 +++-
 .../lib/tests/shell_loader_brk_cleanup.sh     | 20 +++++++++++++++++++
 testcases/lib/tests/shell_loader_cleanup.sh   | 20 +++++++++++++++++++
 testcases/lib/tst_env.sh                      |  4 ++++
 4 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100755 testcases/lib/tests/shell_loader_brk_cleanup.sh
 create mode 100755 testcases/lib/tests/shell_loader_cleanup.sh

diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
index 321f74e5fe..128cee3377 100755
--- a/testcases/lib/run_tests.sh
+++ b/testcases/lib/run_tests.sh
@@ -9,6 +9,7 @@ shell_loader_filesystems.sh
 shell_loader_kconfigs.sh
 shell_loader_supported_archs.sh
 shell_loader_tcnt.sh
+shell_loader_cleanup.sh
 shell_test01
 shell_test02
 shell_test03
@@ -21,7 +22,8 @@ TESTS_TBROK="
 shell_loader_invalid_block.sh
 shell_loader_invalid_metadata.sh
 shell_loader_no_metadata.sh
-shell_loader_wrong_metadata.sh"
+shell_loader_wrong_metadata.sh
+shell_loader_brk_cleanup.sh"
 
 TESTS_TCONF="shell_test06"
 
diff --git a/testcases/lib/tests/shell_loader_brk_cleanup.sh b/testcases/lib/tests/shell_loader_brk_cleanup.sh
new file mode 100755
index 0000000000..8c704a5406
--- /dev/null
+++ b/testcases/lib/tests/shell_loader_brk_cleanup.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2024-2025 Cyril Hrubis <chrubis@suse.cz>
+#
+# ---
+# env
+# {
+# }
+# ---
+
+TST_CLEANUP=cleanup
+
+. tst_loader.sh
+
+cleanup()
+{
+	tst_res TINFO "Cleanup runs"
+}
+
+tst_brk TBROK "Test exits"
diff --git a/testcases/lib/tests/shell_loader_cleanup.sh b/testcases/lib/tests/shell_loader_cleanup.sh
new file mode 100755
index 0000000000..fb7bbdf5a9
--- /dev/null
+++ b/testcases/lib/tests/shell_loader_cleanup.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2024-2025 Cyril Hrubis <chrubis@suse.cz>
+#
+# ---
+# env
+# {
+# }
+# ---
+
+TST_CLEANUP=do_cleanup
+
+. tst_loader.sh
+
+do_cleanup()
+{
+	tst_res TINFO "Cleanup executed"
+}
+
+tst_res TPASS "Test is executed"
diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh
index 68f9a0daa9..b13bab37c3 100644
--- a/testcases/lib/tst_env.sh
+++ b/testcases/lib/tst_env.sh
@@ -35,3 +35,7 @@ tst_brk_()
 
 alias tst_res="tst_res_ $tst_script_name \$LINENO"
 alias tst_brk="tst_brk_ $tst_script_name \$LINENO"
+
+if [ -n "$TST_CLEANUP" ]; then
+	trap $TST_CLEANUP EXIT
+fi
-- 
2.47.2


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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [LTP] [RFC PATCH 2/5] lib: Allow test to have positional args
  2025-02-28 17:24 [LTP] [RFC PATCH 0/5] shell loader rewrite to support TST_SETUP Petr Vorel
  2025-02-28 17:24 ` [LTP] [RFC PATCH 1/5] shell lib: Add support for test cleanup Petr Vorel
@ 2025-02-28 17:24 ` Petr Vorel
  2025-02-28 17:43   ` Petr Vorel
                     ` (2 more replies)
  2025-02-28 17:24 ` [LTP] [RFC PATCH 3/5] shell: Move shell code into functions Petr Vorel
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 22+ messages in thread
From: Petr Vorel @ 2025-02-28 17:24 UTC (permalink / raw)
  To: ltp

Similar to $TST_POS_ARGS in shell API. This will be used in
testcases/lib/tst_run_shell.c.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
I'm not really sure if this is a good approach.
But bounding the +1 value to .runs_script does not look to me good
either.

 doc/developers/writing_tests.rst | 2 +-
 include/tst_test.h               | 5 ++++-
 lib/tst_test.c                   | 9 +++++++--
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/doc/developers/writing_tests.rst b/doc/developers/writing_tests.rst
index 9b18ec059c..f5796ddc49 100644
--- a/doc/developers/writing_tests.rst
+++ b/doc/developers/writing_tests.rst
@@ -521,7 +521,7 @@ LTP C And Shell Test API Comparison
     * - not applicable
       - TST_NEEDS_MODULE
 
-    * - not applicable
+    * - .pos_args (internal use for tst_run_shell.c)
       - TST_POS_ARGS
 
     * - not applicable
diff --git a/include/tst_test.h b/include/tst_test.h
index eb73cd593c..b249f833ab 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -292,8 +292,11 @@ struct tst_fs {
  *
  * @tcnt: A number of tests. If set the test() callback is called tcnt times
  *        and each time passed an increasing counter value.
+ *
  * @options: An NULL optstr terminated array of struct tst_option.
  *
+ * @pos_args: An number of positional parameters passed to tst_run_shell.c.
+ *
  * @min_kver: A minimal kernel version the test can run on. e.g. "3.10".
  *
  * @supported_archs: A NULL terminated array of architectures the test runs on
@@ -528,6 +531,7 @@ struct tst_fs {
 	unsigned int tcnt;
 
 	struct tst_option *options;
+	int pos_args;
 
 	const char *min_kver;
 
@@ -555,7 +559,6 @@ struct tst_fs {
 	unsigned int skip_in_secureboot:1;
 	unsigned int skip_in_compat:1;
 
-
 	int needs_abi_bits;
 
 	unsigned int needs_hugetlbfs:1;
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 3823ea109e..1c2cc5e3b2 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -711,6 +711,9 @@ static void parse_opts(int argc, char *argv[])
 
 	check_option_collision();
 
+	if (tst_test->pos_args < 0)
+		tst_brk(TBROK, ".pos_args must be >= 0");
+
 	optstr[0] = 0;
 
 	for (i = 0; i < ARRAY_SIZE(options); i++)
@@ -751,8 +754,10 @@ static void parse_opts(int argc, char *argv[])
 		}
 	}
 
-	if (optind < argc)
-		tst_brk(TBROK, "Unexpected argument(s) '%s'...", argv[optind]);
+	if (optind + tst_test->pos_args < argc) {
+		tst_brk(TBROK, "Unexpected argument(s) '%s' (%d + %d < %d)",
+			argv[optind], optind, tst_test->pos_args, argc);
+	}
 }
 
 int tst_parse_int(const char *str, int *val, int min, int max)
-- 
2.47.2


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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [LTP] [RFC PATCH 3/5] shell: Move shell code into functions
  2025-02-28 17:24 [LTP] [RFC PATCH 0/5] shell loader rewrite to support TST_SETUP Petr Vorel
  2025-02-28 17:24 ` [LTP] [RFC PATCH 1/5] shell lib: Add support for test cleanup Petr Vorel
  2025-02-28 17:24 ` [LTP] [RFC PATCH 2/5] lib: Allow test to have positional args Petr Vorel
@ 2025-02-28 17:24 ` Petr Vorel
  2025-02-28 17:45   ` Petr Vorel
  2025-02-28 17:24 ` [LTP] [RFC PATCH 4/5] shell lib: Add basic support for test cleanup Petr Vorel
  2025-02-28 17:24 ` [LTP] [RFC PATCH 5/5] shell: Add shell_loader_setup_cleanup.sh test Petr Vorel
  4 siblings, 1 reply; 22+ messages in thread
From: Petr Vorel @ 2025-02-28 17:24 UTC (permalink / raw)
  To: ltp

This is a preparation to make next changes smaller.
No functional changes.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
nit: maybe use local + lower case variables in vma05.sh?

 testcases/kernel/mem/vma/vma05.sh             | 45 ++++++++++---------
 testcases/lib/tests/shell_loader.sh           | 19 +++++---
 .../lib/tests/shell_loader_all_filesystems.sh | 26 ++++++-----
 .../lib/tests/shell_loader_brk_cleanup.sh     |  7 ++-
 testcases/lib/tests/shell_loader_c_child.sh   | 15 ++++---
 testcases/lib/tests/shell_loader_cleanup.sh   |  7 ++-
 .../lib/tests/shell_loader_filesystems.sh     | 23 ++++++----
 .../lib/tests/shell_loader_invalid_block.sh   |  7 ++-
 .../tests/shell_loader_invalid_metadata.sh    |  7 ++-
 testcases/lib/tests/shell_loader_kconfigs.sh  |  7 ++-
 .../lib/tests/shell_loader_no_metadata.sh     |  7 ++-
 .../lib/tests/shell_loader_supported_archs.sh |  7 ++-
 testcases/lib/tests/shell_loader_tags.sh      |  7 ++-
 testcases/lib/tests/shell_loader_tcnt.sh      |  7 ++-
 .../lib/tests/shell_loader_wrong_metadata.sh  |  7 ++-
 15 files changed, 137 insertions(+), 61 deletions(-)

diff --git a/testcases/kernel/mem/vma/vma05.sh b/testcases/kernel/mem/vma/vma05.sh
index f4c76b7034..11d6b2ad86 100755
--- a/testcases/kernel/mem/vma/vma05.sh
+++ b/testcases/kernel/mem/vma/vma05.sh
@@ -41,29 +41,34 @@
 
 . tst_loader.sh
 
-ulimit -c unlimited
-unset DEBUGINFOD_URLS
+tst_test()
+{
+	ulimit -c unlimited
+	unset DEBUGINFOD_URLS
 
-if [ $(uname -m) = "x86_64" ]; then
-	if LINE=$(grep "vsyscall" /proc/self/maps); then
-		RIGHT="ffffffffff600000-ffffffffff601000[[:space:]][r-]-xp"
-		if echo "$LINE" | grep -q "$RIGHT"; then
-			tst_res TPASS "[vsyscall] reported correctly"
-		else
-			tst_res TFAIL "[vsyscall] reporting wrong"
+	if [ $(uname -m) = "x86_64" ]; then
+		if LINE=$(grep "vsyscall" /proc/self/maps); then
+			RIGHT="ffffffffff600000-ffffffffff601000[[:space:]][r-]-xp"
+			if echo "$LINE" | grep -q "$RIGHT"; then
+				tst_res TPASS "[vsyscall] reported correctly"
+			else
+				tst_res TFAIL "[vsyscall] reporting wrong"
+			fi
 		fi
 	fi
-fi
 
-rm -rf core*
-{ vma05_vdso; } > /dev/null 2>&1
-[ -f core ] || tst_brk TBROK "missing core file"
+	rm -rf core*
+	{ vma05_vdso; } > /dev/null 2>&1
+	[ -f core ] || tst_brk TBROK "missing core file"
 
-TRACE=$(gdb -silent -ex="thread apply all backtrace" -ex="quit"\
-	vma05_vdso ./core* 2> /dev/null)
+	TRACE=$(gdb -silent -ex="thread apply all backtrace" -ex="quit"\
+		vma05_vdso ./core* 2> /dev/null)
 
-if echo "$TRACE" | grep -qF "??"; then
-	tst_res TFAIL "[vdso] bug not patched"
-else
-	tst_res TPASS "[vdso] backtrace complete"
-fi
+	if echo "$TRACE" | grep -qF "??"; then
+		tst_res TFAIL "[vdso] bug not patched"
+	else
+		tst_res TPASS "[vdso] backtrace complete"
+	fi
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader.sh b/testcases/lib/tests/shell_loader.sh
index a7c5848ff5..73812c3e23 100755
--- a/testcases/lib/tests/shell_loader.sh
+++ b/testcases/lib/tests/shell_loader.sh
@@ -16,10 +16,15 @@
 
 . tst_loader.sh
 
-tst_res TPASS "Shell loader works fine!"
-case "$PWD" in
-	/tmp/*)
-		tst_res TPASS "We are running in temp directory in $PWD";;
-	*)
-		tst_res TFAIL "We are not running in temp directory but $PWD";;
-esac
+tst_test()
+{
+	tst_res TPASS "Shell loader works fine!"
+	case "$PWD" in
+		/tmp/*)
+			tst_res TPASS "We are running in temp directory in $PWD";;
+		*)
+			tst_res TFAIL "We are not running in temp directory but $PWD";;
+	esac
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader_all_filesystems.sh b/testcases/lib/tests/shell_loader_all_filesystems.sh
index 91fac89fd6..33c73dfb41 100755
--- a/testcases/lib/tests/shell_loader_all_filesystems.sh
+++ b/testcases/lib/tests/shell_loader_all_filesystems.sh
@@ -14,16 +14,22 @@
 
 . tst_loader.sh
 
-tst_res TINFO "In shell"
+tst_test()
+{
+	local mntpath=$(realpath ltp_mntpoint)
+	local mounted=$(grep $mntpath /proc/mounts)
+	local device path
 
-mntpath=$(realpath ltp_mntpoint)
-mounted=$(grep $mntpath /proc/mounts)
+	tst_res TINFO "In shell"
 
-if [ -n "$mounted" ]; then
-	device=$(echo $mounted |cut -d' ' -f 1)
-	path=$(echo $mounted |cut -d' ' -f 2)
+	if [ -n "$mounted" ]; then
+		device=$(echo $mounted |cut -d' ' -f 1)
+		path=$(echo $mounted |cut -d' ' -f 2)
 
-	tst_res TPASS "$device mounted at $path"
-else
-	tst_res TFAIL "Device not mounted!"
-fi
+		tst_res TPASS "$device mounted at $path"
+	else
+		tst_res TFAIL "Device not mounted!"
+	fi
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader_brk_cleanup.sh b/testcases/lib/tests/shell_loader_brk_cleanup.sh
index 8c704a5406..ff33345ce3 100755
--- a/testcases/lib/tests/shell_loader_brk_cleanup.sh
+++ b/testcases/lib/tests/shell_loader_brk_cleanup.sh
@@ -17,4 +17,9 @@ cleanup()
 	tst_res TINFO "Cleanup runs"
 }
 
-tst_brk TBROK "Test exits"
+tst_test()
+{
+	tst_brk TBROK "Test exits"
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader_c_child.sh b/testcases/lib/tests/shell_loader_c_child.sh
index 34629e6d26..b2b8f3d057 100755
--- a/testcases/lib/tests/shell_loader_c_child.sh
+++ b/testcases/lib/tests/shell_loader_c_child.sh
@@ -15,9 +15,14 @@
 
 . tst_loader.sh
 
-if [ -n "LTP_IPC_PATH" ]; then
-	tst_res TPASS "LTP_IPC_PATH=$LTP_IPC_PATH!"
-fi
+tst_test()
+{
+	if [ -n "LTP_IPC_PATH" ]; then
+		tst_res TPASS "LTP_IPC_PATH=$LTP_IPC_PATH!"
+	fi
 
-tst_res TINFO "Running C child"
-shell_c_child
+	tst_res TINFO "Running C child"
+	shell_c_child
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader_cleanup.sh b/testcases/lib/tests/shell_loader_cleanup.sh
index fb7bbdf5a9..684901b51f 100755
--- a/testcases/lib/tests/shell_loader_cleanup.sh
+++ b/testcases/lib/tests/shell_loader_cleanup.sh
@@ -17,4 +17,9 @@ do_cleanup()
 	tst_res TINFO "Cleanup executed"
 }
 
-tst_res TPASS "Test is executed"
+tst_test()
+{
+	tst_res TPASS "Test is executed"
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader_filesystems.sh b/testcases/lib/tests/shell_loader_filesystems.sh
index fe61708004..155cc4d9f6 100755
--- a/testcases/lib/tests/shell_loader_filesystems.sh
+++ b/testcases/lib/tests/shell_loader_filesystems.sh
@@ -21,15 +21,20 @@
 
 . tst_loader.sh
 
-tst_res TINFO "In shell"
+tst_test()
+{
+	tst_res TINFO "In shell"
 
-mntpoint=$(realpath ltp_mntpoint)
-mounted=$(grep $mntpoint /proc/mounts)
+	mntpoint=$(realpath ltp_mntpoint)
+	mounted=$(grep $mntpoint /proc/mounts)
 
-if [ -n "$mounted" ]; then
-	fs=$(echo $mounted |cut -d' ' -f 3)
+	if [ -n "$mounted" ]; then
+		fs=$(echo $mounted |cut -d' ' -f 3)
 
-	tst_res TPASS "Mounted device formatted with $fs"
-else
-	tst_res TFAIL "Device not mounted!"
-fi
+		tst_res TPASS "Mounted device formatted with $fs"
+	else
+		tst_res TFAIL "Device not mounted!"
+	fi
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader_invalid_block.sh b/testcases/lib/tests/shell_loader_invalid_block.sh
index 01811c971d..370c9043bc 100755
--- a/testcases/lib/tests/shell_loader_invalid_block.sh
+++ b/testcases/lib/tests/shell_loader_invalid_block.sh
@@ -22,4 +22,9 @@
 
 . tst_loader.sh
 
-tst_res TPASS "This should pass!"
+tst_test()
+{
+	tst_res TPASS "This should pass!"
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader_invalid_metadata.sh b/testcases/lib/tests/shell_loader_invalid_metadata.sh
index aeae066841..3834f1b9ed 100755
--- a/testcases/lib/tests/shell_loader_invalid_metadata.sh
+++ b/testcases/lib/tests/shell_loader_invalid_metadata.sh
@@ -14,4 +14,9 @@
 
 . tst_loader.sh
 
-tst_res TFAIL "Shell loader should TBROK the test"
+tst_test()
+{
+	tst_res TFAIL "Shell loader should TBROK the test"
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader_kconfigs.sh b/testcases/lib/tests/shell_loader_kconfigs.sh
index b896f03ce0..e1b6187554 100755
--- a/testcases/lib/tests/shell_loader_kconfigs.sh
+++ b/testcases/lib/tests/shell_loader_kconfigs.sh
@@ -11,4 +11,9 @@
 
 . tst_loader.sh
 
-tst_res TPASS "Shell loader works fine!"
+tst_test()
+{
+	tst_res TPASS "Shell loader works fine!"
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader_no_metadata.sh b/testcases/lib/tests/shell_loader_no_metadata.sh
index e344327ed3..b664b48b57 100755
--- a/testcases/lib/tests/shell_loader_no_metadata.sh
+++ b/testcases/lib/tests/shell_loader_no_metadata.sh
@@ -7,4 +7,9 @@
 
 . tst_loader.sh
 
-tst_res TFAIL "Shell loader should TBROK the test"
+tst_test()
+{
+	tst_res TFAIL "Shell loader should TBROK the test"
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader_supported_archs.sh b/testcases/lib/tests/shell_loader_supported_archs.sh
index 45f0b1b1c2..9ad24f9c03 100755
--- a/testcases/lib/tests/shell_loader_supported_archs.sh
+++ b/testcases/lib/tests/shell_loader_supported_archs.sh
@@ -11,4 +11,9 @@
 
 . tst_loader.sh
 
-tst_res TPASS "We are running on supported architecture"
+tst_test()
+{
+	tst_res TPASS "We are running on supported architecture"
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader_tags.sh b/testcases/lib/tests/shell_loader_tags.sh
index 0b9416ea9a..c780a66c57 100755
--- a/testcases/lib/tests/shell_loader_tags.sh
+++ b/testcases/lib/tests/shell_loader_tags.sh
@@ -14,4 +14,9 @@
 
 . tst_loader.sh
 
-tst_res TFAIL "Fails the test so that tags are shown."
+tst_test()
+{
+	tst_res TFAIL "Fails the test so that tags are shown."
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader_tcnt.sh b/testcases/lib/tests/shell_loader_tcnt.sh
index ecf48396d6..93bd612ee2 100755
--- a/testcases/lib/tests/shell_loader_tcnt.sh
+++ b/testcases/lib/tests/shell_loader_tcnt.sh
@@ -14,4 +14,9 @@
 
 . tst_loader.sh
 
-tst_res TPASS "Iteration $1"
+tst_test()
+{
+	tst_res TPASS "Iteration $1"
+}
+
+tst_test
diff --git a/testcases/lib/tests/shell_loader_wrong_metadata.sh b/testcases/lib/tests/shell_loader_wrong_metadata.sh
index b90b212371..8f18741100 100755
--- a/testcases/lib/tests/shell_loader_wrong_metadata.sh
+++ b/testcases/lib/tests/shell_loader_wrong_metadata.sh
@@ -14,4 +14,9 @@
 
 . tst_loader.sh
 
-tst_res TFAIL "Shell loader should TBROK the test"
+tst_test()
+{
+	tst_res TFAIL "Shell loader should TBROK the test"
+}
+
+tst_test
-- 
2.47.2


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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [LTP] [RFC PATCH 4/5] shell lib: Add basic support for test cleanup
  2025-02-28 17:24 [LTP] [RFC PATCH 0/5] shell loader rewrite to support TST_SETUP Petr Vorel
                   ` (2 preceding siblings ...)
  2025-02-28 17:24 ` [LTP] [RFC PATCH 3/5] shell: Move shell code into functions Petr Vorel
@ 2025-02-28 17:24 ` Petr Vorel
  2025-03-07 16:40   ` Cyril Hrubis
  2025-02-28 17:24 ` [LTP] [RFC PATCH 5/5] shell: Add shell_loader_setup_cleanup.sh test Petr Vorel
  4 siblings, 1 reply; 22+ messages in thread
From: Petr Vorel @ 2025-02-28 17:24 UTC (permalink / raw)
  To: ltp

Add basic support for test cleanup in shell loader.

This solves the problem of the order the scripts are sourced. Before it
was:

    test.sh
     . tst_loader.sh
      tst_run_shell test.sh
       . tst_loader.sh
	. tst_env.sh <- at this point in the execution you haven't even started
			parsing test.sh so you cannot run functions from there
			at all

Now new script tst_exec.sh is used to load load the test (loaded itself
by tst_run_shell):

    test.sh
     . tst_loader.sh
     tst_run_shell tst_exec.sh test.sh
      . test.sh
	. tst_env.sh

'. tst_loader.sh' (loading a script) was moved to the end, calling test
function (now forced to be tst_test) and tst_env.sh was moved to
tst_exec.sh.

There will be more improvements in the future, at least adding TST_CNT
support (will require changes in tst_test.c to handle timeouts).

Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Co-developed-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Maybe variables in tst_run_shell.c could be renamed?

 testcases/kernel/mem/vma/vma05.sh             |  4 +---
 testcases/lib/tests/shell_loader.sh           |  4 +---
 .../lib/tests/shell_loader_all_filesystems.sh |  4 +---
 .../lib/tests/shell_loader_brk_cleanup.sh     |  4 +---
 testcases/lib/tests/shell_loader_c_child.sh   |  4 +---
 testcases/lib/tests/shell_loader_cleanup.sh   |  4 +---
 .../lib/tests/shell_loader_filesystems.sh     |  4 +---
 .../lib/tests/shell_loader_invalid_block.sh   |  4 +---
 .../tests/shell_loader_invalid_metadata.sh    |  4 +---
 testcases/lib/tests/shell_loader_kconfigs.sh  |  4 +---
 .../lib/tests/shell_loader_no_metadata.sh     |  4 +---
 .../lib/tests/shell_loader_supported_archs.sh |  4 +---
 testcases/lib/tests/shell_loader_tags.sh      |  4 +---
 testcases/lib/tests/shell_loader_tcnt.sh      |  4 +---
 .../lib/tests/shell_loader_wrong_metadata.sh  |  4 +---
 testcases/lib/tst_env.sh                      |  4 ----
 testcases/lib/tst_exec.sh                     | 19 +++++++++++++++++++
 testcases/lib/tst_loader.sh                   |  5 +----
 testcases/lib/tst_run_shell.c                 | 16 +++++++++++-----
 19 files changed, 46 insertions(+), 58 deletions(-)
 create mode 100755 testcases/lib/tst_exec.sh

diff --git a/testcases/kernel/mem/vma/vma05.sh b/testcases/kernel/mem/vma/vma05.sh
index 11d6b2ad86..5f8940581b 100755
--- a/testcases/kernel/mem/vma/vma05.sh
+++ b/testcases/kernel/mem/vma/vma05.sh
@@ -39,8 +39,6 @@
 # }
 # ---
 
-. tst_loader.sh
-
 tst_test()
 {
 	ulimit -c unlimited
@@ -71,4 +69,4 @@ tst_test()
 	fi
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader.sh b/testcases/lib/tests/shell_loader.sh
index 73812c3e23..eeed122c1e 100755
--- a/testcases/lib/tests/shell_loader.sh
+++ b/testcases/lib/tests/shell_loader.sh
@@ -14,8 +14,6 @@
 # }
 # ---
 
-. tst_loader.sh
-
 tst_test()
 {
 	tst_res TPASS "Shell loader works fine!"
@@ -27,4 +25,4 @@ tst_test()
 	esac
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader_all_filesystems.sh b/testcases/lib/tests/shell_loader_all_filesystems.sh
index 33c73dfb41..caa4c75ff4 100755
--- a/testcases/lib/tests/shell_loader_all_filesystems.sh
+++ b/testcases/lib/tests/shell_loader_all_filesystems.sh
@@ -12,8 +12,6 @@
 # }
 # ---
 
-. tst_loader.sh
-
 tst_test()
 {
 	local mntpath=$(realpath ltp_mntpoint)
@@ -32,4 +30,4 @@ tst_test()
 	fi
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader_brk_cleanup.sh b/testcases/lib/tests/shell_loader_brk_cleanup.sh
index ff33345ce3..c6286176c5 100755
--- a/testcases/lib/tests/shell_loader_brk_cleanup.sh
+++ b/testcases/lib/tests/shell_loader_brk_cleanup.sh
@@ -10,8 +10,6 @@
 
 TST_CLEANUP=cleanup
 
-. tst_loader.sh
-
 cleanup()
 {
 	tst_res TINFO "Cleanup runs"
@@ -22,4 +20,4 @@ tst_test()
 	tst_brk TBROK "Test exits"
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader_c_child.sh b/testcases/lib/tests/shell_loader_c_child.sh
index b2b8f3d057..9acc212145 100755
--- a/testcases/lib/tests/shell_loader_c_child.sh
+++ b/testcases/lib/tests/shell_loader_c_child.sh
@@ -13,8 +13,6 @@
 # }
 # ---
 
-. tst_loader.sh
-
 tst_test()
 {
 	if [ -n "LTP_IPC_PATH" ]; then
@@ -25,4 +23,4 @@ tst_test()
 	shell_c_child
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader_cleanup.sh b/testcases/lib/tests/shell_loader_cleanup.sh
index 684901b51f..a59301a15b 100755
--- a/testcases/lib/tests/shell_loader_cleanup.sh
+++ b/testcases/lib/tests/shell_loader_cleanup.sh
@@ -10,8 +10,6 @@
 
 TST_CLEANUP=do_cleanup
 
-. tst_loader.sh
-
 do_cleanup()
 {
 	tst_res TINFO "Cleanup executed"
@@ -22,4 +20,4 @@ tst_test()
 	tst_res TPASS "Test is executed"
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader_filesystems.sh b/testcases/lib/tests/shell_loader_filesystems.sh
index 155cc4d9f6..c23590d26a 100755
--- a/testcases/lib/tests/shell_loader_filesystems.sh
+++ b/testcases/lib/tests/shell_loader_filesystems.sh
@@ -19,8 +19,6 @@
 # }
 # ---
 
-. tst_loader.sh
-
 tst_test()
 {
 	tst_res TINFO "In shell"
@@ -37,4 +35,4 @@ tst_test()
 	fi
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader_invalid_block.sh b/testcases/lib/tests/shell_loader_invalid_block.sh
index 370c9043bc..7df25c95dd 100755
--- a/testcases/lib/tests/shell_loader_invalid_block.sh
+++ b/testcases/lib/tests/shell_loader_invalid_block.sh
@@ -20,11 +20,9 @@
 # This is an invalid block that breaks the test.
 # ---
 
-. tst_loader.sh
-
 tst_test()
 {
 	tst_res TPASS "This should pass!"
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader_invalid_metadata.sh b/testcases/lib/tests/shell_loader_invalid_metadata.sh
index 3834f1b9ed..2b7f86c5b5 100755
--- a/testcases/lib/tests/shell_loader_invalid_metadata.sh
+++ b/testcases/lib/tests/shell_loader_invalid_metadata.sh
@@ -12,11 +12,9 @@
 # ---
 #
 
-. tst_loader.sh
-
 tst_test()
 {
 	tst_res TFAIL "Shell loader should TBROK the test"
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader_kconfigs.sh b/testcases/lib/tests/shell_loader_kconfigs.sh
index e1b6187554..8fdf3e16c5 100755
--- a/testcases/lib/tests/shell_loader_kconfigs.sh
+++ b/testcases/lib/tests/shell_loader_kconfigs.sh
@@ -9,11 +9,9 @@
 # }
 # ---
 
-. tst_loader.sh
-
 tst_test()
 {
 	tst_res TPASS "Shell loader works fine!"
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader_no_metadata.sh b/testcases/lib/tests/shell_loader_no_metadata.sh
index b664b48b57..4c55afabfe 100755
--- a/testcases/lib/tests/shell_loader_no_metadata.sh
+++ b/testcases/lib/tests/shell_loader_no_metadata.sh
@@ -5,11 +5,9 @@
 # This test has no metadata and should not be executed
 #
 
-. tst_loader.sh
-
 tst_test()
 {
 	tst_res TFAIL "Shell loader should TBROK the test"
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader_supported_archs.sh b/testcases/lib/tests/shell_loader_supported_archs.sh
index 9ad24f9c03..99c387d248 100755
--- a/testcases/lib/tests/shell_loader_supported_archs.sh
+++ b/testcases/lib/tests/shell_loader_supported_archs.sh
@@ -9,11 +9,9 @@
 # }
 # ---
 
-. tst_loader.sh
-
 tst_test()
 {
 	tst_res TPASS "We are running on supported architecture"
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader_tags.sh b/testcases/lib/tests/shell_loader_tags.sh
index c780a66c57..b8f78b61b4 100755
--- a/testcases/lib/tests/shell_loader_tags.sh
+++ b/testcases/lib/tests/shell_loader_tags.sh
@@ -12,11 +12,9 @@
 # }
 # ---
 
-. tst_loader.sh
-
 tst_test()
 {
 	tst_res TFAIL "Fails the test so that tags are shown."
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader_tcnt.sh b/testcases/lib/tests/shell_loader_tcnt.sh
index 93bd612ee2..c29c0558d2 100755
--- a/testcases/lib/tests/shell_loader_tcnt.sh
+++ b/testcases/lib/tests/shell_loader_tcnt.sh
@@ -12,11 +12,9 @@
 # ---
 #
 
-. tst_loader.sh
-
 tst_test()
 {
 	tst_res TPASS "Iteration $1"
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tests/shell_loader_wrong_metadata.sh b/testcases/lib/tests/shell_loader_wrong_metadata.sh
index 8f18741100..cf5e577947 100755
--- a/testcases/lib/tests/shell_loader_wrong_metadata.sh
+++ b/testcases/lib/tests/shell_loader_wrong_metadata.sh
@@ -12,11 +12,9 @@
 # ---
 #
 
-. tst_loader.sh
-
 tst_test()
 {
 	tst_res TFAIL "Shell loader should TBROK the test"
 }
 
-tst_test
+. tst_loader.sh
diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh
index b13bab37c3..68f9a0daa9 100644
--- a/testcases/lib/tst_env.sh
+++ b/testcases/lib/tst_env.sh
@@ -35,7 +35,3 @@ tst_brk_()
 
 alias tst_res="tst_res_ $tst_script_name \$LINENO"
 alias tst_brk="tst_brk_ $tst_script_name \$LINENO"
-
-if [ -n "$TST_CLEANUP" ]; then
-	trap $TST_CLEANUP EXIT
-fi
diff --git a/testcases/lib/tst_exec.sh b/testcases/lib/tst_exec.sh
new file mode 100755
index 0000000000..dcf40fd5bb
--- /dev/null
+++ b/testcases/lib/tst_exec.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+# Copyright (c) 2025 Cyril Hrubis <chrubis@suse.cz>
+# Copyright (c) 2025 Petr Vorel <pvorel@suse.cz>
+
+. tst_env.sh
+
+. "$1"
+
+if [ -n "$TST_CLEANUP" ]; then
+	trap $TST_CLEANUP EXIT
+fi
+
+if [ -n "$TST_SETUP" ]; then
+    $TST_SETUP
+fi
+
+tst_test
+
+# vim: set ft=sh ts=4 sts=4 sw=4 expandtab :
diff --git a/testcases/lib/tst_loader.sh b/testcases/lib/tst_loader.sh
index 62c9cc6d8f..e2d1bd7daf 100644
--- a/testcases/lib/tst_loader.sh
+++ b/testcases/lib/tst_loader.sh
@@ -3,11 +3,8 @@
 # Copyright (c) 2024-2025 Cyril Hrubis <chrubis@suse.cz>
 #
 # This is a loader for shell tests that use the C test library.
-#
 
 if [ -z "$LTP_IPC_PATH" ]; then
-	tst_run_shell $(basename "$0") "$@"
+	tst_run_shell tst_exec.sh $(basename "$0") "$@"
 	exit $?
-else
-	. tst_env.sh
 fi
diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
index 7a446e0040..96e9bee4f5 100644
--- a/testcases/lib/tst_run_shell.c
+++ b/testcases/lib/tst_run_shell.c
@@ -9,30 +9,35 @@
 #include "tst_safe_stdio.h"
 #include "ujson.h"
 
+static char *shell_loader;
+
+/* shell test filename */
 static char *shell_filename;
 
 static void run_shell(void)
 {
-	tst_run_script(shell_filename, NULL);
+	char *const params[] = {shell_filename, NULL};
+	tst_run_script(shell_loader, params);
 }
 
 static void run_shell_tcnt(unsigned int n)
 {
 	char buf[128];
-	char *const params[] = {buf, NULL};
+	char *const params[] = {shell_filename, buf, NULL};
 
 	snprintf(buf, sizeof(buf), "%u", n);
 
-	tst_run_script(shell_filename, params);
+	tst_run_script(shell_loader, params);
 }
 
 static struct tst_test test = {
 	.runs_script = 1,
+	.pos_args = 1,
 };
 
 static void print_help(void)
 {
-	printf("Usage: tst_shell_loader ltp_shell_test.sh ...\n");
+	printf("Usage: tst_run_shell tst_exec.sh ltp_shell_test.sh ...\n");
 }
 
 static char *metadata;
@@ -589,7 +594,8 @@ int main(int argc, char *argv[])
 	if (argc < 2)
 		goto help;
 
-	shell_filename = argv[1];
+	shell_loader = argv[1];
+	shell_filename = argv[2];
 
 	prepare_test_struct();
 
-- 
2.47.2


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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [LTP] [RFC PATCH 5/5] shell: Add shell_loader_setup_cleanup.sh test
  2025-02-28 17:24 [LTP] [RFC PATCH 0/5] shell loader rewrite to support TST_SETUP Petr Vorel
                   ` (3 preceding siblings ...)
  2025-02-28 17:24 ` [LTP] [RFC PATCH 4/5] shell lib: Add basic support for test cleanup Petr Vorel
@ 2025-02-28 17:24 ` Petr Vorel
  4 siblings, 0 replies; 22+ messages in thread
From: Petr Vorel @ 2025-02-28 17:24 UTC (permalink / raw)
  To: ltp

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/run_tests.sh                    |  1 +
 .../lib/tests/shell_loader_setup_cleanup.sh   | 29 +++++++++++++++++++
 2 files changed, 30 insertions(+)
 create mode 100755 testcases/lib/tests/shell_loader_setup_cleanup.sh

diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
index 128cee3377..5c309bbeb5 100755
--- a/testcases/lib/run_tests.sh
+++ b/testcases/lib/run_tests.sh
@@ -10,6 +10,7 @@ shell_loader_kconfigs.sh
 shell_loader_supported_archs.sh
 shell_loader_tcnt.sh
 shell_loader_cleanup.sh
+shell_loader_setup_cleanup.sh
 shell_test01
 shell_test02
 shell_test03
diff --git a/testcases/lib/tests/shell_loader_setup_cleanup.sh b/testcases/lib/tests/shell_loader_setup_cleanup.sh
new file mode 100755
index 0000000000..6a065922b0
--- /dev/null
+++ b/testcases/lib/tests/shell_loader_setup_cleanup.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2025 Petr Vorel <pvorel@suse.cz>
+#
+# ---
+# env
+# {
+# }
+# ---
+
+TST_SETUP=setup
+TST_CLEANUP=cleanup
+
+setup()
+{
+	tst_res TINFO "setup executed"
+}
+
+cleanup()
+{
+	tst_res TINFO "Cleanup executed"
+}
+
+tst_test()
+{
+	tst_res TPASS "Test is executed"
+}
+
+. tst_loader.sh
-- 
2.47.2


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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 2/5] lib: Allow test to have positional args
  2025-02-28 17:24 ` [LTP] [RFC PATCH 2/5] lib: Allow test to have positional args Petr Vorel
@ 2025-02-28 17:43   ` Petr Vorel
  2025-03-07 16:17   ` Cyril Hrubis
  2025-03-07 16:27   ` Cyril Hrubis
  2 siblings, 0 replies; 22+ messages in thread
From: Petr Vorel @ 2025-02-28 17:43 UTC (permalink / raw)
  To: ltp

Hi,

> Similar to $TST_POS_ARGS in shell API. This will be used in
> testcases/lib/tst_run_shell.c.

OK this does not work for ash (busybox sh on Alpine):
https://github.com/pevik/ltp/actions/runs/13593238811/job/38004158999

*** Testing LTP test -h option ***
*** Running 'shell_loader.sh -h' (exp: TPASS) ***
tst_test.c:758: TBROK: Unexpected argument(s) 'shell_loader.sh' (1 + 1 < 3)

*** Testing LTP test -i option ***
tst_test.c:758: TBROK: Unexpected argument(s) 'shell_loader.sh' (1 + 1 < 4)

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 3/5] shell: Move shell code into functions
  2025-02-28 17:24 ` [LTP] [RFC PATCH 3/5] shell: Move shell code into functions Petr Vorel
@ 2025-02-28 17:45   ` Petr Vorel
  0 siblings, 0 replies; 22+ messages in thread
From: Petr Vorel @ 2025-02-28 17:45 UTC (permalink / raw)
  To: ltp

Hi all,

...
> diff --git a/testcases/lib/tests/shell_loader_all_filesystems.sh b/testcases/lib/tests/shell_loader_all_filesystems.sh
> index 91fac89fd6..33c73dfb41 100755
> --- a/testcases/lib/tests/shell_loader_all_filesystems.sh
> +++ b/testcases/lib/tests/shell_loader_all_filesystems.sh
> @@ -14,16 +14,22 @@

>  . tst_loader.sh

> -tst_res TINFO "In shell"
> +tst_test()
> +{
> +	local mntpath=$(realpath ltp_mntpoint)

And this is failing on old dash on Ubuntu Bionic:
https://github.com/pevik/ltp/actions/runs/13593238811/job/38004161850
I need to separate local declaration from adding value.

Kind regards,
Petr

*** Running 'shell_loader_all_filesystems.sh' (exp: TPASS) ***
tst_tmpdir.c:317: TINFO: Using /tmp/LTP_tstr4SvTK as tmpdir (overlayfs filesystem)
tst_device.c:98: TINFO: Found free device 0 '/dev/loop0'
tst_test.c:1905: TINFO: LTP version: 20250130
tst_test.c:1909: TINFO: Tested kernel: 6.8.0-1021-azure #25-Ubuntu SMP Wed Jan 15 20:45:09 UTC 2025 x86_64
tst_kconfig.c:88: TINFO: Parsing kernel config '/boot/config-6.8.0-1021-azure'
tst_test.c:1729: TINFO: Overall timeout per run is 0h 00m 30s
tst_supported_fs_types.c:97: TINFO: Kernel supports ext2
tst_supported_fs_types.c:62: TINFO: mkfs.ext2 does exist
tst_supported_fs_types.c:97: TINFO: Kernel supports ext3
tst_supported_fs_types.c:62: TINFO: mkfs.ext3 does exist
tst_supported_fs_types.c:97: TINFO: Kernel supports ext4
tst_supported_fs_types.c:62: TINFO: mkfs.ext4 does exist
tst_supported_fs_types.c:97: TINFO: Kernel supports xfs
tst_supported_fs_types.c:58: TINFO: mkfs.xfs does not exist
tst_supported_fs_types.c:97: TINFO: Kernel supports btrfs
tst_supported_fs_types.c:58: TINFO: mkfs.btrfs does not exist
tst_supported_fs_types.c:105: TINFO: Skipping bcachefs because of FUSE blacklist
tst_supported_fs_types.c:97: TINFO: Kernel supports vfat
tst_supported_fs_types.c:58: TINFO: mkfs.vfat does not exist
tst_supported_fs_types.c:128: TINFO: Filesystem exfat is not supported
tst_supported_fs_types.c:128: TINFO: Filesystem ntfs is not supported
tst_supported_fs_types.c:97: TINFO: Kernel supports tmpfs
tst_supported_fs_types.c:49: TINFO: mkfs is not needed for tmpfs
tst_test.c:1838: TINFO: === Testing on ext2 ===
tst_test.c:1175: TINFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
mke2fs 1.44.1 (24-Mar-2018)
tst_test.c:1188: TINFO: Mounting /dev/loop0 to /tmp/LTP_tstr4SvTK/ltp_mntpoint fstyp=ext2 flags=0
/__w/ltp/ltp/testcases/lib/tst_exec.sh: 18: local: /tmp/LTP_tstr4SvTK/ltp_mntpoint: bad variable name
tst_test.c:1564: TBROK: Test haven't reported results!

> +	local mounted=$(grep $mntpath /proc/mounts)
> +	local device path

> -mntpath=$(realpath ltp_mntpoint)
> -mounted=$(grep $mntpath /proc/mounts)
> +	tst_res TINFO "In shell"

> -if [ -n "$mounted" ]; then
> -	device=$(echo $mounted |cut -d' ' -f 1)
> -	path=$(echo $mounted |cut -d' ' -f 2)
> +	if [ -n "$mounted" ]; then
> +		device=$(echo $mounted |cut -d' ' -f 1)
> +		path=$(echo $mounted |cut -d' ' -f 2)

> -	tst_res TPASS "$device mounted at $path"
> -else
> -	tst_res TFAIL "Device not mounted!"
> -fi
> +		tst_res TPASS "$device mounted at $path"
> +	else
> +		tst_res TFAIL "Device not mounted!"
> +	fi
> +}
> +
> +tst_test

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 1/5] shell lib: Add support for test cleanup
  2025-02-28 17:24 ` [LTP] [RFC PATCH 1/5] shell lib: Add support for test cleanup Petr Vorel
@ 2025-03-04 12:57   ` Li Wang
  2025-03-04 13:08     ` Petr Vorel
  0 siblings, 1 reply; 22+ messages in thread
From: Li Wang @ 2025-03-04 12:57 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

On Sat, Mar 1, 2025 at 1:25 AM Petr Vorel <pvorel@suse.cz> wrote:

> From: Cyril Hrubis <chrubis@suse.cz>
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
> The same as in:
>
> https://patchwork.ozlabs.org/project/ltp/patch/20250214112135.18947-3-chrubis@suse.cz/
>
>  testcases/lib/run_tests.sh                    |  4 +++-
>  .../lib/tests/shell_loader_brk_cleanup.sh     | 20 +++++++++++++++++++
>  testcases/lib/tests/shell_loader_cleanup.sh   | 20 +++++++++++++++++++
>  testcases/lib/tst_env.sh                      |  4 ++++
>  4 files changed, 47 insertions(+), 1 deletion(-)
>  create mode 100755 testcases/lib/tests/shell_loader_brk_cleanup.sh
>  create mode 100755 testcases/lib/tests/shell_loader_cleanup.sh
>
> diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
> index 321f74e5fe..128cee3377 100755
> --- a/testcases/lib/run_tests.sh
> +++ b/testcases/lib/run_tests.sh
> @@ -9,6 +9,7 @@ shell_loader_filesystems.sh
>  shell_loader_kconfigs.sh
>  shell_loader_supported_archs.sh
>  shell_loader_tcnt.sh
> +shell_loader_cleanup.sh
>  shell_test01
>  shell_test02
>  shell_test03
> @@ -21,7 +22,8 @@ TESTS_TBROK="
>  shell_loader_invalid_block.sh
>  shell_loader_invalid_metadata.sh
>  shell_loader_no_metadata.sh
> -shell_loader_wrong_metadata.sh"
> +shell_loader_wrong_metadata.sh
>

It seems the shell_loader_wrong_metadata.sh is a duplicate of
shell_loader_invalid_metadata. Maybe we can remove one of them.

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



> +shell_loader_brk_cleanup.sh"
>
>  TESTS_TCONF="shell_test06"
>
> diff --git a/testcases/lib/tests/shell_loader_brk_cleanup.sh
> b/testcases/lib/tests/shell_loader_brk_cleanup.sh
> new file mode 100755
> index 0000000000..8c704a5406
> --- /dev/null
> +++ b/testcases/lib/tests/shell_loader_brk_cleanup.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2024-2025 Cyril Hrubis <chrubis@suse.cz>
> +#
> +# ---
> +# env
> +# {
> +# }
> +# ---
> +
> +TST_CLEANUP=cleanup
> +
> +. tst_loader.sh
> +
> +cleanup()
> +{
> +       tst_res TINFO "Cleanup runs"
> +}
> +
> +tst_brk TBROK "Test exits"
> diff --git a/testcases/lib/tests/shell_loader_cleanup.sh
> b/testcases/lib/tests/shell_loader_cleanup.sh
> new file mode 100755
> index 0000000000..fb7bbdf5a9
> --- /dev/null
> +++ b/testcases/lib/tests/shell_loader_cleanup.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2024-2025 Cyril Hrubis <chrubis@suse.cz>
> +#
> +# ---
> +# env
> +# {
> +# }
> +# ---
> +
> +TST_CLEANUP=do_cleanup
> +
> +. tst_loader.sh
> +
> +do_cleanup()
> +{
> +       tst_res TINFO "Cleanup executed"
> +}
> +
> +tst_res TPASS "Test is executed"
> diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh
> index 68f9a0daa9..b13bab37c3 100644
> --- a/testcases/lib/tst_env.sh
> +++ b/testcases/lib/tst_env.sh
> @@ -35,3 +35,7 @@ tst_brk_()
>
>  alias tst_res="tst_res_ $tst_script_name \$LINENO"
>  alias tst_brk="tst_brk_ $tst_script_name \$LINENO"
> +
> +if [ -n "$TST_CLEANUP" ]; then
> +       trap $TST_CLEANUP EXIT
> +fi
> --
> 2.47.2
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 1/5] shell lib: Add support for test cleanup
  2025-03-04 12:57   ` Li Wang
@ 2025-03-04 13:08     ` Petr Vorel
  2025-03-04 13:15       ` Li Wang
  0 siblings, 1 reply; 22+ messages in thread
From: Petr Vorel @ 2025-03-04 13:08 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

Hi Li,

...
> > +++ b/testcases/lib/run_tests.sh
> > @@ -9,6 +9,7 @@ shell_loader_filesystems.sh
> >  shell_loader_kconfigs.sh
> >  shell_loader_supported_archs.sh
> >  shell_loader_tcnt.sh
> > +shell_loader_cleanup.sh
> >  shell_test01
> >  shell_test02
> >  shell_test03
> > @@ -21,7 +22,8 @@ TESTS_TBROK="
> >  shell_loader_invalid_block.sh
> >  shell_loader_invalid_metadata.sh
> >  shell_loader_no_metadata.sh
> > -shell_loader_wrong_metadata.sh"
> > +shell_loader_wrong_metadata.sh

> It seems the shell_loader_wrong_metadata.sh is a duplicate of
> shell_loader_invalid_metadata. Maybe we can remove one of them.

Good catch. But I think there are testing a different thing:

shell_loader_wrong_metadata.sh IMHO has too high int value
("Wrong 'needs_tmpdir' type expected boolean" error):

"needs_tmpdir": 42,

# PATH="$PWD:$PWD/tests:$PATH" shell_loader_wrong_metadata.sh
Parse error at line 002

001: {
002:  "needs_tmpdir": 42,
                        ^
Wrong 'needs_tmpdir' type expected boolean
tst_run_shell.c:508: TBROK: Invalid metadata

shell_loader_invalid_metadata.sh IMHO has invalid JSON
("Expected ID string" error):

{"needs_tmpdir": 42,

# PATH="$PWD:$PWD/tests:$PATH" shell_loader_invalid_metadata.sh
Parse error at line 002

001: {
002:  {"needs_tmpdir": 42,
      ^
Expected ID string
tst_run_shell.c:508: TBROK: Invalid metadata

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 1/5] shell lib: Add support for test cleanup
  2025-03-04 13:08     ` Petr Vorel
@ 2025-03-04 13:15       ` Li Wang
  0 siblings, 0 replies; 22+ messages in thread
From: Li Wang @ 2025-03-04 13:15 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

On Tue, Mar 4, 2025 at 9:08 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> ...
> > > +++ b/testcases/lib/run_tests.sh
> > > @@ -9,6 +9,7 @@ shell_loader_filesystems.sh
> > >  shell_loader_kconfigs.sh
> > >  shell_loader_supported_archs.sh
> > >  shell_loader_tcnt.sh
> > > +shell_loader_cleanup.sh
> > >  shell_test01
> > >  shell_test02
> > >  shell_test03
> > > @@ -21,7 +22,8 @@ TESTS_TBROK="
> > >  shell_loader_invalid_block.sh
> > >  shell_loader_invalid_metadata.sh
> > >  shell_loader_no_metadata.sh
> > > -shell_loader_wrong_metadata.sh"
> > > +shell_loader_wrong_metadata.sh
>
> > It seems the shell_loader_wrong_metadata.sh is a duplicate of
> > shell_loader_invalid_metadata. Maybe we can remove one of them.
>
> Good catch. But I think there are testing a different thing:


> shell_loader_wrong_metadata.sh IMHO has too high int value
> ("Wrong 'needs_tmpdir' type expected boolean" error):
>

Ah indeed, I hadn't noticed that, thank you!



> "needs_tmpdir": 42,
>
> # PATH="$PWD:$PWD/tests:$PATH" shell_loader_wrong_metadata.sh
> Parse error at line 002
>
> 001: {
> 002:  "needs_tmpdir": 42,
>                         ^
> Wrong 'needs_tmpdir' type expected boolean
> tst_run_shell.c:508: TBROK: Invalid metadata
>
> shell_loader_invalid_metadata.sh IMHO has invalid JSON
> ("Expected ID string" error):
>
> {"needs_tmpdir": 42,
>
> # PATH="$PWD:$PWD/tests:$PATH" shell_loader_invalid_metadata.sh
> Parse error at line 002
>
> 001: {
> 002:  {"needs_tmpdir": 42,
>       ^
> Expected ID string
> tst_run_shell.c:508: TBROK: Invalid metadata
>
> Kind regards,
> Petr
>
>

-- 
Regards,
Li Wang

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 2/5] lib: Allow test to have positional args
  2025-02-28 17:24 ` [LTP] [RFC PATCH 2/5] lib: Allow test to have positional args Petr Vorel
  2025-02-28 17:43   ` Petr Vorel
@ 2025-03-07 16:17   ` Cyril Hrubis
  2025-03-10 10:23     ` Petr Vorel
  2025-03-07 16:27   ` Cyril Hrubis
  2 siblings, 1 reply; 22+ messages in thread
From: Cyril Hrubis @ 2025-03-07 16:17 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi!
> diff --git a/doc/developers/writing_tests.rst b/doc/developers/writing_tests.rst
> index 9b18ec059c..f5796ddc49 100644
> --- a/doc/developers/writing_tests.rst
> +++ b/doc/developers/writing_tests.rst
> @@ -521,7 +521,7 @@ LTP C And Shell Test API Comparison
>      * - not applicable
>        - TST_NEEDS_MODULE
>  
> -    * - not applicable
> +    * - .pos_args (internal use for tst_run_shell.c)
>        - TST_POS_ARGS
>  
>      * - not applicable
> diff --git a/include/tst_test.h b/include/tst_test.h
> index eb73cd593c..b249f833ab 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -292,8 +292,11 @@ struct tst_fs {
>   *
>   * @tcnt: A number of tests. If set the test() callback is called tcnt times
>   *        and each time passed an increasing counter value.
> + *
>   * @options: An NULL optstr terminated array of struct tst_option.
>   *
> + * @pos_args: An number of positional parameters passed to tst_run_shell.c.

We do not support positional arguments for the C API. Do we really need
them for shell?

>   * @min_kver: A minimal kernel version the test can run on. e.g. "3.10".
>   *
>   * @supported_archs: A NULL terminated array of architectures the test runs on
> @@ -528,6 +531,7 @@ struct tst_fs {
>  	unsigned int tcnt;
>  
>  	struct tst_option *options;
> +	int pos_args;
>  
>  	const char *min_kver;
>  
> @@ -555,7 +559,6 @@ struct tst_fs {
>  	unsigned int skip_in_secureboot:1;
>  	unsigned int skip_in_compat:1;
>  
> -
>  	int needs_abi_bits;
>  
>  	unsigned int needs_hugetlbfs:1;
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 3823ea109e..1c2cc5e3b2 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -711,6 +711,9 @@ static void parse_opts(int argc, char *argv[])
>  
>  	check_option_collision();
>  
> +	if (tst_test->pos_args < 0)
> +		tst_brk(TBROK, ".pos_args must be >= 0");

You can declare pos_args as unsigned and you don't have to add this
condition.

>  	optstr[0] = 0;
>  
>  	for (i = 0; i < ARRAY_SIZE(options); i++)
> @@ -751,8 +754,10 @@ static void parse_opts(int argc, char *argv[])
>  		}
>  	}
>  
> -	if (optind < argc)
> -		tst_brk(TBROK, "Unexpected argument(s) '%s'...", argv[optind]);
> +	if (optind + tst_test->pos_args < argc) {
> +		tst_brk(TBROK, "Unexpected argument(s) '%s' (%d + %d < %d)",
> +			argv[optind], optind, tst_test->pos_args, argc);
> +	}

And this half enables the positional arguments for the C API as well. If
we set the pos_args in tst_test, then we can pass them, but there is no
way how they can be passed to the test.

So if we are going to add them, we should pass then in
extern char **tst_args or something like that.

>  }
>  
>  int tst_parse_int(const char *str, int *val, int min, int max)
> -- 
> 2.47.2
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 2/5] lib: Allow test to have positional args
  2025-02-28 17:24 ` [LTP] [RFC PATCH 2/5] lib: Allow test to have positional args Petr Vorel
  2025-02-28 17:43   ` Petr Vorel
  2025-03-07 16:17   ` Cyril Hrubis
@ 2025-03-07 16:27   ` Cyril Hrubis
  2025-03-10 10:16     ` Petr Vorel
  2 siblings, 1 reply; 22+ messages in thread
From: Cyril Hrubis @ 2025-03-07 16:27 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi!
Also isn't the patch missing something?

There are only modifications to tst_test.c and test_test.h.

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 4/5] shell lib: Add basic support for test cleanup
  2025-02-28 17:24 ` [LTP] [RFC PATCH 4/5] shell lib: Add basic support for test cleanup Petr Vorel
@ 2025-03-07 16:40   ` Cyril Hrubis
  2025-03-10 10:27     ` Petr Vorel
  2025-04-25 18:33     ` Petr Vorel
  0 siblings, 2 replies; 22+ messages in thread
From: Cyril Hrubis @ 2025-03-07 16:40 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

> diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh
> index b13bab37c3..68f9a0daa9 100644
> --- a/testcases/lib/tst_env.sh
> +++ b/testcases/lib/tst_env.sh
> @@ -35,7 +35,3 @@ tst_brk_()
>  
>  alias tst_res="tst_res_ $tst_script_name \$LINENO"
>  alias tst_brk="tst_brk_ $tst_script_name \$LINENO"
> -
> -if [ -n "$TST_CLEANUP" ]; then
> -	trap $TST_CLEANUP EXIT
> -fi
> diff --git a/testcases/lib/tst_exec.sh b/testcases/lib/tst_exec.sh
> new file mode 100755
> index 0000000000..dcf40fd5bb
> --- /dev/null
> +++ b/testcases/lib/tst_exec.sh
> @@ -0,0 +1,19 @@
> +#!/bin/sh
> +# Copyright (c) 2025 Cyril Hrubis <chrubis@suse.cz>
> +# Copyright (c) 2025 Petr Vorel <pvorel@suse.cz>
> +
> +. tst_env.sh
> +
> +. "$1"
> +
> +if [ -n "$TST_CLEANUP" ]; then
> +	trap $TST_CLEANUP EXIT
> +fi
> +
> +if [ -n "$TST_SETUP" ]; then
> +    $TST_SETUP
> +fi
> +
> +tst_test
> +
> +# vim: set ft=sh ts=4 sts=4 sw=4 expandtab :

Please do not add the vim formatting hints.

> diff --git a/testcases/lib/tst_loader.sh b/testcases/lib/tst_loader.sh
> index 62c9cc6d8f..e2d1bd7daf 100644
> --- a/testcases/lib/tst_loader.sh
> +++ b/testcases/lib/tst_loader.sh
> @@ -3,11 +3,8 @@
>  # Copyright (c) 2024-2025 Cyril Hrubis <chrubis@suse.cz>
>  #
>  # This is a loader for shell tests that use the C test library.
> -#
>  
>  if [ -z "$LTP_IPC_PATH" ]; then
> -	tst_run_shell $(basename "$0") "$@"
> +	tst_run_shell tst_exec.sh $(basename "$0") "$@"
>  	exit $?
> -else
> -	. tst_env.sh
>  fi

Do we really need the tst_exec.sh?

Doesn't it work if we add what is in the tst_exec here?

The whole point of [ -z "$LTP_IPC_PATH" ]; is to detect when we are
being re-executed by the tst_run_shell, so the else branch (which
isn't really needed, because we do exit in the if) was when the test did
run.

If you change the tests to be in functions and source the tst_loader.sh
at the end it should just work.

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 2/5] lib: Allow test to have positional args
  2025-03-07 16:27   ` Cyril Hrubis
@ 2025-03-10 10:16     ` Petr Vorel
  0 siblings, 0 replies; 22+ messages in thread
From: Petr Vorel @ 2025-03-10 10:16 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

> Hi!
> Also isn't the patch missing something?

> There are only modifications to tst_test.c and test_test.h.

FYI I wanted to separate this change from the actual use (next – 3rd commit).
That's why I mentioned it in the commit message.

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 2/5] lib: Allow test to have positional args
  2025-03-07 16:17   ` Cyril Hrubis
@ 2025-03-10 10:23     ` Petr Vorel
  0 siblings, 0 replies; 22+ messages in thread
From: Petr Vorel @ 2025-03-10 10:23 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

Hi Cyril,

> Hi!
> > diff --git a/doc/developers/writing_tests.rst b/doc/developers/writing_tests.rst
> > index 9b18ec059c..f5796ddc49 100644
> > --- a/doc/developers/writing_tests.rst
> > +++ b/doc/developers/writing_tests.rst
> > @@ -521,7 +521,7 @@ LTP C And Shell Test API Comparison
> >      * - not applicable
> >        - TST_NEEDS_MODULE

> > -    * - not applicable
> > +    * - .pos_args (internal use for tst_run_shell.c)
> >        - TST_POS_ARGS

> >      * - not applicable
> > diff --git a/include/tst_test.h b/include/tst_test.h
> > index eb73cd593c..b249f833ab 100644
> > --- a/include/tst_test.h
> > +++ b/include/tst_test.h
> > @@ -292,8 +292,11 @@ struct tst_fs {
> >   *
> >   * @tcnt: A number of tests. If set the test() callback is called tcnt times
> >   *        and each time passed an increasing counter value.
> > + *
> >   * @options: An NULL optstr terminated array of struct tst_option.
> >   *
> > + * @pos_args: An number of positional parameters passed to tst_run_shell.c.

> We do not support positional arguments for the C API. Do we really need
> them for shell?

I needed this for following change 4th commit ("shell lib: Add basic support for
test cleanup", which wrongly mentions cleanup instead of setup):

-	tst_run_shell $(basename "$0") "$@"
+	tst_run_shell tst_exec.sh $(basename "$0") "$@"

I.e. it is only for tst_run_shell.c.  I'll have look on it, if it's not needed
sure this commit would be useless. I would like to avoid this change as well.

> >   * @min_kver: A minimal kernel version the test can run on. e.g. "3.10".
> >   *
> >   * @supported_archs: A NULL terminated array of architectures the test runs on
> > @@ -528,6 +531,7 @@ struct tst_fs {
> >  	unsigned int tcnt;

> >  	struct tst_option *options;
> > +	int pos_args;

> >  	const char *min_kver;

> > @@ -555,7 +559,6 @@ struct tst_fs {
> >  	unsigned int skip_in_secureboot:1;
> >  	unsigned int skip_in_compat:1;

> > -
> >  	int needs_abi_bits;

> >  	unsigned int needs_hugetlbfs:1;
> > diff --git a/lib/tst_test.c b/lib/tst_test.c
> > index 3823ea109e..1c2cc5e3b2 100644
> > --- a/lib/tst_test.c
> > +++ b/lib/tst_test.c
> > @@ -711,6 +711,9 @@ static void parse_opts(int argc, char *argv[])

> >  	check_option_collision();

> > +	if (tst_test->pos_args < 0)
> > +		tst_brk(TBROK, ".pos_args must be >= 0");

> You can declare pos_args as unsigned and you don't have to add this
> condition.

Good point, thanks!

> >  	optstr[0] = 0;

> >  	for (i = 0; i < ARRAY_SIZE(options); i++)
> > @@ -751,8 +754,10 @@ static void parse_opts(int argc, char *argv[])
> >  		}
> >  	}

> > -	if (optind < argc)
> > -		tst_brk(TBROK, "Unexpected argument(s) '%s'...", argv[optind]);
> > +	if (optind + tst_test->pos_args < argc) {
> > +		tst_brk(TBROK, "Unexpected argument(s) '%s' (%d + %d < %d)",
> > +			argv[optind], optind, tst_test->pos_args, argc);
> > +	}

> And this half enables the positional arguments for the C API as well. If
> we set the pos_args in tst_test, then we can pass them, but there is no
> way how they can be passed to the test.

Again, this is only for tst_run_shell.c.

Kind regards,
Petr

> So if we are going to add them, we should pass then in
> extern char **tst_args or something like that.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 4/5] shell lib: Add basic support for test cleanup
  2025-03-07 16:40   ` Cyril Hrubis
@ 2025-03-10 10:27     ` Petr Vorel
  2025-04-25 18:33     ` Petr Vorel
  1 sibling, 0 replies; 22+ messages in thread
From: Petr Vorel @ 2025-03-10 10:27 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

Hi Cyril, all,

Note: subject wrongly mentions cleanup instead of setup

s/cleanup/setup/

> > diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh
> > index b13bab37c3..68f9a0daa9 100644
> > --- a/testcases/lib/tst_env.sh
> > +++ b/testcases/lib/tst_env.sh
> > @@ -35,7 +35,3 @@ tst_brk_()

> >  alias tst_res="tst_res_ $tst_script_name \$LINENO"
> >  alias tst_brk="tst_brk_ $tst_script_name \$LINENO"
> > -
> > -if [ -n "$TST_CLEANUP" ]; then
> > -	trap $TST_CLEANUP EXIT
> > -fi
> > diff --git a/testcases/lib/tst_exec.sh b/testcases/lib/tst_exec.sh
> > new file mode 100755
> > index 0000000000..dcf40fd5bb
> > --- /dev/null
> > +++ b/testcases/lib/tst_exec.sh
> > @@ -0,0 +1,19 @@
> > +#!/bin/sh
> > +# Copyright (c) 2025 Cyril Hrubis <chrubis@suse.cz>
> > +# Copyright (c) 2025 Petr Vorel <pvorel@suse.cz>
> > +
> > +. tst_env.sh
> > +
> > +. "$1"
> > +
> > +if [ -n "$TST_CLEANUP" ]; then
> > +	trap $TST_CLEANUP EXIT
> > +fi
> > +
> > +if [ -n "$TST_SETUP" ]; then
> > +    $TST_SETUP
> > +fi
> > +
> > +tst_test
> > +
> > +# vim: set ft=sh ts=4 sts=4 sw=4 expandtab :

> Please do not add the vim formatting hints.

Ah, sorry (editor macro when creating new file, I forget to delete it).

> > diff --git a/testcases/lib/tst_loader.sh b/testcases/lib/tst_loader.sh
> > index 62c9cc6d8f..e2d1bd7daf 100644
> > --- a/testcases/lib/tst_loader.sh
> > +++ b/testcases/lib/tst_loader.sh
> > @@ -3,11 +3,8 @@
> >  # Copyright (c) 2024-2025 Cyril Hrubis <chrubis@suse.cz>

> >  # This is a loader for shell tests that use the C test library.
> > -#

> >  if [ -z "$LTP_IPC_PATH" ]; then
> > -	tst_run_shell $(basename "$0") "$@"
> > +	tst_run_shell tst_exec.sh $(basename "$0") "$@"
> >  	exit $?
> > -else
> > -	. tst_env.sh
> >  fi

> Do we really need the tst_exec.sh?

> Doesn't it work if we add what is in the tst_exec here?

Do you mean to avoid this change:

-	tst_run_shell $(basename "$0") "$@"
+	tst_run_shell tst_exec.sh $(basename "$0") "$@"

and put the content here? I'll try it during this week.

> The whole point of [ -z "$LTP_IPC_PATH" ]; is to detect when we are
> being re-executed by the tst_run_shell, so the else branch (which
> isn't really needed, because we do exit in the if) was when the test did
> run.

+1. I figured this later after doing more debugging (preparation for internal
SUSE talk).

> If you change the tests to be in functions and source the tst_loader.sh
> at the end it should just work.

+1
Thanks for your review!

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 4/5] shell lib: Add basic support for test cleanup
  2025-03-07 16:40   ` Cyril Hrubis
  2025-03-10 10:27     ` Petr Vorel
@ 2025-04-25 18:33     ` Petr Vorel
  2025-04-30  8:52       ` Cyril Hrubis
  1 sibling, 1 reply; 22+ messages in thread
From: Petr Vorel @ 2025-04-25 18:33 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

Hi Cyril,

...
> > +++ b/testcases/lib/tst_loader.sh
> > @@ -3,11 +3,8 @@
> >  # Copyright (c) 2024-2025 Cyril Hrubis <chrubis@suse.cz>

> >  # This is a loader for shell tests that use the C test library.
> > -#

> >  if [ -z "$LTP_IPC_PATH" ]; then
> > -	tst_run_shell $(basename "$0") "$@"
> > +	tst_run_shell tst_exec.sh $(basename "$0") "$@"
> >  	exit $?
> > -else
> > -	. tst_env.sh
> >  fi

> Do we really need the tst_exec.sh?

> Doesn't it work if we add what is in the tst_exec here?

I guess you mean to keep the original:
-	tst_run_shell $(basename "$0") "$@"

instead of what I proposed:
+	tst_run_shell tst_exec.sh $(basename "$0") "$@"

I added it to get rid of previous error:

tst_res: not found

$ PATH="testcases/lib:testcases/lib/tests:$PATH" shell_loader_setup_cleanup.sh
tst_test.c:1903: TINFO: LTP version: 20250130-239-gc016fb0c0a
tst_test.c:1907: TINFO: Tested kernel: 6.12.20-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.12.20-1 (2025-03-23) x86_64
tst_kconfig.c:88: TINFO: Parsing kernel config '/boot/config-6.12.20-amd64'
tst_test.c:1720: TINFO: Overall timeout per run is 0h 00m 30s
testcases/lib/tests/shell_loader_setup_cleanup.sh: 16: testcases/lib/tst_loader.sh: tst_res: not found
testcases/lib/tests/shell_loader_setup_cleanup.sh: 21: tst_res: not found
tst_test.c:452: TBROK: Invalid child (82160) exit value 127

tst_exec.sh added in this v1 contains:

. tst_env.sh

. "$1"

if [ -n "$TST_CLEANUP" ]; then
	trap $TST_CLEANUP EXIT
fi

if [ -n "$TST_SETUP" ]; then
    $TST_SETUP
fi

tst_test
---

The most important part is:

. "$1"

which is to source the script. This cannot be added into tst_loader.sh
(it would create indefinite loop), this must be somehow added to tst_run_shell.c

And the only way I come up with was to add it via the above mentioned change of
tst_run_shell.c running actually tst_exec.sh instead of the test directly.

I guess I'm missing something.

Feel free to look into my v2, likely it's obvious to you what am I missing:
https://github.com/pevik/ltp/blob/refs/heads/shell-loader-setup-cleanup.v2/testcases/lib/tst_loader.sh

Kind regards,
Petr

> The whole point of [ -z "$LTP_IPC_PATH" ]; is to detect when we are
> being re-executed by the tst_run_shell, so the else branch (which
> isn't really needed, because we do exit in the if) was when the test did
> run.

> If you change the tests to be in functions and source the tst_loader.sh
> at the end it should just work.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 4/5] shell lib: Add basic support for test cleanup
  2025-04-25 18:33     ` Petr Vorel
@ 2025-04-30  8:52       ` Cyril Hrubis
  2025-04-30  8:57         ` Cyril Hrubis
  0 siblings, 1 reply; 22+ messages in thread
From: Cyril Hrubis @ 2025-04-30  8:52 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi!
> tst_exec.sh added in this v1 contains:
> 
> . tst_env.sh
> 
> . "$1"
> 
> if [ -n "$TST_CLEANUP" ]; then
> 	trap $TST_CLEANUP EXIT
> fi
> 
> if [ -n "$TST_SETUP" ]; then
>     $TST_SETUP
> fi
> 
> tst_test
> ---
> 
> The most important part is:
> 
> . "$1"
> 
> which is to source the script. This cannot be added into tst_loader.sh
> (it would create indefinite loop), this must be somehow added to tst_run_shell.c

So the problem seems to be that if shell parses the script before we
define aliases they are not expanded in the code shell already parsed.


This works:

alias tst_res="..."

tst_res TINFO "foo"


This does not:

tst_res TINFO "foo"

alias tst_res="..."

The tst_loader.sh has to be sourced first after all.

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 4/5] shell lib: Add basic support for test cleanup
  2025-04-30  8:52       ` Cyril Hrubis
@ 2025-04-30  8:57         ` Cyril Hrubis
  2025-04-30 11:47           ` Petr Vorel
  0 siblings, 1 reply; 22+ messages in thread
From: Cyril Hrubis @ 2025-04-30  8:57 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi!
And one possible solution is to source the environment first and the
loader last, on the top of your shell-loader-setup-cleanup.v2 branch:

diff --git a/testcases/lib/tests/shell_loader.sh b/testcases/lib/tests/shell_loader.sh
index eeed122c1..01acf6d35 100755
--- a/testcases/lib/tests/shell_loader.sh
+++ b/testcases/lib/tests/shell_loader.sh
@@ -14,6 +14,8 @@
 # }
 # ---

+. tst_env.sh
+
 tst_test()
 {
        tst_res TPASS "Shell loader works fine!"
diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh
index 68f9a0daa..585790a7d 100644
--- a/testcases/lib/tst_env.sh
+++ b/testcases/lib/tst_env.sh
@@ -18,11 +18,6 @@ if [ -z "$LINENO" ]; then
        LINENO=-1
 fi

-if [ -z "$LTP_IPC_PATH" ]; then
-       echo "This script has to be executed from a LTP loader!"
-       exit 1
-fi
-
 tst_brk_()
 {
        tst_res_ "$@"


-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 4/5] shell lib: Add basic support for test cleanup
  2025-04-30  8:57         ` Cyril Hrubis
@ 2025-04-30 11:47           ` Petr Vorel
  2025-04-30 13:39             ` Cyril Hrubis
  0 siblings, 1 reply; 22+ messages in thread
From: Petr Vorel @ 2025-04-30 11:47 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

Hi Cyril,

> Hi!
> And one possible solution is to source the environment first and the
> loader last, on the top of your shell-loader-setup-cleanup.v2 branch:

> diff --git a/testcases/lib/tests/shell_loader.sh b/testcases/lib/tests/shell_loader.sh
> index eeed122c1..01acf6d35 100755
> --- a/testcases/lib/tests/shell_loader.sh
> +++ b/testcases/lib/tests/shell_loader.sh
> @@ -14,6 +14,8 @@
>  # }
>  # ---

> +. tst_env.sh
> +

OK, that is the missing piece, thank you!

i.e. sourcing tst_env.sh at the beginning before test function setup and
tst_loader.sh at the end after test function setup). I thought we would avoid
sourcing two files in the tests, that's why I haven't tried, but that's a minor
detail. And it's better solution than having tst_exec.sh, which requires
tst_run_shell.c modification, just to do the same thing (source tst_env.sh).

Kind regards,
Petr

>  tst_test()
>  {
>         tst_res TPASS "Shell loader works fine!"
> diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh
> index 68f9a0daa..585790a7d 100644
> --- a/testcases/lib/tst_env.sh
> +++ b/testcases/lib/tst_env.sh
> @@ -18,11 +18,6 @@ if [ -z "$LINENO" ]; then
>         LINENO=-1
>  fi

> -if [ -z "$LTP_IPC_PATH" ]; then
> -       echo "This script has to be executed from a LTP loader!"
> -       exit 1
> -fi
> -
>  tst_brk_()
>  {
>         tst_res_ "$@"

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [LTP] [RFC PATCH 4/5] shell lib: Add basic support for test cleanup
  2025-04-30 11:47           ` Petr Vorel
@ 2025-04-30 13:39             ` Cyril Hrubis
  0 siblings, 0 replies; 22+ messages in thread
From: Cyril Hrubis @ 2025-04-30 13:39 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi!
> > And one possible solution is to source the environment first and the
> > loader last, on the top of your shell-loader-setup-cleanup.v2 branch:
> 
> > diff --git a/testcases/lib/tests/shell_loader.sh b/testcases/lib/tests/shell_loader.sh
> > index eeed122c1..01acf6d35 100755
> > --- a/testcases/lib/tests/shell_loader.sh
> > +++ b/testcases/lib/tests/shell_loader.sh
> > @@ -14,6 +14,8 @@
> >  # }
> >  # ---
> 
> > +. tst_env.sh
> > +
> 
> OK, that is the missing piece, thank you!
> 
> i.e. sourcing tst_env.sh at the beginning before test function setup and
> tst_loader.sh at the end after test function setup). I thought we would avoid
> sourcing two files in the tests, that's why I haven't tried, but that's a minor
> detail. And it's better solution than having tst_exec.sh, which requires
> tst_run_shell.c modification, just to do the same thing (source tst_env.sh).

Given all the choices I too think that soucing tst_env.sh first to get
all the defitions and tst_loader.sh to start the test is the cleanest
solution.

-- 
Cyril Hrubis
chrubis@suse.cz

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2025-04-30 13:38 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-28 17:24 [LTP] [RFC PATCH 0/5] shell loader rewrite to support TST_SETUP Petr Vorel
2025-02-28 17:24 ` [LTP] [RFC PATCH 1/5] shell lib: Add support for test cleanup Petr Vorel
2025-03-04 12:57   ` Li Wang
2025-03-04 13:08     ` Petr Vorel
2025-03-04 13:15       ` Li Wang
2025-02-28 17:24 ` [LTP] [RFC PATCH 2/5] lib: Allow test to have positional args Petr Vorel
2025-02-28 17:43   ` Petr Vorel
2025-03-07 16:17   ` Cyril Hrubis
2025-03-10 10:23     ` Petr Vorel
2025-03-07 16:27   ` Cyril Hrubis
2025-03-10 10:16     ` Petr Vorel
2025-02-28 17:24 ` [LTP] [RFC PATCH 3/5] shell: Move shell code into functions Petr Vorel
2025-02-28 17:45   ` Petr Vorel
2025-02-28 17:24 ` [LTP] [RFC PATCH 4/5] shell lib: Add basic support for test cleanup Petr Vorel
2025-03-07 16:40   ` Cyril Hrubis
2025-03-10 10:27     ` Petr Vorel
2025-04-25 18:33     ` Petr Vorel
2025-04-30  8:52       ` Cyril Hrubis
2025-04-30  8:57         ` Cyril Hrubis
2025-04-30 11:47           ` Petr Vorel
2025-04-30 13:39             ` Cyril Hrubis
2025-02-28 17:24 ` [LTP] [RFC PATCH 5/5] shell: Add shell_loader_setup_cleanup.sh test Petr Vorel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox