[Fuego] [PATCH 1/2] main: fix syntax error

[Fuego] [PATCH 1/2] main: fix syntax error

[Liu Wenlong]

If FUEGO_DEBUG not defined, then,
error:
"fuego-core/engine/scripts/main.sh: line 24: & 1 : syntax error: operand
 expected (error token is "& 1 ")"

Signed-off-by: Liu Wenlong <liuwl.fnst@cn.fujitsu.com>
---
 engine/scripts/main.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/engine/scripts/main.sh b/engine/scripts/main.sh
index 62fd0fa..7ce08c3 100644
--- a/engine/scripts/main.sh
+++ b/engine/scripts/main.sh
@@ -21,7 +21,7 @@
 # DESCRIPTION
 # This script contains a sequence of calls that are needed for running a test
 
-if [ -n "$FUEGO_DEBUG" -a $(( $FUEGO_DEBUG & 1 )) = 1 ] ; then
+if [[ -n "$FUEGO_DEBUG" && $(( $FUEGO_DEBUG & 1 )) = 1 ]] ; then
     set -x
 fi
 
-- 
2.7.4

[Fuego] [PATCH 2/2] ftc: add global name 'board_name'

[Liu Wenlong]

Fix the following runtime error,
$ ftc run-test -b intel-up2 -t Functional.bc -s default
  Notice: non-Jenkins test request detected
  Traceback (most recent call last):
    File "/usr/local/bin/ftc", line 4393, in <module>
      main()
    File "/usr/local/bin/ftc", line 4353, in main
      do_run_test(conf, options)
    File "/usr/local/bin/ftc", line 3152, in do_run_test
      print "Running test '%s' on board '%s' using spec '%s'" % (test_name, board_name, spec_name)
  NameError: global name 'board_name' is not defined

Signed-off-by: Liu Wenlong <liuwl.fnst@cn.fujitsu.com>
---
 engine/scripts/ftc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/engine/scripts/ftc b/engine/scripts/ftc
index d66eddd..191f163 100755
--- a/engine/scripts/ftc
+++ b/engine/scripts/ftc
@@ -3076,7 +3076,7 @@ def get_board_arg(cmd, conf, options):
     options.remove(board_name)
     options.remove('-b')
     board = boards[board_name]
-    return (board, options)
+    return (board, board_name, options)
 
 # returns (test, options) with the test argument removed
 def get_test_arg(cmd, conf, options):
@@ -3097,7 +3097,7 @@ def get_test_arg(cmd, conf, options):
 def do_run_test(conf, options):
     global log, tail_fd
 
-    board, options = get_board_arg("Run-test", conf, options)
+    board, board_name, options = get_board_arg("Run-test", conf, options)
 
     test_name = None
     if '-t' in options:
-- 
2.7.4

Re: [Fuego] [PATCH 2/2] ftc: add global name 'board_name'

[Daniel Sangorrin]

Ouch I was touching this part of the code and didn't notice Tim's recent commits.
Tim: no plans on using argparse?

> -----Original Message-----
> From: fuego-bounces@lists.linuxfoundation.org
> [mailto:fuego-bounces@lists.linuxfoundation.org] On Behalf Of Liu Wenlong
> Sent: Thursday, May 24, 2018 3:27 PM
> To: fuego@lists.linuxfoundation.org
> Subject: [Fuego] [PATCH 2/2] ftc: add global name 'board_name'
> 
> Fix the following runtime error,
> $ ftc run-test -b intel-up2 -t Functional.bc -s default
>   Notice: non-Jenkins test request detected
>   Traceback (most recent call last):
>     File "/usr/local/bin/ftc", line 4393, in <module>
>       main()
>     File "/usr/local/bin/ftc", line 4353, in main
>       do_run_test(conf, options)
>     File "/usr/local/bin/ftc", line 3152, in do_run_test
>       print "Running test '%s' on board '%s' using spec '%s'" % (test_name,
> board_name, spec_name)
>   NameError: global name 'board_name' is not defined
> 
> Signed-off-by: Liu Wenlong <liuwl.fnst@cn.fujitsu.com>
> ---
>  engine/scripts/ftc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/engine/scripts/ftc b/engine/scripts/ftc
> index d66eddd..191f163 100755
> --- a/engine/scripts/ftc
> +++ b/engine/scripts/ftc
> @@ -3076,7 +3076,7 @@ def get_board_arg(cmd, conf, options):
>      options.remove(board_name)
>      options.remove('-b')
>      board = boards[board_name]
> -    return (board, options)
> +    return (board, board_name, options)
> 
>  # returns (test, options) with the test argument removed
>  def get_test_arg(cmd, conf, options):
> @@ -3097,7 +3097,7 @@ def get_test_arg(cmd, conf, options):
>  def do_run_test(conf, options):
>      global log, tail_fd
> 
> -    board, options = get_board_arg("Run-test", conf, options)
> +    board, board_name, options = get_board_arg("Run-test", conf, options)
> 
>      test_name = None
>      if '-t' in options:
> --
> 2.7.4
> 
> 
> 
> _______________________________________________
> Fuego mailing list
> Fuego@lists.linuxfoundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/fuego

