diff mbox series

[v2] runltp: fix shellcheck error for sending email

Message ID 20180726083426.19026-1-yixin.zhang@intel.com
State Accepted
Delegated to: Petr Vorel
Headers show
Series [v2] runltp: fix shellcheck error for sending email | expand

Commit Message

Yixin Zhang July 26, 2018, 8:34 a.m. UTC
runltp:894:11: error: Couldn't parse this test expression. [SC1073]
runltp:894:13: error: If grouping expressions inside [..], use \( ..\). [SC1026]
runltp:894:15: error: Expected test to end here (don't wrap commands in []/[[]]).
                      Fix any mentioned problems and try again. [SC1072]

Change '-o' to '&&' according to the comment and context.
Then as "$LOGFILE_NAME" is always not empty, remove below "if" statement
and update indentation accordingly:

if [ -z "$HTMLFILE_NAME" ] && [ -z "$OUTPUTFILE_NAME" ] && [ -z "$LOGFILE_NAME" ]; then
  ##User does not have output/logs/html-output, nothing to be mailed in this situation
  echo "Nothing to be mailed here...."

Signed-off-by: Yixin Zhang <yixin.zhang@intel.com>
---
 runltp | 139 +++++++++++++++++++++++++++++++----------------------------------
 1 file changed, 67 insertions(+), 72 deletions(-)

Comments

Petr Vorel Oct. 15, 2018, 9:52 a.m. UTC | #1
Hi Yixin,

> runltp:894:11: error: Couldn't parse this test expression. [SC1073]
> runltp:894:13: error: If grouping expressions inside [..], use \( ..\). [SC1026]
> runltp:894:15: error: Expected test to end here (don't wrap commands in []/[[]]).
>                       Fix any mentioned problems and try again. [SC1072]

> Change '-o' to '&&' according to the comment and context.
> Then as "$LOGFILE_NAME" is always not empty, remove below "if" statement
> and update indentation accordingly:

> if [ -z "$HTMLFILE_NAME" ] && [ -z "$OUTPUTFILE_NAME" ] && [ -z "$LOGFILE_NAME" ]; then
>   ##User does not have output/logs/html-output, nothing to be mailed in this situation
>   echo "Nothing to be mailed here...."

Just to warn you this file si deprecated as Cyril Hrubis works on new runner [1] [2]
So it might be better test it than invest time into something which is going to
be deleted.

> Signed-off-by: Yixin Zhang <yixin.zhang@intel.com>
> ---
>  runltp | 139 +++++++++++++++++++++++++++++++----------------------------------
>  1 file changed, 67 insertions(+), 72 deletions(-)

...
> +    if [ "$ALT_EMAIL_OUT" -eq 1 ] ; then                    ## User wants reports to be e-mailed
> +        TAR_FILE_NAME=LTP_RUN_$version_date$DEFAULT_FILE_NAME_GENERATION_TIME.tar
> +        if [ "$HTMLFILE_NAME" ] ; then                      ## HTML file Exists
> +            if [ "$ALT_HTML_OUT" -ne 1 ] ; then             ## The HTML file path is absolute and not $LTPROOT/output
> +                mkdir -p $LTPROOT/output                    ## We need to create this Directory
> +                cp $HTMLFILE_NAME $LTPROOT/output/
> +            fi
> +        fi
> +        if [ "$OUTPUTFILE_NAME" ] ; then                    ## Output file exists
> +            if [ "$ALT_DIR_OUT" -ne 1 ] ; then              ## The Output file path is absolute and not $LTPROOT/output
> +                mkdir -p $LTPROOT/output                    ## We need to create this Directory
> +                cp $OUTPUTFILE_NAME $LTPROOT/output/
> +            fi
> +        fi
> +        if [ "$LOGFILE_NAME" ] ; then                       ## Log file exists
As I wrote in comment to v1 [3], $LOGFILE_NAME is always defined, so no need to check for it.
And the same is for for $OUTPUTFILE_NAME

> +            if [ "$ALT_DIR_RES" -ne 1 ] ; then              ## The Log file path is absolute and not $LTPROOT/results
> +                mkdir -p $LTPROOT/results                   ## We need to create this Directory
> +                cp $LOGFILE_NAME $LTPROOT/results/
> +            fi
> +        fi


Kind regards,
Petr

[1] https://hackweek.suse.com/17/projects/prototype-new-ltp-upstream-runltp-script
[2] https://github.com/metan-ucw/ltp
[3] http://lists.linux.it/pipermail/ltp/2018-July/008783.html
Yixin Zhang Oct. 15, 2018, 11:19 a.m. UTC | #2
Thanks Petr, please ignore the patch, I'll look into the new runner.

Yixin

On 2018-10-15 at 11:52:14 +0200, Petr Vorel wrote:
> Hi Yixin,
> 
> > runltp:894:11: error: Couldn't parse this test expression. [SC1073]
> > runltp:894:13: error: If grouping expressions inside [..], use \( ..\). [SC1026]
> > runltp:894:15: error: Expected test to end here (don't wrap commands in []/[[]]).
> >                       Fix any mentioned problems and try again. [SC1072]
> 
> > Change '-o' to '&&' according to the comment and context.
> > Then as "$LOGFILE_NAME" is always not empty, remove below "if" statement
> > and update indentation accordingly:
> 
> > if [ -z "$HTMLFILE_NAME" ] && [ -z "$OUTPUTFILE_NAME" ] && [ -z "$LOGFILE_NAME" ]; then
> >   ##User does not have output/logs/html-output, nothing to be mailed in this situation
> >   echo "Nothing to be mailed here...."
> 
> Just to warn you this file si deprecated as Cyril Hrubis works on new runner [1] [2]
> So it might be better test it than invest time into something which is going to
> be deleted.
> 
> > Signed-off-by: Yixin Zhang <yixin.zhang@intel.com>
> > ---
> >  runltp | 139 +++++++++++++++++++++++++++++++----------------------------------
> >  1 file changed, 67 insertions(+), 72 deletions(-)
> 
> ...
> > +    if [ "$ALT_EMAIL_OUT" -eq 1 ] ; then                    ## User wants reports to be e-mailed
> > +        TAR_FILE_NAME=LTP_RUN_$version_date$DEFAULT_FILE_NAME_GENERATION_TIME.tar
> > +        if [ "$HTMLFILE_NAME" ] ; then                      ## HTML file Exists
> > +            if [ "$ALT_HTML_OUT" -ne 1 ] ; then             ## The HTML file path is absolute and not $LTPROOT/output
> > +                mkdir -p $LTPROOT/output                    ## We need to create this Directory
> > +                cp $HTMLFILE_NAME $LTPROOT/output/
> > +            fi
> > +        fi
> > +        if [ "$OUTPUTFILE_NAME" ] ; then                    ## Output file exists
> > +            if [ "$ALT_DIR_OUT" -ne 1 ] ; then              ## The Output file path is absolute and not $LTPROOT/output
> > +                mkdir -p $LTPROOT/output                    ## We need to create this Directory
> > +                cp $OUTPUTFILE_NAME $LTPROOT/output/
> > +            fi
> > +        fi
> > +        if [ "$LOGFILE_NAME" ] ; then                       ## Log file exists
> As I wrote in comment to v1 [3], $LOGFILE_NAME is always defined, so no need to check for it.
> And the same is for for $OUTPUTFILE_NAME
> 
> > +            if [ "$ALT_DIR_RES" -ne 1 ] ; then              ## The Log file path is absolute and not $LTPROOT/results
> > +                mkdir -p $LTPROOT/results                   ## We need to create this Directory
> > +                cp $LOGFILE_NAME $LTPROOT/results/
> > +            fi
> > +        fi
> 
> 
> Kind regards,
> Petr
> 
> [1] https://hackweek.suse.com/17/projects/prototype-new-ltp-upstream-runltp-script
> [2] https://github.com/metan-ucw/ltp
> [3] http://lists.linux.it/pipermail/ltp/2018-July/008783.html
Dan Rue Oct. 15, 2018, 2:18 p.m. UTC | #3
On Mon, Oct 15, 2018 at 11:52:14AM +0200, Petr Vorel wrote:
> Hi Yixin,
> 
> > runltp:894:11: error: Couldn't parse this test expression. [SC1073]
> > runltp:894:13: error: If grouping expressions inside [..], use \( ..\). [SC1026]
> > runltp:894:15: error: Expected test to end here (don't wrap commands in []/[[]]).
> >                       Fix any mentioned problems and try again. [SC1072]
> 
> > Change '-o' to '&&' according to the comment and context.
> > Then as "$LOGFILE_NAME" is always not empty, remove below "if" statement
> > and update indentation accordingly:
> 
> > if [ -z "$HTMLFILE_NAME" ] && [ -z "$OUTPUTFILE_NAME" ] && [ -z "$LOGFILE_NAME" ]; then
> >   ##User does not have output/logs/html-output, nothing to be mailed in this situation
> >   echo "Nothing to be mailed here...."
> 
> Just to warn you this file si deprecated as Cyril Hrubis works on new runner [1] [2]
> So it might be better test it than invest time into something which is going to
> be deleted.

It might be easier to get people using it if it were in tree. Would it
be possible to have runltp and runltp-ng both in tree? Until then, it's
hard to not support the only available runner.

Looking at the README at
https://github.com/metan-ucw/ltp/tree/master/tools/runltp-ng, it's hard
to tell if it has feature parity with runltp, or if there are still
gaps. Some example usecases would be nice too. For example, I see the
list of back ends, but what if I want to run it locally (today's
behavior)?

Dan

> 
> > Signed-off-by: Yixin Zhang <yixin.zhang@intel.com>
> > ---
> >  runltp | 139 +++++++++++++++++++++++++++++++----------------------------------
> >  1 file changed, 67 insertions(+), 72 deletions(-)
> 
> ...
> > +    if [ "$ALT_EMAIL_OUT" -eq 1 ] ; then                    ## User wants reports to be e-mailed
> > +        TAR_FILE_NAME=LTP_RUN_$version_date$DEFAULT_FILE_NAME_GENERATION_TIME.tar
> > +        if [ "$HTMLFILE_NAME" ] ; then                      ## HTML file Exists
> > +            if [ "$ALT_HTML_OUT" -ne 1 ] ; then             ## The HTML file path is absolute and not $LTPROOT/output
> > +                mkdir -p $LTPROOT/output                    ## We need to create this Directory
> > +                cp $HTMLFILE_NAME $LTPROOT/output/
> > +            fi
> > +        fi
> > +        if [ "$OUTPUTFILE_NAME" ] ; then                    ## Output file exists
> > +            if [ "$ALT_DIR_OUT" -ne 1 ] ; then              ## The Output file path is absolute and not $LTPROOT/output
> > +                mkdir -p $LTPROOT/output                    ## We need to create this Directory
> > +                cp $OUTPUTFILE_NAME $LTPROOT/output/
> > +            fi
> > +        fi
> > +        if [ "$LOGFILE_NAME" ] ; then                       ## Log file exists
> As I wrote in comment to v1 [3], $LOGFILE_NAME is always defined, so no need to check for it.
> And the same is for for $OUTPUTFILE_NAME
> 
> > +            if [ "$ALT_DIR_RES" -ne 1 ] ; then              ## The Log file path is absolute and not $LTPROOT/results
> > +                mkdir -p $LTPROOT/results                   ## We need to create this Directory
> > +                cp $LOGFILE_NAME $LTPROOT/results/
> > +            fi
> > +        fi
> 
> 
> Kind regards,
> Petr
> 
> [1] https://hackweek.suse.com/17/projects/prototype-new-ltp-upstream-runltp-script
> [2] https://github.com/metan-ucw/ltp
> [3] http://lists.linux.it/pipermail/ltp/2018-July/008783.html
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp
Cyril Hrubis Oct. 15, 2018, 2:51 p.m. UTC | #4
Hi!
> It might be easier to get people using it if it were in tree. Would it
> be possible to have runltp and runltp-ng both in tree? Until then, it's
> hard to not support the only available runner.

That is a fair point.

> Looking at the README at
> https://github.com/metan-ucw/ltp/tree/master/tools/runltp-ng, it's hard
> to tell if it has feature parity with runltp, or if there are still
> gaps. Some example usecases would be nice too. For example, I see the
> list of back ends, but what if I want to run it locally (today's
> behavior)?

And the lack of documentation and the fact that the code is not yet
considered stable enough is the reason why it's not in-tree.

I guess that the test-runner can be put into a separate git repo, it
does not depend on the rest of the LTP in any way, which should make it
easier to get the source code, without pulling in the outdated LTP
repostitory.

Also the local test execution is called shell backend, in that case the
testrunner forks a shell instead of doing a ssh connection and executes
the testcases locally.
Petr Vorel Oct. 15, 2018, 7:07 p.m. UTC | #5
Hi Yixin,

> > Signed-off-by: Yixin Zhang <yixin.zhang@intel.com>
> > ---
> >  runltp | 139 +++++++++++++++++++++++++++++++----------------------------------
> >  1 file changed, 67 insertions(+), 72 deletions(-)

> ...
> > +    if [ "$ALT_EMAIL_OUT" -eq 1 ] ; then                    ## User wants reports to be e-mailed
> > +        TAR_FILE_NAME=LTP_RUN_$version_date$DEFAULT_FILE_NAME_GENERATION_TIME.tar
> > +        if [ "$HTMLFILE_NAME" ] ; then                      ## HTML file Exists
> > +            if [ "$ALT_HTML_OUT" -ne 1 ] ; then             ## The HTML file path is absolute and not $LTPROOT/output
> > +                mkdir -p $LTPROOT/output                    ## We need to create this Directory
> > +                cp $HTMLFILE_NAME $LTPROOT/output/
> > +            fi
> > +        fi
> > +        if [ "$OUTPUTFILE_NAME" ] ; then                    ## Output file exists
> > +            if [ "$ALT_DIR_OUT" -ne 1 ] ; then              ## The Output file path is absolute and not $LTPROOT/output
> > +                mkdir -p $LTPROOT/output                    ## We need to create this Directory
> > +                cp $OUTPUTFILE_NAME $LTPROOT/output/
> > +            fi
> > +        fi
> > +        if [ "$LOGFILE_NAME" ] ; then                       ## Log file exists
> As I wrote in comment to v1 [3], $LOGFILE_NAME is always defined, so no need to check for it.
> And the same is for for $OUTPUTFILE_NAME
Although we should omit this check, I ignored that because the state of the file
(it's in maintenance mode). The main point of this commit was to fix basishm and
remove dead branch.

Pushed, with commit message adjustment.
Thanks for your time and care.

Kind regards,
Petr
Petr Vorel Oct. 15, 2018, 7:16 p.m. UTC | #6
Hi,

> > It might be easier to get people using it if it were in tree. Would it
> > be possible to have runltp and runltp-ng both in tree? Until then, it's
> > hard to not support the only available runner.

> That is a fair point.
You're right. I pushed the commit (no matter the state of the code, bashisms
should be fixed).

> > Looking at the README at
> > https://github.com/metan-ucw/ltp/tree/master/tools/runltp-ng, it's hard
> > to tell if it has feature parity with runltp, or if there are still
> > gaps. Some example usecases would be nice too. For example, I see the
> > list of back ends, but what if I want to run it locally (today's
> > behavior)?

> And the lack of documentation and the fact that the code is not yet
> considered stable enough is the reason why it's not in-tree.

Sorry for pointing to runltp-ng, which might not be yet ready for using/testing.
I meant that more like info for users, not to waste time with something
which should have been either rewritten from scratch or replaced.

> I guess that the test-runner can be put into a separate git repo, it
> does not depend on the rest of the LTP in any way, which should make it
> easier to get the source code, without pulling in the outdated LTP
> repostitory.

> Also the local test execution is called shell backend, in that case the
> testrunner forks a shell instead of doing a ssh connection and executes
> the testcases locally.

I haven't looked into the code myself (planning to), but it's really promising :).

Kind regards,
Petr
diff mbox series

Patch

diff --git a/runltp b/runltp
index 24ea94fe5..3f3aa4f53 100755
--- a/runltp
+++ b/runltp
@@ -890,78 +890,73 @@  main()
 
     fi
 
-    if [ "$ALT_EMAIL_OUT" -eq 1 ] ; then                      ## User wants reports to be e-mailed
-       if [ [ ! "$HTMLFILE_NAME" ] -o [ ! "$OUTPUTFILE_NAME" ] -o [ ! "$LOGFILE_NAME" ] ] ; then
-          ##User does not have output/logs/html-output, nothing to be mailed in this situation
-          echo "Nothing to be mailed here...."
-       else
-           TAR_FILE_NAME=LTP_RUN_$version_date$DEFAULT_FILE_NAME_GENERATION_TIME.tar
-           if [ "$HTMLFILE_NAME" ] ; then                          ## HTML file Exists
-              if [ "$ALT_HTML_OUT" -ne 1 ] ; then                  ## The HTML file path is absolute and not $LTPROOT/output
-                 mkdir -p $LTPROOT/output                          ## We need to create this Directory
-                 cp $HTMLFILE_NAME $LTPROOT/output/
-              fi
-           fi
-           if [ "$OUTPUTFILE_NAME" ] ; then                        ## Output file exists
-              if [ "$ALT_DIR_OUT" -ne 1 ] ; then                   ## The Output file path is absolute and not $LTPROOT/output
-                 mkdir -p $LTPROOT/output                          ## We need to create this Directory
-                 cp $OUTPUTFILE_NAME $LTPROOT/output/
-              fi
-           fi
-           if [ "$LOGFILE_NAME" ] ; then                           ## Log file exists
-              if [ "$ALT_DIR_RES" -ne 1 ] ; then                   ## The Log file path is absolute and not $LTPROOT/results
-                 mkdir -p $LTPROOT/results                         ## We need to create this Directory
-                 cp $LOGFILE_NAME $LTPROOT/results/
-              fi
-           fi
-           if [ -d $LTPROOT/output ] ; then
-              tar -cf  ./$TAR_FILE_NAME $LTPROOT/output
-              if [ $? -eq 0 ]; then
-                 echo "Created TAR File: ./$TAR_FILE_NAME successfully, added $LTPROOT/output"
-              else
-                 echo "Cannot Create TAR File: ./$TAR_FILE_NAME for adding $LTPROOT/output"
-              fi
-           fi
-           if [ -d $LTPROOT/results ] ; then
-              tar -uf ./$TAR_FILE_NAME $LTPROOT/results
-              if [ $? -eq 0 ]; then
-                 echo "Updated TAR File: ./$TAR_FILE_NAME successfully, added $LTPROOT/results"
-              else
-                 echo "Cannot Update TAR File: ./$TAR_FILE_NAME for adding $LTPROOT/results"
-              fi
-           fi
-           if [ -e $LTPROOT/nohup.out ] ; then                     ## If User would have Chosen nohup to do ltprun
-              tar -uf ./$TAR_FILE_NAME $LTPROOT/nohup.out
-              if [ $? -eq 0 ]; then
-                 echo "Updated TAR File: ./$TAR_FILE_NAME successfully, added $LTPROOT/nohup.out"
-              else
-                 echo "Cannot Update TAR File: ./$TAR_FILE_NAME for adding $LTPROOT/nohup.out"
-              fi
-           fi
-           gzip ./$TAR_FILE_NAME                                     ## gzip this guy
-           if [ $? -eq 0 ]; then
-              echo "Gunzipped TAR File: ./$TAR_FILE_NAME"
-           else
-              echo "Cannot Gunzip TAR File: ./$TAR_FILE_NAME"
-           fi
-           if which mutt >/dev/null 2>&1; then
-              echo "Starting mailing reports to: $EMAIL_TO, file: ./$TAR_FILE_NAME.gz"
-              mutt -a ./$TAR_FILE_NAME.gz -s "LTP Reports on $test_start_time" -- $EMAIL_TO < /dev/null
-              if [ $? -eq 0 ]; then
-                 echo "Reports Successfully mailed to: $EMAIL_TO"
-              else
-                 echo "Reports cannot be mailed to: $EMAIL_TO"
-              fi
-           else ## Use our Ageold mail program
-              echo "Starting mailing reports to: $EMAIL_TO, file: ./$TAR_FILE_NAME.gz"
-              uuencode ./$TAR_FILE_NAME.gz $TAR_FILE_NAME.gz | mail  $EMAIL_TO -s "LTP Reports on $test_start_time"
-              if [ $? -eq 0 ]; then
-                 echo "Reports Successfully mailed to: $EMAIL_TO"
-              else
-                 echo "Reports cannot be mailed to: $EMAIL_TO"
-              fi
-           fi
-       fi
+    if [ "$ALT_EMAIL_OUT" -eq 1 ] ; then                    ## User wants reports to be e-mailed
+        TAR_FILE_NAME=LTP_RUN_$version_date$DEFAULT_FILE_NAME_GENERATION_TIME.tar
+        if [ "$HTMLFILE_NAME" ] ; then                      ## HTML file Exists
+            if [ "$ALT_HTML_OUT" -ne 1 ] ; then             ## The HTML file path is absolute and not $LTPROOT/output
+                mkdir -p $LTPROOT/output                    ## We need to create this Directory
+                cp $HTMLFILE_NAME $LTPROOT/output/
+            fi
+        fi
+        if [ "$OUTPUTFILE_NAME" ] ; then                    ## Output file exists
+            if [ "$ALT_DIR_OUT" -ne 1 ] ; then              ## The Output file path is absolute and not $LTPROOT/output
+                mkdir -p $LTPROOT/output                    ## We need to create this Directory
+                cp $OUTPUTFILE_NAME $LTPROOT/output/
+            fi
+        fi
+        if [ "$LOGFILE_NAME" ] ; then                       ## Log file exists
+            if [ "$ALT_DIR_RES" -ne 1 ] ; then              ## The Log file path is absolute and not $LTPROOT/results
+                mkdir -p $LTPROOT/results                   ## We need to create this Directory
+                cp $LOGFILE_NAME $LTPROOT/results/
+            fi
+        fi
+        if [ -d $LTPROOT/output ] ; then
+            tar -cf  ./$TAR_FILE_NAME $LTPROOT/output
+            if [ $? -eq 0 ]; then
+                echo "Created TAR File: ./$TAR_FILE_NAME successfully, added $LTPROOT/output"
+            else
+                echo "Cannot Create TAR File: ./$TAR_FILE_NAME for adding $LTPROOT/output"
+            fi
+        fi
+        if [ -d $LTPROOT/results ] ; then
+            tar -uf ./$TAR_FILE_NAME $LTPROOT/results
+            if [ $? -eq 0 ]; then
+                echo "Updated TAR File: ./$TAR_FILE_NAME successfully, added $LTPROOT/results"
+            else
+                echo "Cannot Update TAR File: ./$TAR_FILE_NAME for adding $LTPROOT/results"
+            fi
+        fi
+        if [ -e $LTPROOT/nohup.out ] ; then                 ## If User would have Chosen nohup to do ltprun
+            tar -uf ./$TAR_FILE_NAME $LTPROOT/nohup.out
+            if [ $? -eq 0 ]; then
+                echo "Updated TAR File: ./$TAR_FILE_NAME successfully, added $LTPROOT/nohup.out"
+            else
+                echo "Cannot Update TAR File: ./$TAR_FILE_NAME for adding $LTPROOT/nohup.out"
+            fi
+        fi
+        gzip ./$TAR_FILE_NAME                               ## gzip this guy
+        if [ $? -eq 0 ]; then
+            echo "Gunzipped TAR File: ./$TAR_FILE_NAME"
+        else
+            echo "Cannot Gunzip TAR File: ./$TAR_FILE_NAME"
+        fi
+        if which mutt >/dev/null 2>&1; then
+            echo "Starting mailing reports to: $EMAIL_TO, file: ./$TAR_FILE_NAME.gz"
+            mutt -a ./$TAR_FILE_NAME.gz -s "LTP Reports on $test_start_time" -- $EMAIL_TO < /dev/null
+            if [ $? -eq 0 ]; then
+                echo "Reports Successfully mailed to: $EMAIL_TO"
+            else
+                echo "Reports cannot be mailed to: $EMAIL_TO"
+            fi
+        else ## Use our Ageold mail program
+            echo "Starting mailing reports to: $EMAIL_TO, file: ./$TAR_FILE_NAME.gz"
+            uuencode ./$TAR_FILE_NAME.gz $TAR_FILE_NAME.gz | mail  $EMAIL_TO -s "LTP Reports on $test_start_time"
+            if [ $? -eq 0 ]; then
+                echo "Reports Successfully mailed to: $EMAIL_TO"
+            else
+                echo "Reports cannot be mailed to: $EMAIL_TO"
+            fi
+        fi
     fi
 
     [ ! -z "$QUIET_MODE" ] && { echo "INFO: Test end time: $(date)" ; }