[LTP] [PATCH 0/2] shell API: print environment variables in .h

[LTP] [PATCH 0/2] shell API: print environment variables in .h

[Petr Vorel]

Petr Vorel (2):
  tst_test.sh: Print environment variables in help
  doc: Mention places for environment variables

 doc/library-api-writing-guidelines.txt |  4 ++++
 testcases/lib/tst_test.sh              | 22 ++++++++++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

-- 
2.34.1


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

[LTP] [PATCH 1/2] tst_test.sh: Print environment variables in help

[Petr Vorel]

to sync with C API.

Unlike C API environment variables are printed at the top,
because we expect custom $TST_USAGE function prints part of the usage
itself (but not all tests do).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_test.sh | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index 30614974c3..a7fd7b19c6 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -448,12 +448,30 @@ tst_usage()
 	if [ -n "$TST_USAGE" ]; then
 		$TST_USAGE
 	else
-		echo "usage: $0"
-		echo "OPTIONS"
+		cat << EOF
+usage: $0
+
+Options
+-------
+EOF
 	fi
 
 	echo "-h      Prints this help"
 	echo "-i n    Execute test n times"
+
+		cat << EOF
+
+Environment Variables
+---------------------
+KCONFIG_PATH         Specify kernel config file
+KCONFIG_SKIP_CHECK   Skip kernel config check if variable set (not set by default)
+LTPROOT              Prefix for installed LTP (default: /opt/ltp)
+LTP_COLORIZE_OUTPUT  Force colorized output behaviour (y/1 always, n/0: never)
+LTP_DEV              Path to the block device to be used (for .needs_device)
+LTP_DEV_FS_TYPE      Filesystem used for testing (default: ext2)
+LTP_TIMEOUT_MUL      Timeout multiplier (must be a number >=1, ceiled to int)
+TMPDIR               Base directory for template directory (for .needs_tmpdir, default: /tmp)
+EOF
 }
 
 _tst_resstr()
-- 
2.34.1


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

Re: [LTP] [PATCH 1/2] tst_test.sh: Print environment variables in help

[Li Wang]

For series:
Reviewed-by: Li Wang <liwang@redhat.com>

-- 
Regards,
Li Wang


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

Re: [LTP] [PATCH 1/2] tst_test.sh: Print environment variables in help

[Cyril Hrubis]

Hi!
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index 30614974c3..a7fd7b19c6 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -448,12 +448,30 @@ tst_usage()
>  	if [ -n "$TST_USAGE" ]; then
>  		$TST_USAGE
>  	else
> -		echo "usage: $0"
> -		echo "OPTIONS"
> +		cat << EOF
> +usage: $0
> +
> +Options
> +-------
> +EOF

I think that the cat EOF syntax inside of else branch is a bit
confusing, especially sice it prints just three lines of text...


But other than that:

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

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 1/2] tst_test.sh: Print environment variables in help

[Petr Vorel]

> Hi!
> > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> > index 30614974c3..a7fd7b19c6 100644
> > --- a/testcases/lib/tst_test.sh
> > +++ b/testcases/lib/tst_test.sh
> > @@ -448,12 +448,30 @@ tst_usage()
> >  	if [ -n "$TST_USAGE" ]; then
> >  		$TST_USAGE
> >  	else
> > -		echo "usage: $0"
> > -		echo "OPTIONS"
> > +		cat << EOF
> > +usage: $0
> > +
> > +Options
> > +-------
> > +EOF

> I think that the cat EOF syntax inside of else branch is a bit
> confusing, especially sice it prints just three lines of text...

Good point, I'll replace it with echo.


> But other than that:

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

How about dropping this?
echo "usage: $0"