Re: [Fuego] [PATCH 1/2] main: fix syntax error

[Tim.Bird]

> -----Original Message-----
> From: Liu Wenlong
> 
> If FUEGO_DEBUG not defined, then,
> error:
> "fuego-core/engine/scripts/main.sh: line 24: & 1 : syntax error: operand
>  expected (error token is "& 1 ")"
> 
> Signed-off-by: Liu Wenlong <liuwl.fnst@cn.fujitsu.com>
> ---
>  engine/scripts/main.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/engine/scripts/main.sh b/engine/scripts/main.sh
> index 62fd0fa..7ce08c3 100644
> --- a/engine/scripts/main.sh
> +++ b/engine/scripts/main.sh
> @@ -21,7 +21,7 @@
>  # DESCRIPTION
>  # This script contains a sequence of calls that are needed for running a test
> 
> -if [ -n "$FUEGO_DEBUG" -a $(( $FUEGO_DEBUG & 1 )) = 1 ] ; then
> +if [[ -n "$FUEGO_DEBUG" && $(( $FUEGO_DEBUG & 1 )) = 1 ]] ; then
>      set -x
>  fi
> 
> --
> 2.7.4

Thanks very much.  I can't believe I missed the error message.
Applied.
 -- Tim

Re: [Fuego] [PATCH 2/2] ftc: add global name 'board_name'

[Tim.Bird]

Thanks for the bug report.  I think there's an easier fix for this.
(See below).

> -----Original Message-----
> From: Liu Wenlong
> 
> Fix the following runtime error,
> $ ftc run-test -b intel-up2 -t Functional.bc -s default
>   Notice: non-Jenkins test request detected
>   Traceback (most recent call last):
>     File "/usr/local/bin/ftc", line 4393, in <module>
>       main()
>     File "/usr/local/bin/ftc", line 4353, in main
>       do_run_test(conf, options)
>     File "/usr/local/bin/ftc", line 3152, in do_run_test
>       print "Running test '%s' on board '%s' using spec '%s'" % (test_name,
> board_name, spec_name)
>   NameError: global name 'board_name' is not defined
> 
> Signed-off-by: Liu Wenlong <liuwl.fnst@cn.fujitsu.com>
> ---
>  engine/scripts/ftc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/engine/scripts/ftc b/engine/scripts/ftc
> index d66eddd..191f163 100755
> --- a/engine/scripts/ftc
> +++ b/engine/scripts/ftc
> @@ -3076,7 +3076,7 @@ def get_board_arg(cmd, conf, options):
>      options.remove(board_name)
>      options.remove('-b')
>      board = boards[board_name]
> -    return (board, options)
> +    return (board, board_name, options)
> 
>  # returns (test, options) with the test argument removed
>  def get_test_arg(cmd, conf, options):
> @@ -3097,7 +3097,7 @@ def get_test_arg(cmd, conf, options):
>  def do_run_test(conf, options):
>      global log, tail_fd
> 
> -    board, options = get_board_arg("Run-test", conf, options)
> +    board, board_name, options = get_board_arg("Run-test", conf, options)

This is more easily resolved with:
board_name = board.name

> 
>      test_name = None
>      if '-t' in options:
> --
> 2.7.4

I made the easier fix.  Please test again.

Thanks,
 -- Tim

Re: [Fuego] [PATCH 2/2] ftc: add global name 'board_name'

[Tim.Bird]

> -----Original Message-----
> From: Daniel Sangorrin
> Sent: Thursday, May 24, 2018 12:04 AM
> To: 'Liu Wenlong' <liuwl.fnst@cn.fujitsu.com>;
> fuego@lists.linuxfoundation.org
> Subject: Re: [Fuego] [PATCH 2/2] ftc: add global name 'board_name'
> 
> Ouch I was touching this part of the code and didn't notice Tim's recent
> commits.
> Tim: no plans on using argparse?

I'm not opposed to refactoring and simplifying the argument 
parsing across the board for ftc sub-commands.  I did this
refactoring as a quick one-off as I was working on a
(not published yet) new 'ftc set-criteria' command.

Nothing done here is set in stone, and if you or someone else
wants to refactor stuff using argparse, I'll be receptive to it.
What we have now for argument parsing is a real mishmash, which
I'm not too fond of. But it's rather easy to follow (well - modulo
dumb bugs like the one I introduced and Liu found).

