Re-do argument passing to match zbehan's change

Re-do argument passing to match zbehan's change

zbehan's change: "run_remote_tests: destroy the with_spaces hack,
use the --args instead of -a (which doesn't work)", despite apparently
not working is in active use by others, both by folks using it on the
command line and by the script cros_run_wifi_tests.sh.  Fix the script
to match the new argument.

BUG=none
TEST=rerun

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=30d46e3

[zbehan, 2010-09-17 20:48:50]

Just a FYI: for run_remote_tests.sh, the -a and --args exist and do the same
thing, and there's no need to fix that. The issue was with the autotest
python script. I found no trace of the -a option ever existing, and from my
brief testing, it didn't work, while --args did, although I only used the
heuristic of printing out the option right before autoserv is called and
there may have been other mystical ways how this was propagated all the way
to autoserv, which gives me even more confidence that the cleanup was long
overdue.

I've also found some living witnesses that seemed to swear that they used
the option and it just worked, but the last seen state where it worked has
been ages ago.

Sorry for breaking this, I really was under the impression that the option
has been broken by someone some time ago and was thus not used actively by
anyone.

LGTM with nits that you may ignore:
- You did not have to replace the -a with --args, as explained above.
- The whole local delim='' seems really needless. I'd convert it to just a
oneliner with hardcoded space instead of ${delim}, in fact having an
append_arg() function seems redundant, because in all places where it's
called, you can just do run_remote_args+=" foo", wrapping a oneline
assignment is probably too much.

On Fri, Sep 17, 2010 at 1:29 PM, <pstew@chromium.org> wrote:

> Reviewers: zbehan, djkurtz,
>
> Description:
> Re-do argument passing to match zbehan's change
>
> zbehan's change: "run_remote_tests: destroy the with_spaces hack,
> use the --args instead of -a (which doesn't work)", despite apparently
> not working is in active use by others, both by folks using it on the
> command line and by the script cros_run_wifi_tests.sh.  Fix the script
> to match the new argument.
>
> BUG=none
> TEST=rerun
>
> Please review this at http://codereview.chromium.org/3444010/show
>
> SVN Base: ssh://gitrw.chromium.org/crosutils.git
>
> Affected files:
>  M bin/cros_run_wifi_tests.sh
>
>
> Index: bin/cros_run_wifi_tests.sh
> diff --git a/bin/cros_run_wifi_tests.sh b/bin/cros_run_wifi_tests.sh
> index
>
c11f3134ee1e1f447bb11a9748e4984becc6dd42..f948a4ef4ca5d4b2ec345c59fcdab28d3e3f9bcc
> 100755
> --- a/bin/cros_run_wifi_tests.sh
> +++ b/bin/cros_run_wifi_tests.sh
> @@ -63,7 +63,7 @@ append_flag () {
>
>  append_arg () {
>     local delim=''
> -    [ -n "${run_remote_args}" ] && delim=','
> +    [ -n "${run_remote_args}" ] && delim=' '
>     run_remote_args="${run_remote_args}${delim}$*"
>  }
>
> @@ -145,5 +145,6 @@ for arg in $*; do
>     append_arg $arg
>  done
>
> -eval "exec "${script_root}/run_remote_tests.sh" \
> -    -a "${run_remote_args}" --remote=${remote} $run_remote_flags
> $FLAGS_ARGV"
> +eval "exec ${script_root}/run_remote_tests.sh \
> +      --args=\"${run_remote_args}\" --remote=${remote} $run_remote_flags \
> +      $FLAGS_ARGV"
>
>
>

[Paul Stewart, 2010-09-17 23:07:38]

Thanks for the feedback.  I'll probably keep both of those bits the same
because:

  * The --args is more clear as to its purpose and is more consistent with the
other flags
  * The append_ function might be wordy but it avoids having a leading
delimiter.  In the specific case of the "--args" parameter, I need to quote the
arg (since there are necessarily spaces in there) so leading spaces won't be
swallowed by the shell.  Who knows what leading spaces does to
run_remote_tests/autotest.  And of course, that could change. :-)

[Paul Stewart, 2010-09-17 23:07:41]

Thanks for the feedback.  I'll probably keep both of those bits the same
because:

  * The --args is more clear as to its purpose and is more consistent with the
other flags
  * The append_ function might be wordy but it avoids having a leading
delimiter.  In the specific case of the "--args" parameter, I need to quote the
arg (since there are necessarily spaces in there) so leading spaces won't be
swallowed by the shell.  Who knows what leading spaces does to
run_remote_tests/autotest.  And of course, that could change. :-)