(as a separate commit, I'd sent v2)

Because we don't support it in C API, is it really required to have it in shell.

I mean it's useful to show which options are mandatory or which cannot be
combined together etc. But in reality most of shell tests does not add it and
thus they are missing "OPTIONS" line (sure, I can add usage to them if you
think it's useful).

OTOH some tests have really complex setup, i.e.
testcases/network/netstress/netstress.c, that adding a support for usage string
would help them.

If you notice there were extra new lines (\n) to separate client and server side
to make help at least a bit readable, but Andrea removed them in 98af9ecf9e
("tst_test: Complete help message adding option before desc"):

git show --word-diff 98af9ecf9e334c07251f2f464191635f161a1603 testcases/network/netstress/netstress.c

These extra lines would not be needed when sort of usage added to C API.
I can add it, but I'm aware it's so minor, that I'm wasting a time of all of us.

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/2] tst_test.sh: Print environment variables in help

[Petr Vorel]

Hi Cyril, Li,

> > Hi!
> > > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> > > index 30614974c3..a7fd7b19c6 100644
> > > --- a/testcases/lib/tst_test.sh
> > > +++ b/testcases/lib/tst_test.sh
> > > @@ -448,12 +448,30 @@ tst_usage()
> > >  	if [ -n "$TST_USAGE" ]; then
> > >  		$TST_USAGE
> > >  	else
> > > -		echo "usage: $0"
> > > -		echo "OPTIONS"
> > > +		cat << EOF
> > > +usage: $0
> > > +
> > > +Options
> > > +-------
> > > +EOF

> > I think that the cat EOF syntax inside of else branch is a bit
> > confusing, especially sice it prints just three lines of text...

> Good point, I'll replace it with echo.

FYI merged with replaced echo.


> > But other than that:

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

> How about dropping this?
> echo "usage: $0"

> (as a separate commit, I'd sent v2)

> Because we don't support it in C API, is it really required to have it in shell.

> I mean it's useful to show which options are mandatory or which cannot be
> combined together etc. But in reality most of shell tests does not add it and
> thus they are missing "OPTIONS" line (sure, I can add usage to them if you
> think it's useful).

> OTOH some tests have really complex setup, i.e.
> testcases/network/netstress/netstress.c, that adding a support for usage string
> would help them.

> If you notice there were extra new lines (\n) to separate client and server side
> to make help at least a bit readable, but Andrea removed them in 98af9ecf9e
> ("tst_test: Complete help message adding option before desc"):

> git show --word-diff 98af9ecf9e334c07251f2f464191635f161a1603 testcases/network/netstress/netstress.c

> These extra lines would not be needed when sort of usage added to C API.
> I can add it, but I'm aware it's so minor, that I'm wasting a time of all of us.

FYI I'll probably add options string for shell and might add support for C API
to have help (would be useful for netstress.c).

Kind regards,
Petr

> Kind regards,
> Petr

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

[LTP] [PATCH 2/2] doc: Mention places for environment variables

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/library-api-writing-guidelines.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/library-api-writing-guidelines.txt b/doc/library-api-writing-guidelines.txt
index c820536814..52e37d8c28 100644
--- a/doc/library-api-writing-guidelines.txt
+++ b/doc/library-api-writing-guidelines.txt
@@ -18,6 +18,10 @@ Library tests are in 'lib/newlib_tests' directory.
 
 Don't forget to update docs when you change the API.
 
+Environment variables should be listed in
+https://github.com/linux-test-project/ltp/wiki/User-Guidelines[LTP User Guidelines]
+and in help output (`-h`) for both C and shell API.
+
 2. C API
 --------
 
-- 
2.34.1


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

Re: [LTP] [PATCH 2/2] doc: Mention places for environment variables

[Cyril Hrubis]

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

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 0/2] shell API: print environment variables in .h

[Petr Vorel]

Hi,

Subject was meant to be: "shell API: print environment variables in -h"

Kind regards,
Petr

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

Re: [LTP] [PATCH 0/2] shell API: print environment variables in .h

[xuyang2018.jy]

Hi Petr

This patch series looks good to me,
Reviewed-by: Yang Xu <xuyang2018.jy@fujitsu.com>

Best Regards
Yang Xu
> Petr Vorel (2):
>    tst_test.sh: Print environment variables in help
>    doc: Mention places for environment variables
>
>   doc/library-api-writing-guidelines.txt |  4 ++++
>   testcases/lib/tst_test.sh              | 22 ++++++++++++++++++++--
>   2 files changed, 24 insertions(+), 2 deletions(-)
>

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