One thing that would be good IMHO, would be to always use -b
to specify a board, always -t for a test, etc.  Some commands use
positional arguments for items when they are mandatory, which is a
bit less typing for those commands, but less uniform, and probably 
presents a steeper learning curve.
What do you think?
  -- Tim

Re: [Fuego] [PATCH 2/2] ftc: add global name 'board_name'

[Daniel Sangorrin]

> -----Original Message-----
> From: Tim.Bird@sony.com [mailto:Tim.Bird@sony.com]
> Sent: Friday, May 25, 2018 1:07 PM
> To: daniel.sangorrin@toshiba.co.jp; liuwl.fnst@cn.fujitsu.com;
> fuego@lists.linuxfoundation.org
> Subject: RE: [Fuego] [PATCH 2/2] ftc: add global name 'board_name'
> 
> 
> 
> > -----Original Message-----
> > From: Daniel Sangorrin
> > Sent: Thursday, May 24, 2018 12:04 AM
> > To: 'Liu Wenlong' <liuwl.fnst@cn.fujitsu.com>;
> > fuego@lists.linuxfoundation.org
> > Subject: Re: [Fuego] [PATCH 2/2] ftc: add global name 'board_name'
> >
> > Ouch I was touching this part of the code and didn't notice Tim's recent
> > commits.
> > Tim: no plans on using argparse?
> 
> I'm not opposed to refactoring and simplifying the argument
> parsing across the board for ftc sub-commands.  I did this
> refactoring as a quick one-off as I was working on a
> (not published yet) new 'ftc set-criteria' command.
> 
> Nothing done here is set in stone, and if you or someone else
> wants to refactor stuff using argparse, I'll be receptive to it.
> What we have now for argument parsing is a real mishmash, which
> I'm not too fond of. But it's rather easy to follow (well - modulo
> dumb bugs like the one I introduced and Liu found).
> 
> One thing that would be good IMHO, would be to always use -b
> to specify a board, always -t for a test, etc.  Some commands use
> positional arguments for items when they are mandatory, which is a
> bit less typing for those commands, but less uniform, and probably
> presents a steeper learning curve.
> What do you think?
>   -- Tim

Yes, I noticed those. I agree that they should be the same for all.
For example, -p was used for test_plans but now it is used also for phases.
I think that long names might be better in some cases. Also ftc add-nodes/rm-nodes does not require the -b option.

I will add argparse refactoring to my growing TODO list ;).

Thanks,
Daniel

Re: [Fuego] [PATCH 2/2] ftc: add global name 'board_name'

[Tim.Bird]

> -----Original Message-----
> From: Daniel Sangorrin 
> 
> > -----Original Message-----
> > From: Tim.Bird@sony.com 
> > > -----Original Message-----
> > > From: Daniel Sangorrin
> > > Sent: Thursday, May 24, 2018 12:04 AM
> > > To: 'Liu Wenlong' <liuwl.fnst@cn.fujitsu.com>;
> > > fuego@lists.linuxfoundation.org
> > > Subject: Re: [Fuego] [PATCH 2/2] ftc: add global name 'board_name'
> > >
> > > Ouch I was touching this part of the code and didn't notice Tim's recent
> > > commits.
> > > Tim: no plans on using argparse?
> >
> > I'm not opposed to refactoring and simplifying the argument
> > parsing across the board for ftc sub-commands.  I did this
> > refactoring as a quick one-off as I was working on a
> > (not published yet) new 'ftc set-criteria' command.
> >
> > Nothing done here is set in stone, and if you or someone else
> > wants to refactor stuff using argparse, I'll be receptive to it.
> > What we have now for argument parsing is a real mishmash, which
> > I'm not too fond of. But it's rather easy to follow (well - modulo
> > dumb bugs like the one I introduced and Liu found).
> >
> > One thing that would be good IMHO, would be to always use -b
> > to specify a board, always -t for a test, etc.  Some commands use
> > positional arguments for items when they are mandatory, which is a
> > bit less typing for those commands, but less uniform, and probably
> > presents a steeper learning curve.
> > What do you think?
> >   -- Tim
> 
> Yes, I noticed those. I agree that they should be the same for all.
> For example, -p was used for test_plans but now it is used also for phases.

I regret that choice.  This could be changed to something like '-z' or '--phases'.

> I think that long names might be better in some cases. Also ftc add-
> nodes/rm-nodes does not require the -b option.
It might be nice for consistency sake if they did.  We could add the
wildcard or list-handling to a single place, and all commands using
e.g. boards, tests, etc. could use it.

> 
> I will add argparse refactoring to my growing TODO list ;).
Thanks.

This is something I wanted to get back to, but I've always had 
too many other sticks in the fire.  We can discuss in Japan
if you're likely to get to it soon, and I can take a stab at it
(if I have some free time).
 -- Tim