[LTP] [PATCH 0/2] shell lib: Add very simple cleanup support

[LTP] [PATCH 0/2] shell lib: Add very simple cleanup support

[Cyril Hrubis]

This implements a simple shell library cleanup support by utilizing the
shell trap buildin. With that the cleanup is executed when the shell
script is about to exit.

Alternative we do not have to add the TST_CLEANUP variable at all and
use the trap directly in the shell tests instead. In that case we can
drop the change from tst_env.sh and change the two tests to use trap
directly instead.

Cyril Hrubis (2):
  testcases/lib/run_tests.sh
  shell lib: Add support for test cleanup

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

-- 
2.45.3


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

[LTP] [PATCH 1/2] testcases/lib/run_tests.sh

[Cyril Hrubis]

Enable TESTS_TBROK

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/lib/run_tests.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
index 0abca2f3c..168dbb451 100755
--- a/testcases/lib/run_tests.sh
+++ b/testcases/lib/run_tests.sh
@@ -90,6 +90,7 @@ run_tests()
 
 run_tests 0 $TESTS_PASS
 run_tests 32 $TESTS_TCONF
+run_tests 2 $TESTS_TBROK
 
 echo
 echo "*** Testing LTP test -h option ***"
-- 
2.45.3


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

[LTP] [PATCH 2/2] shell lib: Add support for test cleanup

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/lib/run_tests.sh                    |  4 +++-
 .../lib/tests/shell_loader_brk_cleanup.sh     | 20 ++++++++++++++++++
 testcases/lib/tests/shell_loader_cleanup.sh   | 21 +++++++++++++++++++
 testcases/lib/tst_env.sh                      |  4 ++++
 4 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100755 testcases/lib/tests/shell_loader_brk_cleanup.sh
 create mode 100755 testcases/lib/tests/shell_loader_cleanup.sh

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


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

Re: [LTP] [PATCH 1/2] testcases/lib/run_tests.sh

[Petr Vorel]

> Enable TESTS_TBROK

> diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
> index 0abca2f3c..168dbb451 100755
> --- a/testcases/lib/run_tests.sh
> +++ b/testcases/lib/run_tests.sh
> @@ -90,6 +90,7 @@ run_tests()

>  run_tests 0 $TESTS_PASS
>  run_tests 32 $TESTS_TCONF
> +run_tests 2 $TESTS_TBROK

Maybe also add $TESTS_FAIL (untested)?
run_tests 1 $TESTS_FAIL

Fixes: 29d8613dbd ("testcases/lib/run_tests.sh: Check expected results")
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Thanks!

Kind regards,
Petr

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

Re: [LTP] [PATCH 2/2] shell lib: Add support for test cleanup

[Petr Vorel]

Hi Cyril,


> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  testcases/lib/run_tests.sh                    |  4 +++-
>  .../lib/tests/shell_loader_brk_cleanup.sh     | 20 ++++++++++++++++++
>  testcases/lib/tests/shell_loader_cleanup.sh   | 21 +++++++++++++++++++
>  testcases/lib/tst_env.sh                      |  4 ++++
>  4 files changed, 48 insertions(+), 1 deletion(-)
>  create mode 100755 testcases/lib/tests/shell_loader_brk_cleanup.sh
>  create mode 100755 testcases/lib/tests/shell_loader_cleanup.sh

> diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
> index 168dbb451..d568d4614 100755
> --- a/testcases/lib/run_tests.sh
> +++ b/testcases/lib/run_tests.sh
> @@ -9,6 +9,7 @@ shell_loader_filesystems.sh
>  shell_loader_kconfigs.sh
>  shell_loader_supported_archs.sh
>  shell_loader_tcnt.sh
> +shell_loader_cleanup.sh
>  shell_test01
>  shell_test02
>  shell_test03
> @@ -21,7 +22,8 @@ TESTS_TBROK="
>  shell_loader_invalid_block.sh
>  shell_loader_invalid_metadata.sh
>  shell_loader_no_metadata.sh
> -shell_loader_wrong_metadata.sh"
> +shell_loader_wrong_metadata.sh
> +shell_loader_brk_cleanup.sh"

>  TESTS_TCONF="shell_test06"

