public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
* [LTP] [RFC PATCH 0/2] Shell test library v3
@ 2024-07-16 15:36 Cyril Hrubis
  2024-07-16 15:36 ` [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code Cyril Hrubis
  2024-07-16 15:36 ` [LTP] [RFC PATCH 2/2] testcaes/lib: Add shell loader Cyril Hrubis
  0 siblings, 2 replies; 25+ messages in thread
From: Cyril Hrubis @ 2024-07-16 15:36 UTC (permalink / raw)
  To: ltp

We have discussed shell and C integration during our LTP montly meeting.
I started to dig around after to see how complicated would that task be
and it turns out that it's fairly easy.

This patchset implements a proof of a concept for two things:

- seamless integration of shell with C code
- second patch builds on top of that and implements a shell loader
  that can run shell tests from within the C test library

Cyril Hrubis (2):
  Add support for mixing C and shell code
  testcaes/lib: Add shell loader

 include/tst_test.h                            | 38 +++++++++
 lib/tst_test.c                                | 51 ++++++++++++
 testcases/lib/.gitignore                      |  2 +
 testcases/lib/Makefile                        |  5 +-
 testcases/lib/run_tests.sh                    | 12 +++
 testcases/lib/tests/.gitignore                |  6 ++
 testcases/lib/tests/Makefile                  | 11 +++
 testcases/lib/tests/shell_loader.sh           | 12 +++
 .../lib/tests/shell_loader_all_filesystems.sh | 22 ++++++
 testcases/lib/tests/shell_test01.c            | 17 ++++
 testcases/lib/tests/shell_test02.c            | 18 +++++
 testcases/lib/tests/shell_test03.c            | 25 ++++++
 testcases/lib/tests/shell_test04.c            | 18 +++++
 testcases/lib/tests/shell_test05.c            | 27 +++++++
 testcases/lib/tests/shell_test06.c            | 16 ++++
 testcases/lib/tests/shell_test_brk.sh         |  6 ++
 testcases/lib/tests/shell_test_check_argv.sh  | 25 ++++++
 testcases/lib/tests/shell_test_checkpoint.sh  |  7 ++
 testcases/lib/tests/shell_test_pass.sh        |  6 ++
 testcases/lib/tst_env.sh                      | 16 ++++
 testcases/lib/tst_res_.c                      | 58 ++++++++++++++
 testcases/lib/tst_run_shell.c                 | 78 +++++++++++++++++++
 22 files changed, 474 insertions(+), 2 deletions(-)
 create mode 100755 testcases/lib/run_tests.sh
 create mode 100644 testcases/lib/tests/.gitignore
 create mode 100644 testcases/lib/tests/Makefile
 create mode 100755 testcases/lib/tests/shell_loader.sh
 create mode 100755 testcases/lib/tests/shell_loader_all_filesystems.sh
 create mode 100644 testcases/lib/tests/shell_test01.c
 create mode 100644 testcases/lib/tests/shell_test02.c
 create mode 100644 testcases/lib/tests/shell_test03.c
 create mode 100644 testcases/lib/tests/shell_test04.c
 create mode 100644 testcases/lib/tests/shell_test05.c
 create mode 100644 testcases/lib/tests/shell_test06.c
 create mode 100755 testcases/lib/tests/shell_test_brk.sh
 create mode 100755 testcases/lib/tests/shell_test_check_argv.sh
 create mode 100755 testcases/lib/tests/shell_test_checkpoint.sh
 create mode 100755 testcases/lib/tests/shell_test_pass.sh
 create mode 100644 testcases/lib/tst_env.sh
 create mode 100644 testcases/lib/tst_res_.c
 create mode 100644 testcases/lib/tst_run_shell.c

-- 
2.44.2


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

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

