diff mbox series

[RFC,v6,3/5] tst_test.sh: Filter out commented out lines from warnings

Message ID 20180601072838.13196-4-pvorel@suse.cz
State Rejected
Delegated to: Petr Vorel
Headers show
Series Add TST_TEST_DATA and TST_TEST_DATA_IFS + cleanup | expand

Commit Message

Petr Vorel June 1, 2018, 7:28 a.m. UTC
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/lib/tst_test.sh | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Cyril Hrubis June 4, 2018, 9:06 p.m. UTC | #1
Hi!
What is the usecase for this?

I know that comments does not count as a code but why would the test
need to comment with a reserved name in it?

> ---
>  testcases/lib/tst_test.sh | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index de5d65039..098ff7bd2 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -244,13 +244,21 @@ _tst_rescmp()
>  	fi
>  }
>  
> +_tst_get_used_var()
> +{
> +	local _tst_pattern="$1"
> +
> +	grep $_tst_pattern "$TST_TEST_PATH" | grep -v '^[[:space:]]*#' | \
> +		sed "s/.*${_tst_pattern}//;"' s/[="} \t\/:`].*//'

As far as I can tell this does not work for:

foo # run function foo

> +}
Petr Vorel June 4, 2018, 9:34 p.m. UTC | #2
Hi Cyril,


> Hi!
> What is the usecase for this?

> I know that comments does not count as a code but why would the test
> need to comment with a reserved name in it?
OK, let's drop it.

> > ---
> >  testcases/lib/tst_test.sh | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)

> > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> > index de5d65039..098ff7bd2 100644
> > --- a/testcases/lib/tst_test.sh
> > +++ b/testcases/lib/tst_test.sh
> > @@ -244,13 +244,21 @@ _tst_rescmp()
> >  	fi
> >  }

> > +_tst_get_used_var()
> > +{
> > +	local _tst_pattern="$1"
> > +
> > +	grep $_tst_pattern "$TST_TEST_PATH" | grep -v '^[[:space:]]*#' | \
> > +		sed "s/.*${_tst_pattern}//;"' s/[="} \t\/:`].*//'

> As far as I can tell this does not work for:

> foo # run function foo
Not sure what you expected. The behavior is:

* no warning for:
  # _tst_foo run function _tst_foo

* warning for:
  _tst_foo # run function _tst_foo

I take it as your NACK anyway :).

Kind regards,
Petr
Cyril Hrubis June 5, 2018, 2:38 p.m. UTC | #3
Hi!
> > foo # run function foo
> Not sure what you expected. The behavior is:
> 
> * no warning for:
>   # _tst_foo run function _tst_foo
> 
> * warning for:
>   _tst_foo # run function _tst_foo

We will also get warning for:

tst_foo # we are not allowed to call _tst_foo

I was trying to point out that the solution is incomplete...

But anyway I see no point in proceeding further with the implemtation.
diff mbox series

Patch

diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index de5d65039..098ff7bd2 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -244,13 +244,21 @@  _tst_rescmp()
 	fi
 }
 
+_tst_get_used_var()
+{
+	local _tst_pattern="$1"
+
+	grep $_tst_pattern "$TST_TEST_PATH" | grep -v '^[[:space:]]*#' | \
+		sed "s/.*${_tst_pattern}//;"' s/[="} \t\/:`].*//'
+}
+
 tst_run()
 {
 	local _tst_i
 	local _tst_name
 
 	if [ -n "$TST_TEST_PATH" ]; then
-		for _tst_i in $(grep TST_ "$TST_TEST_PATH" | sed 's/.*TST_//; s/[="} \t\/:`].*//'); do
+		for _tst_i in $(_tst_get_used_var "TST_"); do
 			case "$_tst_i" in
 			SETUP|CLEANUP|TESTFUNC|ID|CNT|MIN_KVER);;
 			OPTS|USAGE|PARSE_ARGS|POS_ARGS);;
@@ -261,7 +269,7 @@  tst_run()
 			esac
 		done
 
-		for _tst_i in $(grep _tst_ "$TST_TEST_PATH" | sed 's/.*_tst_//; s/[="} \t\/:`].*//'); do
+		for _tst_i in $(_tst_get_used_var "_tst_"); do
 			tst_res TWARN "Private variable or function _tst_$_tst_i used!"
 		done
 	fi