> diff --git a/testcases/lib/tests/shell_loader_brk_cleanup.sh b/testcases/lib/tests/shell_loader_brk_cleanup.sh
> new file mode 100755
> index 000000000..8c704a540
> --- /dev/null
> +++ b/testcases/lib/tests/shell_loader_brk_cleanup.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2024-2025 Cyril Hrubis <chrubis@suse.cz>
> +#
> +# ---
> +# env
> +# {
> +# }
> +# ---
> +
> +TST_CLEANUP=cleanup
> +
> +. tst_loader.sh
> +
> +cleanup()
> +{
> +	tst_res TINFO "Cleanup runs"
> +}
> +
> +tst_brk TBROK "Test exits"
> diff --git a/testcases/lib/tests/shell_loader_cleanup.sh b/testcases/lib/tests/shell_loader_cleanup.sh
> new file mode 100755
> index 000000000..b183f1d28
> --- /dev/null
> +++ b/testcases/lib/tests/shell_loader_cleanup.sh
> @@ -0,0 +1,21 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2024-2025 Cyril Hrubis <chrubis@suse.cz>
> +#
> +# ---
> +# env
> +# {
> +# }
> +# ---
> +
> +TST_CLEANUP=do_cleanup
> +
> +. tst_loader.sh
> +
> +do_cleanup()
> +{
> +	tst_res TINFO "Cleanup executed"
> +}
> +
> +tst_res TPASS "Test is executed"
> +
> diff --git a/testcases/lib/tst_env.sh b/testcases/lib/tst_env.sh
> index 68f9a0daa..b13bab37c 100644
> --- a/testcases/lib/tst_env.sh
> +++ b/testcases/lib/tst_env.sh
> @@ -35,3 +35,7 @@ tst_brk_()

>  alias tst_res="tst_res_ $tst_script_name \$LINENO"
>  alias tst_brk="tst_brk_ $tst_script_name \$LINENO"
> +
> +if [ -n "$TST_CLEANUP" ]; then
> +	trap $TST_CLEANUP EXIT
> +fi

thanks!

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Unfortunately TST_SETUP will not work that way - function is not found or if you
load tst_env.sh at the end, it complains about missing tst_res or tst_brk:

./tests/./shell_loader_brk_cleanup.sh: line 16: tst_res: command not found

Obviously it does not help to add it to tst_loader.sh.

I'm surprised it checks that, is there shopt option which could disable it?

Kind regards,
Petr


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

Re: [LTP] [PATCH 2/2] shell lib: Add support for test cleanup

[Cyril Hrubis]

Hi!
> Unfortunately TST_SETUP will not work that way - function is not found or if you
> load tst_env.sh at the end, it complains about missing tst_res or tst_brk:
> 
> ./tests/./shell_loader_brk_cleanup.sh: line 16: tst_res: command not found
> 
> Obviously it does not help to add it to tst_loader.sh.
> 
> I'm surprised it checks that, is there shopt option which could disable it?

The problem here the order the scripts are sourced. It looks like this:

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

If you wanted to have separate setup function you would have to build
things differently, you would have to execute a library shell script
by the tst_run_shell tool, which would source the test.sh and execute
the functions passed to it. So the execution would look like:

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

And then the tst_exec.sh would do:

	. "$1"

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

	$TST_TEST

or something along these lines.

And the test.sh would have to look like:

TST_SETUP=setup
TST_CLEANUP=cleanup
TST_TEST=runtest

. tst_loader.sh


setup()
{
...
}

cleanup()
{
...
}

runtest()
{
...
}


All the code would have to be in functions.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 2/2] shell lib: Add support for test cleanup

[Cyril Hrubis]

Hi!
> And then the tst_exec.sh would do:
> 
> 	. "$1"
        . tst_env.sh

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

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 2/2] shell lib: Add support for test cleanup

[Petr Vorel]

Hi Cyril,

could we get check for file existence, the same way it's done in metaparse.c?

Also, I'd prefer if both tools exit 1 in case of missing file (so that CI
notifies the problem).

Kind regards,
Petr

+++ metadata/metaparse-sh.c
@@ -6,6 +6,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <libgen.h>
+#include <unistd.h>
 
 #include "data_storage.h"
 