* [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-16 15:36 [LTP] [RFC PATCH 0/2] Shell test library v3 Cyril Hrubis
@ 2024-07-16 15:36 ` Cyril Hrubis
  2024-07-17  8:33   ` Li Wang
                     ` (3 more replies)
  2024-07-16 15:36 ` [LTP] [RFC PATCH 2/2] testcaes/lib: Add shell loader Cyril Hrubis
  1 sibling, 4 replies; 25+ messages in thread
From: Cyril Hrubis @ 2024-07-16 15:36 UTC (permalink / raw)
  To: ltp

This is a proof of a concept of a seamless C and shell integration. The
idea is that with this you can mix shell and C code as much as as you
wish to get the best of the two worlds.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 include/tst_test.h                           | 38 +++++++++++++
 lib/tst_test.c                               | 51 +++++++++++++++++
 testcases/lib/.gitignore                     |  1 +
 testcases/lib/Makefile                       |  4 +-
 testcases/lib/run_tests.sh                   | 10 ++++
 testcases/lib/tests/.gitignore               |  6 ++
 testcases/lib/tests/Makefile                 | 11 ++++
 testcases/lib/tests/shell_loader.sh          |  5 ++
 testcases/lib/tests/shell_test01.c           | 17 ++++++
 testcases/lib/tests/shell_test02.c           | 18 ++++++
 testcases/lib/tests/shell_test03.c           | 25 +++++++++
 testcases/lib/tests/shell_test04.c           | 18 ++++++
 testcases/lib/tests/shell_test05.c           | 27 +++++++++
 testcases/lib/tests/shell_test06.c           | 16 ++++++
 testcases/lib/tests/shell_test_brk.sh        |  6 ++
 testcases/lib/tests/shell_test_check_argv.sh | 25 +++++++++
 testcases/lib/tests/shell_test_checkpoint.sh |  7 +++
 testcases/lib/tests/shell_test_pass.sh       |  6 ++
 testcases/lib/tst_env.sh                     | 16 ++++++
 testcases/lib/tst_res_.c                     | 58 ++++++++++++++++++++
 20 files changed, 363 insertions(+), 2 deletions(-)
 create mode 100755 testcases/lib/run_tests.sh
 create mode 100644 testcases/lib/tests/.gitignore
 create mode 100644 testcases/lib/tests/Makefile
 create mode 100644 testcases/lib/tests/shell_loader.sh
 create mode 100644 testcases/lib/tests/shell_test01.c
 create mode 100644 testcases/lib/tests/shell_test02.c
 create mode 100644 testcases/lib/tests/shell_test03.c
 create mode 100644 testcases/lib/tests/shell_test04.c
 create mode 100644 testcases/lib/tests/shell_test05.c
 create mode 100644 testcases/lib/tests/shell_test06.c
 create mode 100755 testcases/lib/tests/shell_test_brk.sh
 create mode 100755 testcases/lib/tests/shell_test_check_argv.sh
 create mode 100755 testcases/lib/tests/shell_test_checkpoint.sh
 create mode 100755 testcases/lib/tests/shell_test_pass.sh
 create mode 100644 testcases/lib/tst_env.sh
 create mode 100644 testcases/lib/tst_res_.c

diff --git a/include/tst_test.h b/include/tst_test.h
index 517c8d032..fde8a1414 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -331,6 +331,8 @@ struct tst_fs {
  * @child_needs_reinit: Has to be set if the test needs to call tst_reinit()
  *                      from a process started by exec().
  *
+ * @runs_shell: Implies child_needs_reinit and forks_child at the moment.
+ *
  * @needs_devfs: If set the devfs is mounted at tst_test.mntpoint. This is
  *               needed for tests that need to create device files since tmpfs
  *               at /tmp is usually mounted with 'nodev' option.
@@ -514,6 +516,7 @@ struct tst_fs {
 	unsigned int mount_device:1;
 	unsigned int needs_rofs:1;
 	unsigned int child_needs_reinit:1;
+	unsigned int runs_shell:1;
 	unsigned int needs_devfs:1;
 	unsigned int restore_wallclock:1;
 
@@ -522,6 +525,8 @@ struct tst_fs {
 	unsigned int skip_in_lockdown:1;
 	unsigned int skip_in_secureboot:1;
 	unsigned int skip_in_compat:1;
+
+
 	int needs_abi_bits;
 
 	unsigned int needs_hugetlbfs:1;
@@ -607,6 +612,39 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
  */
 void tst_reinit(void);
 
+/**
+ * tst_run_shell() - Prepare the environment and execute a shell script.
+ *
+ * @script_name: A filename of the script.
+ * @params: A NULL terminated array of shell script parameters, pass NULL if
+ *          none are needed. This what is passed starting from argv[1].
+ *
+ * The shell script is executed with LTP_IPC_PATH in environment so that the
+ * binary helpers such as tst_res_ or tst_checkpoint work properly when executed
+ * from the script. This also means that the tst_test.runs_shell flag needs to
+ * be set.
+ *
+ * The shell script itself has to source the tst_env.sh shell script at the
+ * start and after that it's free to use tst_res in the same way C code would
+ * use.
+ *
+ * Example shell script that reports success::
+ *
+ *   #!/bin/sh
+ *   . tst_env.sh
+ *
+ *   tst_res TPASS "Example test works"
+ *
+ * The call returns a pid in a case that you want to examine the return value
+ * of the script yourself. If you do not need to check the return value
+ * yourself you can use tst_reap_children() to wait for the completion. Or let
+ * the test library collect the child automatically, just be wary that the
+ * script and the test both runs concurently at the same time in this case.
+ *
+ * Return: A pid of the shell process.
+ */
+int tst_run_shell(char *script_name, char *const params[]);
+
 unsigned int tst_multiply_timeout(unsigned int timeout);
 
 /*
diff --git a/lib/tst_test.c b/lib/tst_test.c
index e5bc5bf4d..fa0907353 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -173,6 +173,52 @@ void tst_reinit(void)
 	SAFE_CLOSE(fd);
 }
 
+extern char **environ;
+
+static unsigned int params_array_len(char *const array[])
+{
+	unsigned int ret = 0;
+
+	if (!array)
+		return 0;
+
+	while (*(array++))
+		ret++;
+
+	return ret;
+}
+
+int tst_run_shell(char *script_name, char *const params[])
+{
+	int pid;
+	unsigned int i, params_len = params_array_len(params);
+	char *argv[params_len + 2] = {};
+
+	if (!tst_test->runs_shell)
+		tst_brk(TBROK, "runs_shell flag must be set!");
+
+	argv[0] = script_name;
+
+	if (params) {
+		for (i = 0; i < params_len; i++)
+			argv[i+1] = params[i];
+	}
+
+	pid = SAFE_FORK();
+	if (pid)
+		return pid;
+
+	char *script_name_env = NULL;
+	SAFE_ASPRINTF(&script_name_env, "LTP_SCRIPT_NAME=%s", script_name);
+	putenv(script_name_env);
+
+	execvpe(script_name, argv, environ);
+
+	tst_brk(TBROK | TERRNO, "execv(%s, ...) failed!", script_name);
+
+	return -1;
+}
+
 static void update_results(int ttype)
 {
 	if (!results)
@@ -1224,6 +1270,11 @@ static void do_setup(int argc, char *argv[])
 		tdebug = 1;
 	}
 
+	if (tst_test->runs_shell) {
+		tst_test->child_needs_reinit = 1;
+		tst_test->forks_child = 1;
+	}
+
 	if (tst_test->needs_kconfigs && tst_kconfig_check(tst_test->needs_kconfigs))
 		tst_brk(TCONF, "Aborting due to unsuitable kernel config, see above!");
 
diff --git a/testcases/lib/.gitignore b/testcases/lib/.gitignore
index e8afd06f3..d0dacf62a 100644
--- a/testcases/lib/.gitignore
+++ b/testcases/lib/.gitignore
@@ -23,3 +23,4 @@
 /tst_sleep
 /tst_supported_fs
 /tst_timeout_kill
+/tst_res_
diff --git a/testcases/lib/Makefile b/testcases/lib/Makefile
index 990b46089..928d76d62 100644
--- a/testcases/lib/Makefile
+++ b/testcases/lib/Makefile
@@ -13,6 +13,6 @@ MAKE_TARGETS		:= tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
 			   tst_getconf tst_supported_fs tst_check_drivers tst_get_unused_port\
 			   tst_get_median tst_hexdump tst_get_free_pids tst_timeout_kill\
 			   tst_check_kconfigs tst_cgctl tst_fsfreeze tst_ns_create tst_ns_exec\
-			   tst_ns_ifmove tst_lockdown_enabled tst_secureboot_enabled
+			   tst_ns_ifmove tst_lockdown_enabled tst_secureboot_enabled tst_res_
 
-include $(top_srcdir)/include/mk/generic_leaf_target.mk
+include $(top_srcdir)/include/mk/generic_trunk_target.mk
diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
new file mode 100755
index 000000000..88607b146
--- /dev/null
+++ b/testcases/lib/run_tests.sh
@@ -0,0 +1,10 @@
+#!/bin/sh
+
+export PATH=$PATH:$PWD:$PWD/tests/
+
+./tests/shell_test01
+./tests/shell_test02
+./tests/shell_test03
+./tests/shell_test04
+./tests/shell_test05
+./tests/shell_test06
diff --git a/testcases/lib/tests/.gitignore b/testcases/lib/tests/.gitignore
new file mode 100644
index 000000000..da967c4d6
--- /dev/null
+++ b/testcases/lib/tests/.gitignore
@@ -0,0 +1,6 @@
+shell_test01
+shell_test02
+shell_test03
+shell_test04
+shell_test05
+shell_test06
diff --git a/testcases/lib/tests/Makefile b/testcases/lib/tests/Makefile
new file mode 100644
index 000000000..5a5cf5310
--- /dev/null
+++ b/testcases/lib/tests/Makefile
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (C) 2009, Cisco Systems Inc.
+# Ngie Cooper, August 2009
+
+top_srcdir		?= ../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+INSTALL_TARGETS=
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/lib/tests/shell_loader.sh b/testcases/lib/tests/shell_loader.sh
new file mode 100644
index 000000000..c3b3cf5fd
--- /dev/null
+++ b/testcases/lib/tests/shell_loader.sh
@@ -0,0 +1,5 @@
+#!/usr/bin/env tst_run_shell
+
+. tst_env.sh
+
+tst_res TPASS "Shell loader works fine!"
diff --git a/testcases/lib/tests/shell_test01.c b/testcases/lib/tests/shell_test01.c
new file mode 100644
index 000000000..18f834138
--- /dev/null
+++ b/testcases/lib/tests/shell_test01.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Shell test example.
+ */
+
+#include "tst_test.h"
+
+static void run_test(void)
+{
+	tst_run_shell("shell_test_pass.sh", NULL);
+	tst_res(TINFO, "C test exits now");
+}
+
+static struct tst_test test = {
+	.runs_shell = 1,
+	.test_all = run_test,
+};
diff --git a/testcases/lib/tests/shell_test02.c b/testcases/lib/tests/shell_test02.c
new file mode 100644
index 000000000..3b0534370
--- /dev/null
+++ b/testcases/lib/tests/shell_test02.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Shell test example.
+ */
+
+#include "tst_test.h"
+
+static void run_test(void)
+{
+	tst_run_shell("shell_test_pass.sh", NULL);
+	tst_reap_children();
+	tst_res(TINFO, "Shell test has finished at this point!");
+}
+
+static struct tst_test test = {
+	.runs_shell = 1,
+	.test_all = run_test,
+};
diff --git a/testcases/lib/tests/shell_test03.c b/testcases/lib/tests/shell_test03.c
new file mode 100644
index 000000000..c11a09e4e
--- /dev/null
+++ b/testcases/lib/tests/shell_test03.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Shell test example.
+ */
+
+#include <sys/wait.h>
+#include "tst_test.h"
+
+static void run_test(void)
+{
+	int pid, status;
+
+	pid = tst_run_shell("shell_test_pass.sh", NULL);
+
+	tst_res(TINFO, "Waiting for the pid %i", pid);
+
+	waitpid(pid, &status, 0);
+
+	tst_res(TINFO, "Shell test has %s", tst_strstatus(status));
+}
+
+static struct tst_test test = {
+	.runs_shell = 1,
+	.test_all = run_test,
+};
diff --git a/testcases/lib/tests/shell_test04.c b/testcases/lib/tests/shell_test04.c
new file mode 100644
index 000000000..fd1fd122c
--- /dev/null
+++ b/testcases/lib/tests/shell_test04.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Shell test example.
+ */
+
+#include "tst_test.h"
+
+static void run_test(void)
+{
+	char *const params[] = {"param1", "param2", NULL};
+
+	tst_run_shell("shell_test_check_argv.sh", params);
+}
+
+static struct tst_test test = {
+	.runs_shell = 1,
+	.test_all = run_test,
+};
diff --git a/testcases/lib/tests/shell_test05.c b/testcases/lib/tests/shell_test05.c
new file mode 100644
index 000000000..2141d4790
--- /dev/null
+++ b/testcases/lib/tests/shell_test05.c
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Shell test example.
+ */
+
+#include "tst_test.h"
+
+static void run_test(void)
+{
+	int pid;
+
+	pid = tst_run_shell("shell_test_checkpoint.sh", NULL);
+
+	tst_res(TINFO, "Waiting for shell to sleep on checkpoint!");
+
+	TST_PROCESS_STATE_WAIT(pid, 'S', 10000);
+
+	tst_res(TINFO, "Waking shell child!");
+
+	TST_CHECKPOINT_WAKE(0);
+}
+
+static struct tst_test test = {
+	.runs_shell = 1,
+	.needs_checkpoints = 1,
+	.test_all = run_test,
+};
diff --git a/testcases/lib/tests/shell_test06.c b/testcases/lib/tests/shell_test06.c
new file mode 100644
index 000000000..25abc1e7b
--- /dev/null
+++ b/testcases/lib/tests/shell_test06.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Shell test example.
+ */
+
+#include "tst_test.h"
+
+static void run_test(void)
+{
+	tst_run_shell("shell_test_brk.sh", NULL);
+}
+
+static struct tst_test test = {
+	.runs_shell = 1,
+	.test_all = run_test,
+};
diff --git a/testcases/lib/tests/shell_test_brk.sh b/testcases/lib/tests/shell_test_brk.sh
new file mode 100755
index 000000000..f266dc3fe
--- /dev/null
+++ b/testcases/lib/tests/shell_test_brk.sh
@@ -0,0 +1,6 @@
+#!/bin/sh
+
+. tst_env.sh
+
+tst_brk TCONF "This exits test and the next message should not be reached"
+tst_res TFAIL "If you see this the test failed"
diff --git a/testcases/lib/tests/shell_test_check_argv.sh b/testcases/lib/tests/shell_test_check_argv.sh
new file mode 100755
index 000000000..756189ee7
--- /dev/null
+++ b/testcases/lib/tests/shell_test_check_argv.sh
@@ -0,0 +1,25 @@
+#!/bin/sh
+
+. tst_env.sh
+
+echo "$@"
+
+tst_res TINFO "argv = $@"
+
+if [ $# -ne 2 ]; then
+	tst_res TFAIL "Wrong number of parameters got $# expected 2"
+else
+	tst_res TPASS "Got 2 parameters"
+fi
+
+if [ "$1" != "param1" ]; then
+	tst_res TFAIL "First parameter is $1 expected param1"
+else
+	tst_res TPASS "First parameter is $1"
+fi
+
+if [ "$2" != "param2" ]; then
+	tst_res TFAIL "Second parameter is $2 expected param2"
+else
+	tst_res TPASS "Second parameter is $2"
+fi
diff --git a/testcases/lib/tests/shell_test_checkpoint.sh b/testcases/lib/tests/shell_test_checkpoint.sh
new file mode 100755
index 000000000..0ceb7cf66
--- /dev/null
+++ b/testcases/lib/tests/shell_test_checkpoint.sh
@@ -0,0 +1,7 @@
+#!/bin/sh
+
+. tst_env.sh
+
+tst_res TINFO "Waiting for a checkpoint 0"
+tst_checkpoint wait 10000 0
+tst_res TPASS "Continuing after checkpoint"
diff --git a/testcases/lib/tests/shell_test_pass.sh b/testcases/lib/tests/shell_test_pass.sh
new file mode 100755
index 000000000..fd0684eb2
--- /dev/null
+++ b/testcases/lib/tests/shell_test_pass.sh
@@ -0,0 +1,6 @@
+#!/bin/sh
+
+. tst_env.sh
+
+tst_res TPASS "This is called from the shell script!"
+tst_sleep 100ms
diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh
new file mode 100644
index 000000000..c8f3c2160
--- /dev/null
+++ b/testcases/lib/tst_env.sh
@@ -0,0 +1,16 @@
+#!/bin/sh
+
+tst_script_name=$(basename $0)
+
+tst_brk_()
+{
+	tst_res_ $@
+
+	case "$3" in
+		"TBROK") exit 2;;
+		*) exit 0;;
+	esac
+}
+
+alias tst_res="tst_res_ $tst_script_name \$LINENO"
+alias tst_brk="tst_brk_ $tst_script_name \$LINENO"
diff --git a/testcases/lib/tst_res_.c b/testcases/lib/tst_res_.c
new file mode 100644
index 000000000..cc3fa53d3
--- /dev/null
+++ b/testcases/lib/tst_res_.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+
+static void print_help(void)
+{
+	printf("Usage: tst_res_ filename lineno [TPASS|TFAIL|TCONF|TINFO|TDEBUG] 'A short description'\n");
+}
+
+int main(int argc, char *argv[])
+{
+	int type, i;
+
+	if (argc < 5)
+		goto help;
+
+	if (!strcmp(argv[3], "TPASS"))
+		type = TPASS;
+	else if (!strcmp(argv[3], "TFAIL"))
+		type = TFAIL;
+	else if (!strcmp(argv[3], "TCONF"))
+		type = TCONF;
+	else if (!strcmp(argv[3], "TINFO"))
+		type = TINFO;
+	else if (!strcmp(argv[3], "TDEBUG"))
+		type = TDEBUG;
+	else
+		goto help;
+
+	size_t len = 0;
+
+	for (i = 4; i < argc; i++)
+		len += strlen(argv[i]) + 1;
+
+	char *msg = SAFE_MALLOC(len);
+	char *msgp = msg;
+
+	for (i = 4; i < argc; i++) {
+		msgp = strcpy(msgp, argv[i]);
+		msgp += strlen(argv[i]);
+		*(msgp++) = ' ';
+	}
+
+	*(msgp - 1) = 0;
+
+	tst_reinit();
+
+	tst_res_(argv[1], atoi(argv[2]), type, msg);
+
+	return 0;
+help:
+	print_help();
+	return 1;
+}
-- 
2.44.2


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

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

* [LTP] [RFC PATCH 2/2] testcaes/lib: Add shell loader
  2024-07-16 15:36 [LTP] [RFC PATCH 0/2] Shell test library v3 Cyril Hrubis
  2024-07-16 15:36 ` [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code Cyril Hrubis
@ 2024-07-16 15:36 ` Cyril Hrubis
  2024-07-24 12:54   ` Martin Doucha
  1 sibling, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2024-07-16 15:36 UTC (permalink / raw)
  To: ltp

This commit implements a shell loader so that we don't have to write a C
loader for each LTP shell test. The idea is simple, the loader parses
the shell test and prepares the tst_test structure accordingly, then
runs the actual shell test. Only small subset of the flags is
implemented at the moment, since this is RFC, however it should be clear
where this is going.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/lib/.gitignore                      |  1 +
 testcases/lib/Makefile                        |  3 +-
 testcases/lib/run_tests.sh                    |  2 +
 testcases/lib/tests/shell_loader.sh           | 11 ++-
 .../lib/tests/shell_loader_all_filesystems.sh | 22 ++++++
 testcases/lib/tst_run_shell.c                 | 78 +++++++++++++++++++
 6 files changed, 114 insertions(+), 3 deletions(-)
 mode change 100644 => 100755 testcases/lib/tests/shell_loader.sh
 create mode 100755 testcases/lib/tests/shell_loader_all_filesystems.sh
 create mode 100644 testcases/lib/tst_run_shell.c

diff --git a/testcases/lib/.gitignore b/testcases/lib/.gitignore
index d0dacf62a..385f3c3ca 100644
--- a/testcases/lib/.gitignore
+++ b/testcases/lib/.gitignore
@@ -24,3 +24,4 @@
 /tst_supported_fs
 /tst_timeout_kill
 /tst_res_
+/tst_run_shell
diff --git a/testcases/lib/Makefile b/testcases/lib/Makefile
index 928d76d62..fef284e35 100644
--- a/testcases/lib/Makefile
+++ b/testcases/lib/Makefile
@@ -13,6 +13,7 @@ MAKE_TARGETS		:= tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
 			   tst_getconf tst_supported_fs tst_check_drivers tst_get_unused_port\
 			   tst_get_median tst_hexdump tst_get_free_pids tst_timeout_kill\
 			   tst_check_kconfigs tst_cgctl tst_fsfreeze tst_ns_create tst_ns_exec\
-			   tst_ns_ifmove tst_lockdown_enabled tst_secureboot_enabled tst_res_
+			   tst_ns_ifmove tst_lockdown_enabled tst_secureboot_enabled tst_res_\
+			   tst_run_shell
 
 include $(top_srcdir)/include/mk/generic_trunk_target.mk
diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
index 88607b146..9637ae327 100755
--- a/testcases/lib/run_tests.sh
+++ b/testcases/lib/run_tests.sh
@@ -8,3 +8,5 @@ export PATH=$PATH:$PWD:$PWD/tests/
 ./tests/shell_test04
 ./tests/shell_test05
 ./tests/shell_test06
+./tst_run_shell shell_loader.sh
+./tst_run_shell shell_loader_all_filesystems.sh
diff --git a/testcases/lib/tests/shell_loader.sh b/testcases/lib/tests/shell_loader.sh
old mode 100644
new mode 100755
index c3b3cf5fd..1a255fdee
--- a/testcases/lib/tests/shell_loader.sh
+++ b/testcases/lib/tests/shell_loader.sh
@@ -1,5 +1,12 @@
-#!/usr/bin/env tst_run_shell
+#!/bin/sh
+#
+# needs_tmpdir=1
 
 . tst_env.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
diff --git a/testcases/lib/tests/shell_loader_all_filesystems.sh b/testcases/lib/tests/shell_loader_all_filesystems.sh
new file mode 100755
index 000000000..86d09d393
--- /dev/null
+++ b/testcases/lib/tests/shell_loader_all_filesystems.sh
@@ -0,0 +1,22 @@
+#!/bin/sh
+#
+# needs_root=1
+# mount_device=1
+# all_filesystems=1
+
+TST_MNTPOINT=ltp_mntpoint
+
+. tst_env.sh
+
+tst_res TINFO "IN shell"
+
+mounted=$(grep $TST_MNTPOINT /proc/mounts)
+
+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
diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
new file mode 100644
index 000000000..583807376
--- /dev/null
+++ b/testcases/lib/tst_run_shell.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2024 Cyril Hrubis <chrubis@suse.cz>
+ */
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "tst_safe_stdio.h"
+
+static char *shell_filename;
+
+static void run_shell(void)
+{
+	tst_run_shell(shell_filename, NULL);
+}
+
+struct tst_test test = {
+	.test_all = run_shell,
+	.runs_shell = 1,
+};
+
+static void print_help(void)
+{
+	printf("Usage: tst_shell_loader ltp_shell_test.sh ...");
+}
+
+#define FLAG_MATCH(str, name) (!strcmp(str, name "=1"))
+#define PARAM_MATCH(str, name) (!strncmp(str, name "=", sizeof(name)))
+#define PARAM_VAL(str, name) strdup(str + sizeof(name))
+
+static void prepare_test_struct(void)
+{
+	FILE *f;
+	char line[4096];
+	char path[4096];
+
+	if (tst_get_path(shell_filename, path, sizeof(path)) == -1)
+		tst_brk(TBROK, "Failed to find %s in $PATH", shell_filename);
+
+	f = SAFE_FOPEN(path, "r");
+
+	while (fscanf(f, "%4096s[^\n]", line) != EOF) {
+		if (FLAG_MATCH(line, "needs_tmpdir"))
+			test.needs_tmpdir = 1;
+		else if (FLAG_MATCH(line, "needs_root"))
+			test.needs_root = 1;
+		else if (FLAG_MATCH(line, "needs_device"))
+			test.needs_device = 1;
+		else if (FLAG_MATCH(line, "needs_checkpoints"))
+			test.needs_checkpoints = 1;
+		else if (FLAG_MATCH(line, "needs_overlay"))
+			test.needs_overlay = 1;
+		else if (FLAG_MATCH(line, "format_device"))
+			test.format_device = 1;
+		else if (FLAG_MATCH(line, "mount_device"))
+			test.mount_device = 1;
+		else if (PARAM_MATCH(line, "TST_MNTPOINT"))
+			test.mntpoint = PARAM_VAL(line, "TST_MNTPOINT");
+		else if (FLAG_MATCH(line, "all_filesystems"))
+			test.all_filesystems = 1;
+	}
+
+	fclose(f);
+}
+
+int main(int argc, char *argv[])
+{
+	if (argc < 2)
+		goto help;
+
+	shell_filename = argv[1];
+
+	prepare_test_struct();
+
+	tst_run_tcases(argc - 1, argv + 1, &test);
+help:
+	print_help();
+	return 1;
+}
-- 
2.44.2


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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-16 15:36 ` [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code Cyril Hrubis
@ 2024-07-17  8:33   ` Li Wang
  2024-07-17  8:41     ` Cyril Hrubis
  2024-07-17  8:52     ` Cyril Hrubis
  2024-07-18 12:43   ` Petr Vorel
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 25+ messages in thread
From: Li Wang @ 2024-07-17  8:33 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

Hi Cyril,

I haven't finished my code review, here just comment inline
to highlight errors from my local compiling.

On Tue, Jul 16, 2024 at 11:35 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> This is a proof of a concept of a seamless C and shell integration. The
> idea is that with this you can mix shell and C code as much as as you
> wish to get the best of the two worlds.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  include/tst_test.h                           | 38 +++++++++++++
>  lib/tst_test.c                               | 51 +++++++++++++++++
>  testcases/lib/.gitignore                     |  1 +
>  testcases/lib/Makefile                       |  4 +-
>  testcases/lib/run_tests.sh                   | 10 ++++
>  testcases/lib/tests/.gitignore               |  6 ++
>  testcases/lib/tests/Makefile                 | 11 ++++
>  testcases/lib/tests/shell_loader.sh          |  5 ++
>  testcases/lib/tests/shell_test01.c           | 17 ++++++
>  testcases/lib/tests/shell_test02.c           | 18 ++++++
>  testcases/lib/tests/shell_test03.c           | 25 +++++++++
>  testcases/lib/tests/shell_test04.c           | 18 ++++++
>  testcases/lib/tests/shell_test05.c           | 27 +++++++++
>  testcases/lib/tests/shell_test06.c           | 16 ++++++
>  testcases/lib/tests/shell_test_brk.sh        |  6 ++
>  testcases/lib/tests/shell_test_check_argv.sh | 25 +++++++++
>  testcases/lib/tests/shell_test_checkpoint.sh |  7 +++
>  testcases/lib/tests/shell_test_pass.sh       |  6 ++
>  testcases/lib/tst_env.sh                     | 16 ++++++
>  testcases/lib/tst_res_.c                     | 58 ++++++++++++++++++++
>  20 files changed, 363 insertions(+), 2 deletions(-)
>  create mode 100755 testcases/lib/run_tests.sh
>  create mode 100644 testcases/lib/tests/.gitignore
>  create mode 100644 testcases/lib/tests/Makefile
>  create mode 100644 testcases/lib/tests/shell_loader.sh
>  create mode 100644 testcases/lib/tests/shell_test01.c
>  create mode 100644 testcases/lib/tests/shell_test02.c
>  create mode 100644 testcases/lib/tests/shell_test03.c
>  create mode 100644 testcases/lib/tests/shell_test04.c
>  create mode 100644 testcases/lib/tests/shell_test05.c
>  create mode 100644 testcases/lib/tests/shell_test06.c
>  create mode 100755 testcases/lib/tests/shell_test_brk.sh
>  create mode 100755 testcases/lib/tests/shell_test_check_argv.sh
>  create mode 100755 testcases/lib/tests/shell_test_checkpoint.sh
>  create mode 100755 testcases/lib/tests/shell_test_pass.sh
>  create mode 100644 testcases/lib/tst_env.sh
>  create mode 100644 testcases/lib/tst_res_.c
>
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 517c8d032..fde8a1414 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -331,6 +331,8 @@ struct tst_fs {
>   * @child_needs_reinit: Has to be set if the test needs to call
> tst_reinit()
>   *                      from a process started by exec().
>   *
> + * @runs_shell: Implies child_needs_reinit and forks_child at the moment.
> + *
>   * @needs_devfs: If set the devfs is mounted at tst_test.mntpoint. This is
>   *               needed for tests that need to create device files since
> tmpfs
>   *               at /tmp is usually mounted with 'nodev' option.
> @@ -514,6 +516,7 @@ struct tst_fs {
>         unsigned int mount_device:1;
>         unsigned int needs_rofs:1;
>         unsigned int child_needs_reinit:1;
> +       unsigned int runs_shell:1;
>         unsigned int needs_devfs:1;
>         unsigned int restore_wallclock:1;
>
> @@ -522,6 +525,8 @@ struct tst_fs {
>         unsigned int skip_in_lockdown:1;
>         unsigned int skip_in_secureboot:1;
>         unsigned int skip_in_compat:1;
> +
> +
>         int needs_abi_bits;
>
>         unsigned int needs_hugetlbfs:1;
> @@ -607,6 +612,39 @@ void tst_run_tcases(int argc, char *argv[], struct
> tst_test *self)
>   */
>  void tst_reinit(void);
>
> +/**
> + * tst_run_shell() - Prepare the environment and execute a shell script.
> + *
> + * @script_name: A filename of the script.
> + * @params: A NULL terminated array of shell script parameters, pass NULL
> if
> + *          none are needed. This what is passed starting from argv[1].
> + *
> + * The shell script is executed with LTP_IPC_PATH in environment so that
> the
> + * binary helpers such as tst_res_ or tst_checkpoint work properly when
> executed
> + * from the script. This also means that the tst_test.runs_shell flag
> needs to
> + * be set.
> + *
> + * The shell script itself has to source the tst_env.sh shell script at
> the
> + * start and after that it's free to use tst_res in the same way C code
> would
> + * use.
> + *
> + * Example shell script that reports success::
> + *
> + *   #!/bin/sh
> + *   . tst_env.sh
> + *
> + *   tst_res TPASS "Example test works"
> + *
> + * The call returns a pid in a case that you want to examine the return
> value
> + * of the script yourself. If you do not need to check the return value
> + * yourself you can use tst_reap_children() to wait for the completion.
> Or let
> + * the test library collect the child automatically, just be wary that the
> + * script and the test both runs concurently at the same time in this
> case.
> + *
> + * Return: A pid of the shell process.
> + */
> +int tst_run_shell(char *script_name, char *const params[]);
> +
>  unsigned int tst_multiply_timeout(unsigned int timeout);
>
>  /*
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index e5bc5bf4d..fa0907353 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -173,6 +173,52 @@ void tst_reinit(void)
>         SAFE_CLOSE(fd);
>  }
>
> +extern char **environ;
> +
> +static unsigned int params_array_len(char *const array[])
> +{
> +       unsigned int ret = 0;
> +
> +       if (!array)
> +               return 0;
> +
> +       while (*(array++))
> +               ret++;
> +
> +       return ret;
> +}
> +
> +int tst_run_shell(char *script_name, char *const params[])
> +{
> +       int pid;
> +       unsigned int i, params_len = params_array_len(params);
> +       char *argv[params_len + 2] = {};
> +
> +       if (!tst_test->runs_shell)
> +               tst_brk(TBROK, "runs_shell flag must be set!");
> +
> +       argv[0] = script_name;
> +
> +       if (params) {
> +               for (i = 0; i < params_len; i++)
> +                       argv[i+1] = params[i];
> +       }
> +
> +       pid = SAFE_FORK();
> +       if (pid)
> +               return pid;
> +
> +       char *script_name_env = NULL;
> +       SAFE_ASPRINTF(&script_name_env, "LTP_SCRIPT_NAME=%s", script_name);
> +       putenv(script_name_env);
> +
> +       execvpe(script_name, argv, environ);
>

Since execvpe() is a GNU extension, we need to ensure that
we are compiling with GNU extensions enabled.

add this line into the head of tst_test.c:
#define _GNU_SOURCE


+
> +       tst_brk(TBROK | TERRNO, "execv(%s, ...) failed!", script_name);
> +
> +       return -1;
> +}
> +
>  static void update_results(int ttype)
>  {
>         if (!results)
> @@ -1224,6 +1270,11 @@ static void do_setup(int argc, char *argv[])
>                 tdebug = 1;
>         }
>
> +       if (tst_test->runs_shell) {
> +               tst_test->child_needs_reinit = 1;
> +               tst_test->forks_child = 1;
> +       }
> +
>         if (tst_test->needs_kconfigs &&
> tst_kconfig_check(tst_test->needs_kconfigs))
>                 tst_brk(TCONF, "Aborting due to unsuitable kernel config,
> see above!");
>
> diff --git a/testcases/lib/.gitignore b/testcases/lib/.gitignore
> index e8afd06f3..d0dacf62a 100644
> --- a/testcases/lib/.gitignore
> +++ b/testcases/lib/.gitignore
> @@ -23,3 +23,4 @@
>  /tst_sleep
>  /tst_supported_fs
>  /tst_timeout_kill
> +/tst_res_
> diff --git a/testcases/lib/Makefile b/testcases/lib/Makefile
> index 990b46089..928d76d62 100644
> --- a/testcases/lib/Makefile
> +++ b/testcases/lib/Makefile
> @@ -13,6 +13,6 @@ MAKE_TARGETS          := tst_sleep tst_random
> tst_checkpoint tst_rod tst_kvcmp\
>                            tst_getconf tst_supported_fs tst_check_drivers
> tst_get_unused_port\
>                            tst_get_median tst_hexdump tst_get_free_pids
> tst_timeout_kill\
>                            tst_check_kconfigs tst_cgctl tst_fsfreeze
> tst_ns_create tst_ns_exec\
> -                          tst_ns_ifmove tst_lockdown_enabled
> tst_secureboot_enabled
> +                          tst_ns_ifmove tst_lockdown_enabled
> tst_secureboot_enabled tst_res_
>
> -include $(top_srcdir)/include/mk/generic_leaf_target.mk
> +include $(top_srcdir)/include/mk/generic_trunk_target.mk
> diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
> new file mode 100755
> index 000000000..88607b146
> --- /dev/null
> +++ b/testcases/lib/run_tests.sh
> @@ -0,0 +1,10 @@
> +#!/bin/sh
> +
> +export PATH=$PATH:$PWD:$PWD/tests/
> +
> +./tests/shell_test01
> +./tests/shell_test02
> +./tests/shell_test03
> +./tests/shell_test04
> +./tests/shell_test05
> +./tests/shell_test06
> diff --git a/testcases/lib/tests/.gitignore
> b/testcases/lib/tests/.gitignore
> new file mode 100644
> index 000000000..da967c4d6
> --- /dev/null
> +++ b/testcases/lib/tests/.gitignore
> @@ -0,0 +1,6 @@
> +shell_test01
> +shell_test02
> +shell_test03
> +shell_test04
> +shell_test05
> +shell_test06
> diff --git a/testcases/lib/tests/Makefile b/testcases/lib/tests/Makefile
> new file mode 100644
> index 000000000..5a5cf5310
> --- /dev/null
> +++ b/testcases/lib/tests/Makefile
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (C) 2009, Cisco Systems Inc.
> +# Ngie Cooper, August 2009
> +
> +top_srcdir             ?= ../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +
> +INSTALL_TARGETS=
> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/lib/tests/shell_loader.sh
> b/testcases/lib/tests/shell_loader.sh
> new file mode 100644
> index 000000000..c3b3cf5fd
> --- /dev/null
> +++ b/testcases/lib/tests/shell_loader.sh
> @@ -0,0 +1,5 @@
> +#!/usr/bin/env tst_run_shell
> +
> +. tst_env.sh
> +
> +tst_res TPASS "Shell loader works fine!"
> diff --git a/testcases/lib/tests/shell_test01.c
> b/testcases/lib/tests/shell_test01.c
> new file mode 100644
> index 000000000..18f834138
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test01.c
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Shell test example.
> + */
> +
> +#include "tst_test.h"
> +
> +static void run_test(void)
> +{
> +       tst_run_shell("shell_test_pass.sh", NULL);
> +       tst_res(TINFO, "C test exits now");
> +}
> +
> +static struct tst_test test = {
> +       .runs_shell = 1,
> +       .test_all = run_test,
> +};
> diff --git a/testcases/lib/tests/shell_test02.c
> b/testcases/lib/tests/shell_test02.c
> new file mode 100644
> index 000000000..3b0534370
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test02.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Shell test example.
> + */
> +
> +#include "tst_test.h"
> +
> +static void run_test(void)
> +{
> +       tst_run_shell("shell_test_pass.sh", NULL);
> +       tst_reap_children();
> +       tst_res(TINFO, "Shell test has finished at this point!");
> +}
> +
> +static struct tst_test test = {
> +       .runs_shell = 1,
> +       .test_all = run_test,
> +};
> diff --git a/testcases/lib/tests/shell_test03.c
> b/testcases/lib/tests/shell_test03.c
> new file mode 100644
> index 000000000..c11a09e4e
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test03.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Shell test example.
> + */
> +
> +#include <sys/wait.h>
> +#include "tst_test.h"
> +
> +static void run_test(void)
> +{
> +       int pid, status;
> +
> +       pid = tst_run_shell("shell_test_pass.sh", NULL);
> +
> +       tst_res(TINFO, "Waiting for the pid %i", pid);
> +
> +       waitpid(pid, &status, 0);
> +
> +       tst_res(TINFO, "Shell test has %s", tst_strstatus(status));
> +}
> +
> +static struct tst_test test = {
> +       .runs_shell = 1,
> +       .test_all = run_test,
> +};
> diff --git a/testcases/lib/tests/shell_test04.c
> b/testcases/lib/tests/shell_test04.c
> new file mode 100644
> index 000000000..fd1fd122c
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test04.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Shell test example.
> + */
> +
> +#include "tst_test.h"
> +
> +static void run_test(void)
> +{
> +       char *const params[] = {"param1", "param2", NULL};
> +
> +       tst_run_shell("shell_test_check_argv.sh", params);
> +}
> +
> +static struct tst_test test = {
> +       .runs_shell = 1,
> +       .test_all = run_test,
> +};
> diff --git a/testcases/lib/tests/shell_test05.c
> b/testcases/lib/tests/shell_test05.c
> new file mode 100644
> index 000000000..2141d4790
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test05.c
> @@ -0,0 +1,27 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Shell test example.
> + */
> +
> +#include "tst_test.h"
> +
> +static void run_test(void)
> +{
> +       int pid;
> +
> +       pid = tst_run_shell("shell_test_checkpoint.sh", NULL);
> +
> +       tst_res(TINFO, "Waiting for shell to sleep on checkpoint!");
> +
> +       TST_PROCESS_STATE_WAIT(pid, 'S', 10000);
> +
> +       tst_res(TINFO, "Waking shell child!");
> +
> +       TST_CHECKPOINT_WAKE(0);
> +}
> +
> +static struct tst_test test = {
> +       .runs_shell = 1,
> +       .needs_checkpoints = 1,
> +       .test_all = run_test,
> +};
> diff --git a/testcases/lib/tests/shell_test06.c
> b/testcases/lib/tests/shell_test06.c
> new file mode 100644
> index 000000000..25abc1e7b
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test06.c
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Shell test example.
> + */
> +
> +#include "tst_test.h"
> +
> +static void run_test(void)
> +{
> +       tst_run_shell("shell_test_brk.sh", NULL);
> +}
> +
> +static struct tst_test test = {
> +       .runs_shell = 1,
> +       .test_all = run_test,
> +};
> diff --git a/testcases/lib/tests/shell_test_brk.sh
> b/testcases/lib/tests/shell_test_brk.sh
> new file mode 100755
> index 000000000..f266dc3fe
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test_brk.sh
> @@ -0,0 +1,6 @@
> +#!/bin/sh
> +
> +. tst_env.sh
> +
> +tst_brk TCONF "This exits test and the next message should not be reached"
> +tst_res TFAIL "If you see this the test failed"
> diff --git a/testcases/lib/tests/shell_test_check_argv.sh
> b/testcases/lib/tests/shell_test_check_argv.sh
> new file mode 100755
> index 000000000..756189ee7
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test_check_argv.sh
> @@ -0,0 +1,25 @@
> +#!/bin/sh
> +
> +. tst_env.sh
> +
> +echo "$@"
> +
> +tst_res TINFO "argv = $@"
> +
> +if [ $# -ne 2 ]; then
> +       tst_res TFAIL "Wrong number of parameters got $# expected 2"
> +else
> +       tst_res TPASS "Got 2 parameters"
> +fi
> +
> +if [ "$1" != "param1" ]; then
> +       tst_res TFAIL "First parameter is $1 expected param1"
> +else
> +       tst_res TPASS "First parameter is $1"
> +fi
> +
> +if [ "$2" != "param2" ]; then
> +       tst_res TFAIL "Second parameter is $2 expected param2"
> +else
> +       tst_res TPASS "Second parameter is $2"
> +fi
> diff --git a/testcases/lib/tests/shell_test_checkpoint.sh
> b/testcases/lib/tests/shell_test_checkpoint.sh
> new file mode 100755
> index 000000000..0ceb7cf66
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test_checkpoint.sh
> @@ -0,0 +1,7 @@
> +#!/bin/sh
> +
> +. tst_env.sh
> +
> +tst_res TINFO "Waiting for a checkpoint 0"
> +tst_checkpoint wait 10000 0
> +tst_res TPASS "Continuing after checkpoint"
> diff --git a/testcases/lib/tests/shell_test_pass.sh
> b/testcases/lib/tests/shell_test_pass.sh
> new file mode 100755
> index 000000000..fd0684eb2
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test_pass.sh
> @@ -0,0 +1,6 @@
> +#!/bin/sh
> +
> +. tst_env.sh
> +
> +tst_res TPASS "This is called from the shell script!"
> +tst_sleep 100ms
> diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh
> new file mode 100644
> index 000000000..c8f3c2160
> --- /dev/null
> +++ b/testcases/lib/tst_env.sh
> @@ -0,0 +1,16 @@
> +#!/bin/sh
> +
> +tst_script_name=$(basename $0)
> +
> +tst_brk_()
> +{
> +       tst_res_ $@
> +
> +       case "$3" in
> +               "TBROK") exit 2;;
> +               *) exit 0;;
> +       esac
> +}
> +
> +alias tst_res="tst_res_ $tst_script_name \$LINENO"
> +alias tst_brk="tst_brk_ $tst_script_name \$LINENO"
> diff --git a/testcases/lib/tst_res_.c b/testcases/lib/tst_res_.c
> new file mode 100644
> index 000000000..cc3fa53d3
> --- /dev/null
> +++ b/testcases/lib/tst_res_.c
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2024 Cyril Hrubis <chrubis@suse.cz>
> + */
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +
> +static void print_help(void)
> +{
> +       printf("Usage: tst_res_ filename lineno
> [TPASS|TFAIL|TCONF|TINFO|TDEBUG] 'A short description'\n");
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +       int type, i;
> +
> +       if (argc < 5)
> +               goto help;
> +
> +       if (!strcmp(argv[3], "TPASS"))
> +               type = TPASS;
> +       else if (!strcmp(argv[3], "TFAIL"))
> +               type = TFAIL;
> +       else if (!strcmp(argv[3], "TCONF"))
> +               type = TCONF;
> +       else if (!strcmp(argv[3], "TINFO"))
> +               type = TINFO;
> +       else if (!strcmp(argv[3], "TDEBUG"))
> +               type = TDEBUG;
> +       else
> +               goto help;
> +
> +       size_t len = 0;
> +
> +       for (i = 4; i < argc; i++)
> +               len += strlen(argv[i]) + 1;
> +
> +       char *msg = SAFE_MALLOC(len);
> +       char *msgp = msg;
> +
> +       for (i = 4; i < argc; i++) {
> +               msgp = strcpy(msgp, argv[i]);
> +               msgp += strlen(argv[i]);
> +               *(msgp++) = ' ';
> +       }
> +
> +       *(msgp - 1) = 0;
> +
> +       tst_reinit();
> +
> +       tst_res_(argv[1], atoi(argv[2]), type, msg);
>


tst_res_.c:52:9: error: format not a string literal and no format arguments
[-Werror=format-security]
   52 |         tst_res_(argv[1], atoi(argv[2]), type, msg);
      |         ^~~~~~~~
cc1: some warnings being treated as errors
make: *** [../../include/mk/rules.mk:45: tst_res_] Error 1



> +
> +       return 0;
> +help:
> +       print_help();
> +       return 1;
> +}
> --
> 2.44.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] 25+ messages in thread

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-17  8:33   ` Li Wang
@ 2024-07-17  8:41     ` Cyril Hrubis
  2024-07-17 10:07       ` Li Wang
  2024-07-17  8:52     ` Cyril Hrubis
  1 sibling, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2024-07-17  8:41 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

Hi!
> tst_res_.c:52:9: error: format not a string literal and no format arguments
> [-Werror=format-security]
>    52 |         tst_res_(argv[1], atoi(argv[2]), type, msg);
>       |         ^~~~~~~~

Right, this needs:

tst_res_(argv[1], atoi(argv[2]), type, "%s", msg);

I wonder why I didn't get warning at my end.

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-17  8:33   ` Li Wang
  2024-07-17  8:41     ` Cyril Hrubis
@ 2024-07-17  8:52     ` Cyril Hrubis
  2024-07-17 10:21       ` Li Wang
  1 sibling, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2024-07-17  8:52 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

Hi!
> Since execvpe() is a GNU extension, we need to ensure that
> we are compiling with GNU extensions enabled.
> 
> add this line into the head of tst_test.c:
> #define _GNU_SOURCE

As for the execvpe() I've used that for the prototype but I'm unsure if
it could be used in the final product, since this is the test library it
has to compile on all kinds of libc out there. It looks like musl does
support it but I haven't checked Android bionic.

So we may need to write our own implemtantion on the top of the execve()
syscall. But that should be as easy as getting the path to the script
before we pass it to execve().

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-17  8:41     ` Cyril Hrubis
@ 2024-07-17 10:07       ` Li Wang
  0 siblings, 0 replies; 25+ messages in thread
From: Li Wang @ 2024-07-17 10:07 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

On Wed, Jul 17, 2024 at 4:38 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > tst_res_.c:52:9: error: format not a string literal and no format
> arguments
> > [-Werror=format-security]
> >    52 |         tst_res_(argv[1], atoi(argv[2]), type, msg);
> >       |         ^~~~~~~~
>
> Right, this needs:
>
> tst_res_(argv[1], atoi(argv[2]), type, "%s", msg);
>
> I wonder why I didn't get warning at my end.
>

Probably because my GCC version is newer:
   gcc version 14.1.1 20240607 (Red Hat 14.1.1-5) (GCC)


-- 
Regards,
Li Wang

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-17  8:52     ` Cyril Hrubis
@ 2024-07-17 10:21       ` Li Wang
  2024-07-17 17:51         ` Petr Vorel
  2024-07-17 17:55         ` Petr Vorel
  0 siblings, 2 replies; 25+ messages in thread
From: Li Wang @ 2024-07-17 10:21 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

On Wed, Jul 17, 2024 at 4:49 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > Since execvpe() is a GNU extension, we need to ensure that
> > we are compiling with GNU extensions enabled.
> >
> > add this line into the head of tst_test.c:
> > #define _GNU_SOURCE
>
> As for the execvpe() I've used that for the prototype but I'm unsure if
> it could be used in the final product, since this is the test library it
> has to compile on all kinds of libc out there. It looks like musl does
> support it but I haven't checked Android bionic.


> So we may need to write our own implemtantion on the top of the execve()
> syscall. But that should be as easy as getting the path to the script
> before we pass it to execve().
>

Indeed.  We need to create a ltp_execvpe() based on execve().

-- 
Regards,
Li Wang

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-17 10:21       ` Li Wang
@ 2024-07-17 17:51         ` Petr Vorel
  2024-07-17 17:55         ` Petr Vorel
  1 sibling, 0 replies; 25+ messages in thread
From: Petr Vorel @ 2024-07-17 17:51 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

Hi all,

> On Wed, Jul 17, 2024 at 4:49 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> > Hi!
> > > Since execvpe() is a GNU extension, we need to ensure that
> > > we are compiling with GNU extensions enabled.

> > > add this line into the head of tst_test.c:
> > > #define _GNU_SOURCE

Good, you already noticed.

> > As for the execvpe() I've used that for the prototype but I'm unsure if
> > it could be used in the final product, since this is the test library it
> > has to compile on all kinds of libc out there. It looks like musl does
> > support it but I haven't checked Android bionic.


> > So we may need to write our own implemtantion on the top of the execve()
> > syscall. But that should be as easy as getting the path to the script
> > before we pass it to execve().

It supports it. It looks like even without _GNU_SOURCE:
https://android.googlesource.com/platform/bionic.git/+/refs/heads/main/libc/include/unistd.h#142

But better to keep Edward in loop.

> Indeed.  We need to create a ltp_execvpe() based on execve().

So there will be v2, right?

Kind regards,
Petr

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-17 10:21       ` Li Wang
  2024-07-17 17:51         ` Petr Vorel
@ 2024-07-17 17:55         ` Petr Vorel
  2024-07-17 18:13           ` Cyril Hrubis
  1 sibling, 1 reply; 25+ messages in thread
From: Petr Vorel @ 2024-07-17 17:55 UTC (permalink / raw)
  To: Li Wang; +Cc: ltp

Hi all,

> On Wed, Jul 17, 2024 at 4:49 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> > Hi!
> > > Since execvpe() is a GNU extension, we need to ensure that
> > > we are compiling with GNU extensions enabled.

> > > add this line into the head of tst_test.c:
> > > #define _GNU_SOURCE

FYI besides this (which brings CI build) it fails to compile also on musl:

https://github.com/pevik/ltp/actions/runs/9978962369/job/27577025071

tst_res_.c: In function 'main':
tst_res_.c:52:9: error: format not a string literal and no format arguments [-Werror=format-security]
   52 |         tst_res_(argv[1], atoi(argv[2]), type, msg);


Kind regards,
Petr

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-17 17:55         ` Petr Vorel
@ 2024-07-17 18:13           ` Cyril Hrubis
  0 siblings, 0 replies; 25+ messages in thread
From: Cyril Hrubis @ 2024-07-17 18:13 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi!
> tst_res_.c: In function 'main':
> tst_res_.c:52:9: error: format not a string literal and no format arguments [-Werror=format-security]
>    52 |         tst_res_(argv[1], atoi(argv[2]), type, msg);

Already reported by Li, it's missing the format string "%s" before the
msg.

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-16 15:36 ` [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code Cyril Hrubis
  2024-07-17  8:33   ` Li Wang
@ 2024-07-18 12:43   ` Petr Vorel
  2024-07-18 13:03     ` Cyril Hrubis
  2024-07-18 12:57   ` Petr Vorel
  2024-07-24  9:37   ` Martin Doucha
  3 siblings, 1 reply; 25+ messages in thread
From: Petr Vorel @ 2024-07-18 12:43 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

Hi all,

one more thing to fix (or did I overlook the discussion)?

tst_test.c: In function ‘tst_run_shell’:
tst_test.c:196:2: error: variable-sized object may not be initialized
  char *argv[params_len + 2] = {};
  ^~~~

Found on gcc in SLES 15-SP6:
gcc --version
gcc (SUSE Linux) 7.5.0

Kind regards,
Petr

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-16 15:36 ` [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code Cyril Hrubis
  2024-07-17  8:33   ` Li Wang
  2024-07-18 12:43   ` Petr Vorel
@ 2024-07-18 12:57   ` Petr Vorel
  2024-07-18 13:07     ` Cyril Hrubis
  2024-07-24  9:37   ` Martin Doucha
  3 siblings, 1 reply; 25+ messages in thread
From: Petr Vorel @ 2024-07-18 12:57 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

Hi Cyril,

> This is a proof of a concept of a seamless C and shell integration. The
> idea is that with this you can mix shell and C code as much as as you
> wish to get the best of the two worlds.

> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  include/tst_test.h                           | 38 +++++++++++++
>  lib/tst_test.c                               | 51 +++++++++++++++++
>  testcases/lib/.gitignore                     |  1 +
>  testcases/lib/Makefile                       |  4 +-
>  testcases/lib/run_tests.sh                   | 10 ++++
>  testcases/lib/tests/.gitignore               |  6 ++
>  testcases/lib/tests/Makefile                 | 11 ++++
>  testcases/lib/tests/shell_loader.sh          |  5 ++
>  testcases/lib/tests/shell_test01.c           | 17 ++++++
>  testcases/lib/tests/shell_test02.c           | 18 ++++++
>  testcases/lib/tests/shell_test03.c           | 25 +++++++++
>  testcases/lib/tests/shell_test04.c           | 18 ++++++
>  testcases/lib/tests/shell_test05.c           | 27 +++++++++
>  testcases/lib/tests/shell_test06.c           | 16 ++++++
FYI we have shell tests for new library in lib/newlib_tests (C tests) and
lib/newlib_tests/shell/ (shell tests), is it necessary to add new location? Or,
if you prefer this, we should move existing tests from lib/newlib_tests/shell/
to this new location.

Also, we have lib/newlib_tests/runtest.sh script, which runs currently only te
tests which exit 0 (TPASS or TCONF). Here are LTP_SHELL_API_TESTS listed.

Also we have test-shell target in the top level Makefile.

=> these tests (currently only these which exits 0) should be run by test-shell
and thus in CI.

Generally the approach LGTM.

>  	unsigned int needs_hugetlbfs:1;
> @@ -607,6 +612,39 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
>   */
>  void tst_reinit(void);

> +/**
> + * tst_run_shell() - Prepare the environment and execute a shell script.
> + *
> + * @script_name: A filename of the script.
> + * @params: A NULL terminated array of shell script parameters, pass NULL if
> + *          none are needed. This what is passed starting from argv[1].
> + *
> + * The shell script is executed with LTP_IPC_PATH in environment so that the
> + * binary helpers such as tst_res_ or tst_checkpoint work properly when executed
> + * from the script. This also means that the tst_test.runs_shell flag needs to
> + * be set.
> + *
> + * The shell script itself has to source the tst_env.sh shell script at the
> + * start and after that it's free to use tst_res in the same way C code would
> + * use.
> + *
> + * Example shell script that reports success::
nit: double :

> + *
> + *   #!/bin/sh
> + *   . tst_env.sh
> + *
> + *   tst_res TPASS "Example test works"
> + *
> + * The call returns a pid in a case that you want to examine the return value
> + * of the script yourself. If you do not need to check the return value
> + * yourself you can use tst_reap_children() to wait for the completion. Or let
> + * the test library collect the child automatically, just be wary that the
> + * script and the test both runs concurently at the same time in this case.
> + *
> + * Return: A pid of the shell process.
> + */
> +int tst_run_shell(char *script_name, char *const params[]);
> +
>  unsigned int tst_multiply_timeout(unsigned int timeout);

>  /*
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index e5bc5bf4d..fa0907353 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -173,6 +173,52 @@ void tst_reinit(void)
>  	SAFE_CLOSE(fd);
>  }
...

> +int tst_run_shell(char *script_name, char *const params[])
> +{
> +	int pid;
> +	unsigned int i, params_len = params_array_len(params);
> +	char *argv[params_len + 2] = {};
As I noted, this is a problem for old gcc 7 (still used in 15-SP6).

Kind regards,
Petr

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-18 12:43   ` Petr Vorel
@ 2024-07-18 13:03     ` Cyril Hrubis
  0 siblings, 0 replies; 25+ messages in thread
From: Cyril Hrubis @ 2024-07-18 13:03 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi!
> one more thing to fix (or did I overlook the discussion)?
> 
> tst_test.c: In function ‘tst_run_shell’:
> tst_test.c:196:2: error: variable-sized object may not be initialized
>   char *argv[params_len + 2] = {};
>   ^~~~

That's a new one, I will fix it in v2.

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-18 12:57   ` Petr Vorel
@ 2024-07-18 13:07     ` Cyril Hrubis
  2024-07-18 13:15       ` Petr Vorel
  0 siblings, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2024-07-18 13:07 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

Hi!
> > This is a proof of a concept of a seamless C and shell integration. The
> > idea is that with this you can mix shell and C code as much as as you
> > wish to get the best of the two worlds.
> 
> > Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> > ---
> >  include/tst_test.h                           | 38 +++++++++++++
> >  lib/tst_test.c                               | 51 +++++++++++++++++
> >  testcases/lib/.gitignore                     |  1 +
> >  testcases/lib/Makefile                       |  4 +-
> >  testcases/lib/run_tests.sh                   | 10 ++++
> >  testcases/lib/tests/.gitignore               |  6 ++
> >  testcases/lib/tests/Makefile                 | 11 ++++
> >  testcases/lib/tests/shell_loader.sh          |  5 ++
> >  testcases/lib/tests/shell_test01.c           | 17 ++++++
> >  testcases/lib/tests/shell_test02.c           | 18 ++++++
> >  testcases/lib/tests/shell_test03.c           | 25 +++++++++
> >  testcases/lib/tests/shell_test04.c           | 18 ++++++
> >  testcases/lib/tests/shell_test05.c           | 27 +++++++++
> >  testcases/lib/tests/shell_test06.c           | 16 ++++++
> FYI we have shell tests for new library in lib/newlib_tests (C tests) and
> lib/newlib_tests/shell/ (shell tests), is it necessary to add new location? Or,
> if you prefer this, we should move existing tests from lib/newlib_tests/shell/
> to this new location.

For a historical reasons the lib for shell is in testcases/lib/ and the
tests use paths to binaries and scripts in there so I added the code
there. We may as well move it to the top level lib/shell, or create top
level slib (as for shell lib) or anywhere else as long as we agree on a
better place to put the code.

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-18 13:07     ` Cyril Hrubis
@ 2024-07-18 13:15       ` Petr Vorel
  0 siblings, 0 replies; 25+ messages in thread
From: Petr Vorel @ 2024-07-18 13:15 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

> Hi!
> > > This is a proof of a concept of a seamless C and shell integration. The
> > > idea is that with this you can mix shell and C code as much as as you
> > > wish to get the best of the two worlds.

> > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> > > ---
> > >  include/tst_test.h                           | 38 +++++++++++++
> > >  lib/tst_test.c                               | 51 +++++++++++++++++
> > >  testcases/lib/.gitignore                     |  1 +
> > >  testcases/lib/Makefile                       |  4 +-
> > >  testcases/lib/run_tests.sh                   | 10 ++++
> > >  testcases/lib/tests/.gitignore               |  6 ++
> > >  testcases/lib/tests/Makefile                 | 11 ++++
> > >  testcases/lib/tests/shell_loader.sh          |  5 ++
> > >  testcases/lib/tests/shell_test01.c           | 17 ++++++
> > >  testcases/lib/tests/shell_test02.c           | 18 ++++++
> > >  testcases/lib/tests/shell_test03.c           | 25 +++++++++
> > >  testcases/lib/tests/shell_test04.c           | 18 ++++++
> > >  testcases/lib/tests/shell_test05.c           | 27 +++++++++
> > >  testcases/lib/tests/shell_test06.c           | 16 ++++++
> > FYI we have shell tests for new library in lib/newlib_tests (C tests) and
> > lib/newlib_tests/shell/ (shell tests), is it necessary to add new location? Or,
> > if you prefer this, we should move existing tests from lib/newlib_tests/shell/
> > to this new location.

> For a historical reasons the lib for shell is in testcases/lib/ and the
> tests use paths to binaries and scripts in there so I added the code
> there. We may as well move it to the top level lib/shell, or create top
> level slib (as for shell lib) or anywhere else as long as we agree on a
> better place to put the code.

Yeah, we could move everything to lib/shell. I'm also OK with different location
with the tests, i.e. we can keep things where they are.

My main concern is to run the tests in CI and in 'make test' target (also
specify new subtest e.g. test-c-shell: or add them to test-c).

Kind regards,
Petr

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-16 15:36 ` [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code Cyril Hrubis
                     ` (2 preceding siblings ...)
  2024-07-18 12:57   ` Petr Vorel
@ 2024-07-24  9:37   ` Martin Doucha
  2024-07-24  9:47     ` Cyril Hrubis
  2024-07-30  9:49     ` Cyril Hrubis
  3 siblings, 2 replies; 25+ messages in thread
From: Martin Doucha @ 2024-07-24  9:37 UTC (permalink / raw)
  To: Cyril Hrubis, ltp

Hi,
I'll skip bugs reported by others but I have some comments below.

On 16. 07. 24 17:36, Cyril Hrubis wrote:
> This is a proof of a concept of a seamless C and shell integration. The
> idea is that with this you can mix shell and C code as much as as you
> wish to get the best of the two worlds.
> 
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>   include/tst_test.h                           | 38 +++++++++++++
>   lib/tst_test.c                               | 51 +++++++++++++++++
>   testcases/lib/.gitignore                     |  1 +
>   testcases/lib/Makefile                       |  4 +-
>   testcases/lib/run_tests.sh                   | 10 ++++
>   testcases/lib/tests/.gitignore               |  6 ++
>   testcases/lib/tests/Makefile                 | 11 ++++
>   testcases/lib/tests/shell_loader.sh          |  5 ++
>   testcases/lib/tests/shell_test01.c           | 17 ++++++
>   testcases/lib/tests/shell_test02.c           | 18 ++++++
>   testcases/lib/tests/shell_test03.c           | 25 +++++++++
>   testcases/lib/tests/shell_test04.c           | 18 ++++++
>   testcases/lib/tests/shell_test05.c           | 27 +++++++++
>   testcases/lib/tests/shell_test06.c           | 16 ++++++
>   testcases/lib/tests/shell_test_brk.sh        |  6 ++
>   testcases/lib/tests/shell_test_check_argv.sh | 25 +++++++++
>   testcases/lib/tests/shell_test_checkpoint.sh |  7 +++
>   testcases/lib/tests/shell_test_pass.sh       |  6 ++
>   testcases/lib/tst_env.sh                     | 16 ++++++
>   testcases/lib/tst_res_.c                     | 58 ++++++++++++++++++++
>   20 files changed, 363 insertions(+), 2 deletions(-)
>   create mode 100755 testcases/lib/run_tests.sh
>   create mode 100644 testcases/lib/tests/.gitignore
>   create mode 100644 testcases/lib/tests/Makefile
>   create mode 100644 testcases/lib/tests/shell_loader.sh
>   create mode 100644 testcases/lib/tests/shell_test01.c
>   create mode 100644 testcases/lib/tests/shell_test02.c
>   create mode 100644 testcases/lib/tests/shell_test03.c
>   create mode 100644 testcases/lib/tests/shell_test04.c
>   create mode 100644 testcases/lib/tests/shell_test05.c
>   create mode 100644 testcases/lib/tests/shell_test06.c
>   create mode 100755 testcases/lib/tests/shell_test_brk.sh
>   create mode 100755 testcases/lib/tests/shell_test_check_argv.sh
>   create mode 100755 testcases/lib/tests/shell_test_checkpoint.sh
>   create mode 100755 testcases/lib/tests/shell_test_pass.sh
>   create mode 100644 testcases/lib/tst_env.sh
>   create mode 100644 testcases/lib/tst_res_.c
> 
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 517c8d032..fde8a1414 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -331,6 +331,8 @@ struct tst_fs {
>    * @child_needs_reinit: Has to be set if the test needs to call tst_reinit()
>    *                      from a process started by exec().
>    *
> + * @runs_shell: Implies child_needs_reinit and forks_child at the moment.
> + *

This could also work as is for Python or Perl scripts (which we don't 
support at the moment) but consider renaming the attribute to 
.runs_script anyway.

>    * @needs_devfs: If set the devfs is mounted at tst_test.mntpoint. This is
>    *               needed for tests that need to create device files since tmpfs
>    *               at /tmp is usually mounted with 'nodev' option.
> @@ -514,6 +516,7 @@ struct tst_fs {
>   	unsigned int mount_device:1;
>   	unsigned int needs_rofs:1;
>   	unsigned int child_needs_reinit:1;
> +	unsigned int runs_shell:1;
>   	unsigned int needs_devfs:1;
>   	unsigned int restore_wallclock:1;
>   
> @@ -522,6 +525,8 @@ struct tst_fs {
>   	unsigned int skip_in_lockdown:1;
>   	unsigned int skip_in_secureboot:1;
>   	unsigned int skip_in_compat:1;
> +
> +
>   	int needs_abi_bits;
>   
>   	unsigned int needs_hugetlbfs:1;
> @@ -607,6 +612,39 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
>    */
>   void tst_reinit(void);
>   
> +/**
> + * tst_run_shell() - Prepare the environment and execute a shell script.
> + *
> + * @script_name: A filename of the script.
> + * @params: A NULL terminated array of shell script parameters, pass NULL if
> + *          none are needed. This what is passed starting from argv[1].
> + *
> + * The shell script is executed with LTP_IPC_PATH in environment so that the
> + * binary helpers such as tst_res_ or tst_checkpoint work properly when executed
> + * from the script. This also means that the tst_test.runs_shell flag needs to
> + * be set.
> + *
> + * The shell script itself has to source the tst_env.sh shell script at the
> + * start and after that it's free to use tst_res in the same way C code would
> + * use.
> + *
> + * Example shell script that reports success::
> + *
> + *   #!/bin/sh
> + *   . tst_env.sh
> + *
> + *   tst_res TPASS "Example test works"
> + *
> + * The call returns a pid in a case that you want to examine the return value
> + * of the script yourself. If you do not need to check the return value
> + * yourself you can use tst_reap_children() to wait for the completion. Or let
> + * the test library collect the child automatically, just be wary that the
> + * script and the test both runs concurently at the same time in this case.
> + *
> + * Return: A pid of the shell process.
> + */
> +int tst_run_shell(char *script_name, char *const params[]);
> +
>   unsigned int tst_multiply_timeout(unsigned int timeout);
>   
>   /*
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index e5bc5bf4d..fa0907353 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -173,6 +173,52 @@ void tst_reinit(void)
>   	SAFE_CLOSE(fd);
>   }
>   
> +extern char **environ;
> +
> +static unsigned int params_array_len(char *const array[])
> +{
> +	unsigned int ret = 0;
> +
> +	if (!array)
> +		return 0;
> +
> +	while (*(array++))
> +		ret++;
> +
> +	return ret;
> +}
> +
> +int tst_run_shell(char *script_name, char *const params[])
> +{
> +	int pid;
> +	unsigned int i, params_len = params_array_len(params);
> +	char *argv[params_len + 2] = {};
> +
> +	if (!tst_test->runs_shell)
> +		tst_brk(TBROK, "runs_shell flag must be set!");
> +
> +	argv[0] = script_name;
> +
> +	if (params) {
> +		for (i = 0; i < params_len; i++)
> +			argv[i+1] = params[i];
> +	}
> +
> +	pid = SAFE_FORK();
> +	if (pid)
> +		return pid;
> +
> +	char *script_name_env = NULL;
> +	SAFE_ASPRINTF(&script_name_env, "LTP_SCRIPT_NAME=%s", script_name);
> +	putenv(script_name_env);
> +
> +	execvpe(script_name, argv, environ);
> +
> +	tst_brk(TBROK | TERRNO, "execv(%s, ...) failed!", script_name);
> +
> +	return -1;
> +}
> +
>   static void update_results(int ttype)
>   {
>   	if (!results)
> @@ -1224,6 +1270,11 @@ static void do_setup(int argc, char *argv[])
>   		tdebug = 1;
>   	}
>   
> +	if (tst_test->runs_shell) {
> +		tst_test->child_needs_reinit = 1;
> +		tst_test->forks_child = 1;
> +	}
> +
>   	if (tst_test->needs_kconfigs && tst_kconfig_check(tst_test->needs_kconfigs))
>   		tst_brk(TCONF, "Aborting due to unsuitable kernel config, see above!");
>   
> diff --git a/testcases/lib/.gitignore b/testcases/lib/.gitignore
> index e8afd06f3..d0dacf62a 100644
> --- a/testcases/lib/.gitignore
> +++ b/testcases/lib/.gitignore
> @@ -23,3 +23,4 @@
>   /tst_sleep
>   /tst_supported_fs
>   /tst_timeout_kill
> +/tst_res_
> diff --git a/testcases/lib/Makefile b/testcases/lib/Makefile
> index 990b46089..928d76d62 100644
> --- a/testcases/lib/Makefile
> +++ b/testcases/lib/Makefile
> @@ -13,6 +13,6 @@ MAKE_TARGETS		:= tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
>   			   tst_getconf tst_supported_fs tst_check_drivers tst_get_unused_port\
>   			   tst_get_median tst_hexdump tst_get_free_pids tst_timeout_kill\
>   			   tst_check_kconfigs tst_cgctl tst_fsfreeze tst_ns_create tst_ns_exec\
> -			   tst_ns_ifmove tst_lockdown_enabled tst_secureboot_enabled
> +			   tst_ns_ifmove tst_lockdown_enabled tst_secureboot_enabled tst_res_
>   
> -include $(top_srcdir)/include/mk/generic_leaf_target.mk
> +include $(top_srcdir)/include/mk/generic_trunk_target.mk
> diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
> new file mode 100755
> index 000000000..88607b146
> --- /dev/null
> +++ b/testcases/lib/run_tests.sh
> @@ -0,0 +1,10 @@
> +#!/bin/sh
> +
> +export PATH=$PATH:$PWD:$PWD/tests/

$PWD is not the correct path. It should be set as follows:

testdir=$(dirname $0)
export PATH=$PATH:$testdir:$testdir/tests/

> +
> +./tests/shell_test01
> +./tests/shell_test02
> +./tests/shell_test03
> +./tests/shell_test04
> +./tests/shell_test05
> +./tests/shell_test06
> diff --git a/testcases/lib/tests/.gitignore b/testcases/lib/tests/.gitignore
> new file mode 100644
> index 000000000..da967c4d6
> --- /dev/null
> +++ b/testcases/lib/tests/.gitignore
> @@ -0,0 +1,6 @@
> +shell_test01
> +shell_test02
> +shell_test03
> +shell_test04
> +shell_test05
> +shell_test06
> diff --git a/testcases/lib/tests/Makefile b/testcases/lib/tests/Makefile
> new file mode 100644
> index 000000000..5a5cf5310
> --- /dev/null
> +++ b/testcases/lib/tests/Makefile
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (C) 2009, Cisco Systems Inc.
> +# Ngie Cooper, August 2009
> +
> +top_srcdir		?= ../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +
> +INSTALL_TARGETS=
> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/lib/tests/shell_loader.sh b/testcases/lib/tests/shell_loader.sh
> new file mode 100644
> index 000000000..c3b3cf5fd
> --- /dev/null
> +++ b/testcases/lib/tests/shell_loader.sh
> @@ -0,0 +1,5 @@
> +#!/usr/bin/env tst_run_shell
> +
> +. tst_env.sh
> +
> +tst_res TPASS "Shell loader works fine!"
> diff --git a/testcases/lib/tests/shell_test01.c b/testcases/lib/tests/shell_test01.c
> new file mode 100644
> index 000000000..18f834138
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test01.c
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Shell test example.
> + */
> +
> +#include "tst_test.h"
> +
> +static void run_test(void)
> +{
> +	tst_run_shell("shell_test_pass.sh", NULL);
> +	tst_res(TINFO, "C test exits now");
> +}
> +
> +static struct tst_test test = {
> +	.runs_shell = 1,
> +	.test_all = run_test,
> +};
> diff --git a/testcases/lib/tests/shell_test02.c b/testcases/lib/tests/shell_test02.c
> new file mode 100644
> index 000000000..3b0534370
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test02.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Shell test example.
> + */
> +
> +#include "tst_test.h"
> +
> +static void run_test(void)
> +{
> +	tst_run_shell("shell_test_pass.sh", NULL);
> +	tst_reap_children();
> +	tst_res(TINFO, "Shell test has finished at this point!");
> +}
> +
> +static struct tst_test test = {
> +	.runs_shell = 1,
> +	.test_all = run_test,
> +};
> diff --git a/testcases/lib/tests/shell_test03.c b/testcases/lib/tests/shell_test03.c
> new file mode 100644
> index 000000000..c11a09e4e
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test03.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Shell test example.
> + */
> +
> +#include <sys/wait.h>
> +#include "tst_test.h"
> +
> +static void run_test(void)
> +{
> +	int pid, status;
> +
> +	pid = tst_run_shell("shell_test_pass.sh", NULL);
> +
> +	tst_res(TINFO, "Waiting for the pid %i", pid);
> +
> +	waitpid(pid, &status, 0);
> +
> +	tst_res(TINFO, "Shell test has %s", tst_strstatus(status));
> +}
> +
> +static struct tst_test test = {
> +	.runs_shell = 1,
> +	.test_all = run_test,
> +};
> diff --git a/testcases/lib/tests/shell_test04.c b/testcases/lib/tests/shell_test04.c
> new file mode 100644
> index 000000000..fd1fd122c
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test04.c
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Shell test example.
> + */
> +
> +#include "tst_test.h"
> +
> +static void run_test(void)
> +{
> +	char *const params[] = {"param1", "param2", NULL};
> +
> +	tst_run_shell("shell_test_check_argv.sh", params);
> +}
> +
> +static struct tst_test test = {
> +	.runs_shell = 1,
> +	.test_all = run_test,
> +};
> diff --git a/testcases/lib/tests/shell_test05.c b/testcases/lib/tests/shell_test05.c
> new file mode 100644
> index 000000000..2141d4790
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test05.c
> @@ -0,0 +1,27 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Shell test example.
> + */
> +
> +#include "tst_test.h"
> +
> +static void run_test(void)
> +{
> +	int pid;
> +
> +	pid = tst_run_shell("shell_test_checkpoint.sh", NULL);
> +
> +	tst_res(TINFO, "Waiting for shell to sleep on checkpoint!");
> +
> +	TST_PROCESS_STATE_WAIT(pid, 'S', 10000);
> +
> +	tst_res(TINFO, "Waking shell child!");
> +
> +	TST_CHECKPOINT_WAKE(0);
> +}
> +
> +static struct tst_test test = {
> +	.runs_shell = 1,
> +	.needs_checkpoints = 1,
> +	.test_all = run_test,
> +};
> diff --git a/testcases/lib/tests/shell_test06.c b/testcases/lib/tests/shell_test06.c
> new file mode 100644
> index 000000000..25abc1e7b
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test06.c
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Shell test example.
> + */
> +
> +#include "tst_test.h"
> +
> +static void run_test(void)
> +{
> +	tst_run_shell("shell_test_brk.sh", NULL);
> +}
> +
> +static struct tst_test test = {
> +	.runs_shell = 1,
> +	.test_all = run_test,
> +};
> diff --git a/testcases/lib/tests/shell_test_brk.sh b/testcases/lib/tests/shell_test_brk.sh
> new file mode 100755
> index 000000000..f266dc3fe
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test_brk.sh
> @@ -0,0 +1,6 @@
> +#!/bin/sh
> +
> +. tst_env.sh
> +
> +tst_brk TCONF "This exits test and the next message should not be reached"
> +tst_res TFAIL "If you see this the test failed"
> diff --git a/testcases/lib/tests/shell_test_check_argv.sh b/testcases/lib/tests/shell_test_check_argv.sh
> new file mode 100755
> index 000000000..756189ee7
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test_check_argv.sh
> @@ -0,0 +1,25 @@
> +#!/bin/sh
> +
> +. tst_env.sh
> +
> +echo "$@"
> +
> +tst_res TINFO "argv = $@"
> +
> +if [ $# -ne 2 ]; then
> +	tst_res TFAIL "Wrong number of parameters got $# expected 2"
> +else
> +	tst_res TPASS "Got 2 parameters"
> +fi
> +
> +if [ "$1" != "param1" ]; then
> +	tst_res TFAIL "First parameter is $1 expected param1"
> +else
> +	tst_res TPASS "First parameter is $1"
> +fi
> +
> +if [ "$2" != "param2" ]; then
> +	tst_res TFAIL "Second parameter is $2 expected param2"
> +else
> +	tst_res TPASS "Second parameter is $2"
> +fi
> diff --git a/testcases/lib/tests/shell_test_checkpoint.sh b/testcases/lib/tests/shell_test_checkpoint.sh
> new file mode 100755
> index 000000000..0ceb7cf66
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test_checkpoint.sh
> @@ -0,0 +1,7 @@
> +#!/bin/sh
> +
> +. tst_env.sh
> +
> +tst_res TINFO "Waiting for a checkpoint 0"
> +tst_checkpoint wait 10000 0
> +tst_res TPASS "Continuing after checkpoint"
> diff --git a/testcases/lib/tests/shell_test_pass.sh b/testcases/lib/tests/shell_test_pass.sh
> new file mode 100755
> index 000000000..fd0684eb2
> --- /dev/null
> +++ b/testcases/lib/tests/shell_test_pass.sh
> @@ -0,0 +1,6 @@
> +#!/bin/sh
> +
> +. tst_env.sh
> +
> +tst_res TPASS "This is called from the shell script!"
> +tst_sleep 100ms
> diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh
> new file mode 100644
> index 000000000..c8f3c2160
> --- /dev/null
> +++ b/testcases/lib/tst_env.sh
> @@ -0,0 +1,16 @@
> +#!/bin/sh
> +
> +tst_script_name=$(basename $0)
> +
> +tst_brk_()
> +{
> +	tst_res_ $@
> +
> +	case "$3" in
> +		"TBROK") exit 2;;
> +		*) exit 0;;

The exit command above will ignore any other result flags already set 
during the test. You could modify tst_res_ to return the correct exit 
code and then call it like this:

tst_brk() {
	tst_res_ "$@"
	exit $?
}

Also note the quotation marks around $@. Without them, tst_brk will 
compress whitespace which the caller wants to preserve.

> +	esac
> +}
> +
> +alias tst_res="tst_res_ $tst_script_name \$LINENO"
> +alias tst_brk="tst_brk_ $tst_script_name \$LINENO"
> diff --git a/testcases/lib/tst_res_.c b/testcases/lib/tst_res_.c
> new file mode 100644
> index 000000000..cc3fa53d3
> --- /dev/null
> +++ b/testcases/lib/tst_res_.c
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2024 Cyril Hrubis <chrubis@suse.cz>
> + */
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +
> +static void print_help(void)
> +{
> +	printf("Usage: tst_res_ filename lineno [TPASS|TFAIL|TCONF|TINFO|TDEBUG] 'A short description'\n");
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	int type, i;
> +
> +	if (argc < 5)
> +		goto help;
> +
> +	if (!strcmp(argv[3], "TPASS"))
> +		type = TPASS;
> +	else if (!strcmp(argv[3], "TFAIL"))
> +		type = TFAIL;
> +	else if (!strcmp(argv[3], "TCONF"))
> +		type = TCONF;
> +	else if (!strcmp(argv[3], "TINFO"))
> +		type = TINFO;
> +	else if (!strcmp(argv[3], "TDEBUG"))
> +		type = TDEBUG;
> +	else
> +		goto help;
> +
> +	size_t len = 0;
> +
> +	for (i = 4; i < argc; i++)
> +		len += strlen(argv[i]) + 1;
> +
> +	char *msg = SAFE_MALLOC(len);
> +	char *msgp = msg;
> +
> +	for (i = 4; i < argc; i++) {
> +		msgp = strcpy(msgp, argv[i]);
> +		msgp += strlen(argv[i]);
> +		*(msgp++) = ' ';
> +	}
> +
> +	*(msgp - 1) = 0;
> +
> +	tst_reinit();
> +
> +	tst_res_(argv[1], atoi(argv[2]), type, msg);
> +
> +	return 0;
> +help:
> +	print_help();
> +	return 1;
> +}

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic



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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-24  9:37   ` Martin Doucha
@ 2024-07-24  9:47     ` Cyril Hrubis
  2024-07-24  9:53       ` Martin Doucha
  2024-07-30  9:49     ` Cyril Hrubis
  1 sibling, 1 reply; 25+ messages in thread
From: Cyril Hrubis @ 2024-07-24  9:47 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi!
> The exit command above will ignore any other result flags already set 
> during the test. You could modify tst_res_ to return the correct exit 
> code and then call it like this:

It does not work like this. The tst_res helper modifies the shm memory
that the test library uses, so all test results are propagated once the
tst_res helper returns.

Only the TBROK flag is not propagated via the shared memory in the new
test library which I consider a mistake when I designed the new test
library and we should move it to the shared memory...

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-24  9:47     ` Cyril Hrubis
@ 2024-07-24  9:53       ` Martin Doucha
  2024-07-24 10:05         ` Cyril Hrubis
  0 siblings, 1 reply; 25+ messages in thread
From: Martin Doucha @ 2024-07-24  9:53 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp

On 24. 07. 24 11:47, Cyril Hrubis wrote:
> Hi!
>> The exit command above will ignore any other result flags already set
>> during the test. You could modify tst_res_ to return the correct exit
>> code and then call it like this:
> 
> It does not work like this. The tst_res helper modifies the shm memory
> that the test library uses, so all test results are propagated once the
> tst_res helper returns.
> 
> Only the TBROK flag is not propagated via the shared memory in the new
> test library which I consider a mistake when I designed the new test
> library and we should move it to the shared memory...

That still doesn't solve the incorrect return value from the script if 
it's intentionally possible to run it as a stand-alone test, without any 
parent process written in C.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-24  9:53       ` Martin Doucha
@ 2024-07-24 10:05         ` Cyril Hrubis
  0 siblings, 0 replies; 25+ messages in thread
From: Cyril Hrubis @ 2024-07-24 10:05 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi!
> That still doesn't solve the incorrect return value from the script if 
> it's intentionally possible to run it as a stand-alone test, without any 
> parent process written in C.

I do not think that it will make sense or be possible to use these
scripts without the C library. The whole point of this patchset is to
use as much of the C library as possible so that we do not have to
reimplement it in shell. Which would mean that most of the functionality
needed to run these shell tests will not we available without the C
library.

Maybe these script shouldn't have a shebang and shouldn't have
executable flag so that people are not confused about this. The test
library can run them with /bin/sh $filename instead.

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

* Re: [LTP] [RFC PATCH 2/2] testcaes/lib: Add shell loader
  2024-07-16 15:36 ` [LTP] [RFC PATCH 2/2] testcaes/lib: Add shell loader Cyril Hrubis
@ 2024-07-24 12:54   ` Martin Doucha
  2024-07-25 11:02     ` Petr Vorel
  0 siblings, 1 reply; 25+ messages in thread
From: Martin Doucha @ 2024-07-24 12:54 UTC (permalink / raw)
  To: Cyril Hrubis, ltp

Hi,
one note below.

On 16. 07. 24 17:36, Cyril Hrubis wrote:
> This commit implements a shell loader so that we don't have to write a C
> loader for each LTP shell test. The idea is simple, the loader parses
> the shell test and prepares the tst_test structure accordingly, then
> runs the actual shell test. Only small subset of the flags is
> implemented at the moment, since this is RFC, however it should be clear
> where this is going.
> 
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>   testcases/lib/.gitignore                      |  1 +
>   testcases/lib/Makefile                        |  3 +-
>   testcases/lib/run_tests.sh                    |  2 +
>   testcases/lib/tests/shell_loader.sh           | 11 ++-
>   .../lib/tests/shell_loader_all_filesystems.sh | 22 ++++++
>   testcases/lib/tst_run_shell.c                 | 78 +++++++++++++++++++
>   6 files changed, 114 insertions(+), 3 deletions(-)
>   mode change 100644 => 100755 testcases/lib/tests/shell_loader.sh
>   create mode 100755 testcases/lib/tests/shell_loader_all_filesystems.sh
>   create mode 100644 testcases/lib/tst_run_shell.c
> 
> diff --git a/testcases/lib/.gitignore b/testcases/lib/.gitignore
> index d0dacf62a..385f3c3ca 100644
> --- a/testcases/lib/.gitignore
> +++ b/testcases/lib/.gitignore
> @@ -24,3 +24,4 @@
>   /tst_supported_fs
>   /tst_timeout_kill
>   /tst_res_
> +/tst_run_shell
> diff --git a/testcases/lib/Makefile b/testcases/lib/Makefile
> index 928d76d62..fef284e35 100644
> --- a/testcases/lib/Makefile
> +++ b/testcases/lib/Makefile
> @@ -13,6 +13,7 @@ MAKE_TARGETS		:= tst_sleep tst_random tst_checkpoint tst_rod tst_kvcmp\
>   			   tst_getconf tst_supported_fs tst_check_drivers tst_get_unused_port\
>   			   tst_get_median tst_hexdump tst_get_free_pids tst_timeout_kill\
>   			   tst_check_kconfigs tst_cgctl tst_fsfreeze tst_ns_create tst_ns_exec\
> -			   tst_ns_ifmove tst_lockdown_enabled tst_secureboot_enabled tst_res_
> +			   tst_ns_ifmove tst_lockdown_enabled tst_secureboot_enabled tst_res_\
> +			   tst_run_shell
>   
>   include $(top_srcdir)/include/mk/generic_trunk_target.mk
> diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
> index 88607b146..9637ae327 100755
> --- a/testcases/lib/run_tests.sh
> +++ b/testcases/lib/run_tests.sh
> @@ -8,3 +8,5 @@ export PATH=$PATH:$PWD:$PWD/tests/
>   ./tests/shell_test04
>   ./tests/shell_test05
>   ./tests/shell_test06
> +./tst_run_shell shell_loader.sh
> +./tst_run_shell shell_loader_all_filesystems.sh
> diff --git a/testcases/lib/tests/shell_loader.sh b/testcases/lib/tests/shell_loader.sh
> old mode 100644
> new mode 100755
> index c3b3cf5fd..1a255fdee
> --- a/testcases/lib/tests/shell_loader.sh
> +++ b/testcases/lib/tests/shell_loader.sh
> @@ -1,5 +1,12 @@
> -#!/usr/bin/env tst_run_shell
> +#!/bin/sh
> +#
> +# needs_tmpdir=1
>   
>   . tst_env.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
> diff --git a/testcases/lib/tests/shell_loader_all_filesystems.sh b/testcases/lib/tests/shell_loader_all_filesystems.sh
> new file mode 100755
> index 000000000..86d09d393
> --- /dev/null
> +++ b/testcases/lib/tests/shell_loader_all_filesystems.sh
> @@ -0,0 +1,22 @@
> +#!/bin/sh
> +#
> +# needs_root=1
> +# mount_device=1
> +# all_filesystems=1
> +
> +TST_MNTPOINT=ltp_mntpoint
> +
> +. tst_env.sh
> +
> +tst_res TINFO "IN shell"
> +
> +mounted=$(grep $TST_MNTPOINT /proc/mounts)

This check might produce false positive for example when another LTP 
shell script runs in parallel in another temp directory. I'd recommend 
using $(realpath ...) to disambiguate the mountpoints.

> +
> +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
> diff --git a/testcases/lib/tst_run_shell.c b/testcases/lib/tst_run_shell.c
> new file mode 100644
> index 000000000..583807376
> --- /dev/null
> +++ b/testcases/lib/tst_run_shell.c
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2024 Cyril Hrubis <chrubis@suse.cz>
> + */
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "tst_safe_stdio.h"
> +
> +static char *shell_filename;
> +
> +static void run_shell(void)
> +{
> +	tst_run_shell(shell_filename, NULL);
> +}
> +
> +struct tst_test test = {
> +	.test_all = run_shell,
> +	.runs_shell = 1,
> +};
> +
> +static void print_help(void)
> +{
> +	printf("Usage: tst_shell_loader ltp_shell_test.sh ...");
> +}
> +
> +#define FLAG_MATCH(str, name) (!strcmp(str, name "=1"))
> +#define PARAM_MATCH(str, name) (!strncmp(str, name "=", sizeof(name)))
> +#define PARAM_VAL(str, name) strdup(str + sizeof(name))
> +
> +static void prepare_test_struct(void)
> +{
> +	FILE *f;
> +	char line[4096];
> +	char path[4096];
> +
> +	if (tst_get_path(shell_filename, path, sizeof(path)) == -1)
> +		tst_brk(TBROK, "Failed to find %s in $PATH", shell_filename);
> +
> +	f = SAFE_FOPEN(path, "r");
> +
> +	while (fscanf(f, "%4096s[^\n]", line) != EOF) {
> +		if (FLAG_MATCH(line, "needs_tmpdir"))
> +			test.needs_tmpdir = 1;
> +		else if (FLAG_MATCH(line, "needs_root"))
> +			test.needs_root = 1;
> +		else if (FLAG_MATCH(line, "needs_device"))
> +			test.needs_device = 1;
> +		else if (FLAG_MATCH(line, "needs_checkpoints"))
> +			test.needs_checkpoints = 1;
> +		else if (FLAG_MATCH(line, "needs_overlay"))
> +			test.needs_overlay = 1;
> +		else if (FLAG_MATCH(line, "format_device"))
> +			test.format_device = 1;
> +		else if (FLAG_MATCH(line, "mount_device"))
> +			test.mount_device = 1;
> +		else if (PARAM_MATCH(line, "TST_MNTPOINT"))
> +			test.mntpoint = PARAM_VAL(line, "TST_MNTPOINT");
> +		else if (FLAG_MATCH(line, "all_filesystems"))
> +			test.all_filesystems = 1;
> +	}
> +
> +	fclose(f);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	if (argc < 2)
> +		goto help;
> +
> +	shell_filename = argv[1];
> +
> +	prepare_test_struct();
> +
> +	tst_run_tcases(argc - 1, argv + 1, &test);
> +help:
> +	print_help();
> +	return 1;
> +}

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


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

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

* Re: [LTP] [RFC PATCH 2/2] testcaes/lib: Add shell loader
  2024-07-24 12:54   ` Martin Doucha
@ 2024-07-25 11:02     ` Petr Vorel
  2024-07-25 16:01       ` Martin Doucha
  0 siblings, 1 reply; 25+ messages in thread
From: Petr Vorel @ 2024-07-25 11:02 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi Martin, Cyril,

> Hi,
> one note below.
...
> > --- /dev/null
> > +++ b/testcases/lib/tests/shell_loader_all_filesystems.sh
> > @@ -0,0 +1,22 @@
> > +#!/bin/sh
> > +#
> > +# needs_root=1
> > +# mount_device=1
> > +# all_filesystems=1
> > +
> > +TST_MNTPOINT=ltp_mntpoint
> > +
> > +. tst_env.sh
> > +
> > +tst_res TINFO "IN shell"
> > +
> > +mounted=$(grep $TST_MNTPOINT /proc/mounts)

> This check might produce false positive for example when another LTP shell
> script runs in parallel in another temp directory. I'd recommend using
> $(realpath ...) to disambiguate the mountpoints.

FYI this is just a test, not a library script.

Also maybe just prepend it with $PWD/ would be enough:

mounted=$(grep $PWD/$TST_MNTPOINT /proc/mounts)

(Not to require yet another binary. It's ok to have it here, because developers
have coreutils, but in case it's in the tst_test.sh or its dependencies some
minimal systems might have problem - busybox based embedded systems realpath is
configurable via CONFIG_REALPATH=y. It would help if we specify shell library
dependencies).

Kind regards,
Petr

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

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

* Re: [LTP] [RFC PATCH 2/2] testcaes/lib: Add shell loader
  2024-07-25 11:02     ` Petr Vorel
@ 2024-07-25 16:01       ` Martin Doucha
  2024-07-25 21:44         ` Petr Vorel
  0 siblings, 1 reply; 25+ messages in thread
From: Martin Doucha @ 2024-07-25 16:01 UTC (permalink / raw)
  To: Petr Vorel; +Cc: ltp

On 25. 07. 24 13:02, Petr Vorel wrote:
> Hi Martin, Cyril,
> 
>>> +mounted=$(grep $TST_MNTPOINT /proc/mounts)
> 
>> This check might produce false positive for example when another LTP shell
>> script runs in parallel in another temp directory. I'd recommend using
>> $(realpath ...) to disambiguate the mountpoints.
> 
> FYI this is just a test, not a library script.
> 
> Also maybe just prepend it with $PWD/ would be enough:
> 
> mounted=$(grep $PWD/$TST_MNTPOINT /proc/mounts)
> 
> (Not to require yet another binary. It's ok to have it here, because developers
> have coreutils, but in case it's in the tst_test.sh or its dependencies some
> minimal systems might have problem - busybox based embedded systems realpath is
> configurable via CONFIG_REALPATH=y. It would help if we specify shell library
> dependencies).

The test could still get confused by leftover mounts from other tests or 
failed runs of itself. That'll be annoying to deal with when you try to 
modify something in the shell loader.

I've checked what happens if you mount a filesystem on a mountpoint path 
that contains symlinks and in that case, /proc/mounts will report the 
real path. So $(realpath $TST_MNTPOINT) is what you need to search for. 
Grep might get confused by special characters in the path, though. 
Another problem is that /proc/mounts converts spaces to octal codes to 
prevent column parsing issues.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


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

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

* Re: [LTP] [RFC PATCH 2/2] testcaes/lib: Add shell loader
  2024-07-25 16:01       ` Martin Doucha
@ 2024-07-25 21:44         ` Petr Vorel
  0 siblings, 0 replies; 25+ messages in thread
From: Petr Vorel @ 2024-07-25 21:44 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

> On 25. 07. 24 13:02, Petr Vorel wrote:
> > Hi Martin, Cyril,

> > > > +mounted=$(grep $TST_MNTPOINT /proc/mounts)

> > > This check might produce false positive for example when another LTP shell
> > > script runs in parallel in another temp directory. I'd recommend using
> > > $(realpath ...) to disambiguate the mountpoints.

> > FYI this is just a test, not a library script.

> > Also maybe just prepend it with $PWD/ would be enough:

> > mounted=$(grep $PWD/$TST_MNTPOINT /proc/mounts)

> > (Not to require yet another binary. It's ok to have it here, because developers
> > have coreutils, but in case it's in the tst_test.sh or its dependencies some
> > minimal systems might have problem - busybox based embedded systems realpath is
> > configurable via CONFIG_REALPATH=y. It would help if we specify shell library
> > dependencies).

> The test could still get confused by leftover mounts from other tests or
> failed runs of itself. That'll be annoying to deal with when you try to
> modify something in the shell loader.

Fair point.

> I've checked what happens if you mount a filesystem on a mountpoint path
> that contains symlinks and in that case, /proc/mounts will report the real
> path. So $(realpath $TST_MNTPOINT) is what you need to search for. Grep
> might get confused by special characters in the path, though. Another
> problem is that /proc/mounts converts spaces to octal codes to prevent
> column parsing issues.

OK, I would hope users would not use symlinks, but OTOH I'm ok with using
realpath for tests, thus no reason to block using symlinks (which we would do
with my suggestion to use $PWD instead of realpath).

But I would not waste our time with effort to fix special characters:

1) Build system does not work with some of them anyway (Cyril reported at least
" due out-of-tree complexity of our build system).
2) Old doc somewhere warned that project would ignore broken compilation due
special characters.
3) I don't expect many people will be forced to have LTP on directory with
strange characters.

Kind regards,
Petr

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

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

* Re: [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code
  2024-07-24  9:37   ` Martin Doucha
  2024-07-24  9:47     ` Cyril Hrubis
@ 2024-07-30  9:49     ` Cyril Hrubis
  1 sibling, 0 replies; 25+ messages in thread
From: Cyril Hrubis @ 2024-07-30  9:49 UTC (permalink / raw)
  To: Martin Doucha; +Cc: ltp

Hi!
> $PWD is not the correct path. It should be set as follows:
> 
> testdir=$(dirname $0)
> export PATH=$PATH:$testdir:$testdir/tests/

This does not work for tests that have .needs_tmpdir because $(dirname
$0) is '.' I suppose that we need realpath as well:

testdir=$(realpath $(dirname $0))

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

end of thread, other threads:[~2024-07-30  9:50 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-16 15:36 [LTP] [RFC PATCH 0/2] Shell test library v3 Cyril Hrubis
2024-07-16 15:36 ` [LTP] [RFC PATCH 1/2] Add support for mixing C and shell code Cyril Hrubis
2024-07-17  8:33   ` Li Wang
2024-07-17  8:41     ` Cyril Hrubis
2024-07-17 10:07       ` Li Wang
2024-07-17  8:52     ` Cyril Hrubis
2024-07-17 10:21       ` Li Wang
2024-07-17 17:51         ` Petr Vorel
2024-07-17 17:55         ` Petr Vorel
2024-07-17 18:13           ` Cyril Hrubis
2024-07-18 12:43   ` Petr Vorel
2024-07-18 13:03     ` Cyril Hrubis
2024-07-18 12:57   ` Petr Vorel
2024-07-18 13:07     ` Cyril Hrubis
2024-07-18 13:15       ` Petr Vorel
2024-07-24  9:37   ` Martin Doucha
2024-07-24  9:47     ` Cyril Hrubis
2024-07-24  9:53       ` Martin Doucha
2024-07-24 10:05         ` Cyril Hrubis
2024-07-30  9:49     ` Cyril Hrubis
2024-07-16 15:36 ` [LTP] [RFC PATCH 2/2] testcaes/lib: Add shell loader Cyril Hrubis
2024-07-24 12:54   ` Martin Doucha
2024-07-25 11:02     ` Petr Vorel
2024-07-25 16:01       ` Martin Doucha
2024-07-25 21:44         ` Petr Vorel

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