@@ -46,6 +47,11 @@ static void parse_shell(char *path)
 	FILE *f = fopen(path, "r");
 	enum state state = NONE;
 
+	if (access(path, F_OK)) {
+		fprintf(stderr, "file %s does not exist\n", path);
+		return;
+	}
+
 	while (fgets(line, sizeof(line), f)) {
 		/* Strip newline */
 		line[strlen(line)-1] = 0;

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

Re: [LTP] [PATCH 2/2] shell lib: Add support for test cleanup

[Cyril Hrubis]

Hi!
> could we get check for file existence, the same way it's done in metaparse.c?
> 
> Also, I'd prefer if both tools exit 1 in case of missing file (so that CI
> notifies the problem).

Will do.

> +++ metadata/metaparse-sh.c
> @@ -6,6 +6,7 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <libgen.h>
> +#include <unistd.h>
>  
>  #include "data_storage.h"
>  
> @@ -46,6 +47,11 @@ static void parse_shell(char *path)
>  	FILE *f = fopen(path, "r");
>  	enum state state = NONE;
>  
> +	if (access(path, F_OK)) {
> +		fprintf(stderr, "file %s does not exist\n", path);
> +		return;
> +	}

I guess that I should check the f instead:

	if (!f) {
		fprintf(stderr, "Failed to open '%s': %s", path, strerror(errno));
		exit(1);
	}

>  	while (fgets(line, sizeof(line), f)) {
>  		/* Strip newline */
>  		line[strlen(line)-1] = 0;

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 2/2] shell lib: Add support for test cleanup

[Petr Vorel]

Hi Cyril,

> Hi!
> > could we get check for file existence, the same way it's done in metaparse.c?

> > Also, I'd prefer if both tools exit 1 in case of missing file (so that CI
> > notifies the problem).

> Will do.

> > +++ metadata/metaparse-sh.c
> > @@ -6,6 +6,7 @@
> >  #include <stdio.h>
> >  #include <string.h>
> >  #include <libgen.h>
> > +#include <unistd.h>

> >  #include "data_storage.h"

> > @@ -46,6 +47,11 @@ static void parse_shell(char *path)
> >  	FILE *f = fopen(path, "r");
> >  	enum state state = NONE;

> > +	if (access(path, F_OK)) {
> > +		fprintf(stderr, "file %s does not exist\n", path);
> > +		return;
> > +	}

> I guess that I should check the f instead:

> 	if (!f) {
> 		fprintf(stderr, "Failed to open '%s': %s", path, strerror(errno));
> 		exit(1);
> 	}

Yes, that's it. Thanks for both.

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/2] testcases/lib/run_tests.sh

[Petr Vorel]

Hi Cyril,

> > Enable TESTS_TBROK

> > diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
> > index 0abca2f3c..168dbb451 100755
> > --- a/testcases/lib/run_tests.sh
> > +++ b/testcases/lib/run_tests.sh
> > @@ -90,6 +90,7 @@ run_tests()

> >  run_tests 0 $TESTS_PASS
> >  run_tests 32 $TESTS_TCONF
> > +run_tests 2 $TESTS_TBROK

> Maybe also add $TESTS_FAIL (untested)?
> run_tests 1 $TESTS_FAIL

> Fixes: 29d8613dbd ("testcases/lib/run_tests.sh: Check expected results")
> Reviewed-by: Petr Vorel <pvorel@suse.cz>

I dared to merge this obvious fix with adding also $TESTS_FAIL:
https://github.com/linux-test-project/ltp/commit/a92aedfabd5826d07809559508c8486c12ff7b96

+ sort by exit code, see following diff.

Thanks!

Kind regards,
Petr

> Thanks!

> Kind regards,
> Petr

+++ testcases/lib/run_tests.sh
@@ -89,8 +89,9 @@ run_tests()
 }
 
 run_tests 0 $TESTS_PASS
-run_tests 32 $TESTS_TCONF
+run_tests 1 $TESTS_FAIL
 run_tests 2 $TESTS_TBROK
+run_tests 32 $TESTS_TCONF
 
 echo
 echo "*** Testing LTP test -h option ***"

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