[v1,4/7] OssTest: Add target_cmd_root_rc which returns return code.

Commit Message

Konrad Rzeszutek Wilk Nov. 17, 2016, 1:37 a.m. UTC

All the different target_cmd_* end up calling tcmdex
which has the unfortunate side-effect of calling 'die' if
the SSH sessions results in any return code not zero.

That is fine, except for tests where we want to get a non-zero
return value.

This patch moves the: die "status $r" from tcmdex
to all the other functions which use tcmdex.

Also we make tcmd behave in the normal old way (die "status $r")
or if returnrc is set to one, we will return the error code.

The only exposed function that does that is target_cmd_root_rc.

Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>
---
 Osstest/TestSupport.pm | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

Comments

Ian Jackson Nov. 17, 2016, 11:53 a.m. UTC | #1

Konrad Rzeszutek Wilk writes ("[PATCH v1 4/7] OssTest: Add target_cmd_root_rc which returns return code."):
> All the different target_cmd_* end up calling tcmdex
> which has the unfortunate side-effect of calling 'die' if
> the SSH sessions results in any return code not zero.
...
> Also we make tcmd behave in the normal old way (die "status $r")
> or if returnrc is set to one, we will return the error code.

Why not add the $returnrc argument to tcmdex ?  That would mean just
adding an extra argument to the callers, rather than extra code.
(Also I would call it "$badstatusok" or something.)

> -sub target_cmd ($$;$$) { tcmd(undef,undef,'osstest',@_); }
> -sub target_cmd_root ($$;$$) { tcmd(undef,undef,'root',@_); }
> +sub target_cmd ($$;$$) { tcmd(undef,undef,0, 'osstest',@_); }
> +sub target_cmd_root ($$;$$) { tcmd(undef,undef,0, 'root',@_); }
> +sub target_cmd_root_rc ($$;$$) { tcmd(undef,undef,1, 'root',@_); }

Can you call this "target_cmd_root_status" please ?

Thanks,
Ian.

Patch

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 888f0ac..92f043a 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -51,6 +51,7 @@  BEGIN {
                       get_runvar_default need_runvars
                       unique_incrementing_runvar next_unique_name
 
+                      target_cmd_root_rc
                       target_cmd_root target_cmd target_cmd_build
                       target_cmd_output_root target_cmd_output
                       target_cmd_inputfh_root sshuho
@@ -424,17 +425,17 @@  sub tcmdex {
     # We use timeout(1) as a backstop, in case $cmd doesn't die.  We
     # need $cmd to die because we won't release the resources we own
     # until all of our children are dead.
-    my $r= cmd($timeout,$stdin,$stdout,
+    cmd($timeout,$stdin,$stdout,
 	       'timeout',$timeout+30, $cmd,@$optsref,@args);
-    $r and die "status $r";
 }
 
 sub tgetfileex {
     my ($ruser, $ho,$timeout, $rsrc,$ldst) = @_;
     unlink $ldst or $!==&ENOENT or die "$ldst $!";
-    tcmdex($timeout,undef,undef,
+    my $r = tcmdex($timeout,undef,undef,
            'scp', sshopts(),
            sshuho($ruser,$ho).":$rsrc", $ldst);
+    $r and die "status $r";
 } 
 sub target_getfile ($$$$) {
     my ($ho,$timeout, $rsrc,$ldst) = @_;
@@ -448,16 +449,18 @@  sub target_getfile_root ($$$$) {
 sub tputfileex {
     my ($ruser, $ho,$timeout, $lsrc,$rdst, $rsync) = @_;
     my @args= ($lsrc, sshuho($ruser,$ho).":$rdst");
+    my $r = 0;
     if (!defined $rsync) {
-        tcmdex($timeout,undef,undef,
+        $r = tcmdex($timeout,undef,undef,
                'scp', sshopts(),
                @args);
     } else {
         unshift @args, $rsync if length $rsync;
-        tcmdex($timeout,undef,undef,
+        $r = tcmdex($timeout,undef,undef,
                'rsync', [ '-e', 'ssh '.join(' ',@{ sshopts() }) ],
                @args);
     }
+    $r and die "status $r";
 }
 sub target_putfile ($$$$;$) {
     # $ho,$timeout,$lsrc,$rdst,[$rsync_opt]
@@ -652,19 +655,22 @@  sub target_await_down ($$) {
 }    
 
 sub tcmd { # $tcmd will be put between '' but not escaped
-    my ($stdin,$stdout,$user,$ho,$tcmd,$timeout,$extrasshopts) = @_;
+    my ($stdin,$stdout,$returnrc,$user,$ho,$tcmd,$timeout,$extrasshopts) = @_;
     $timeout=30 if !defined $timeout;
     target_adjust_timeout($ho,\$timeout);
-    tcmdex($timeout,$stdin,$stdout,
+    my $r = tcmdex($timeout,$stdin,$stdout,
            'ssh', sshopts(), @{ $extrasshopts || [] },
            sshuho($user,$ho), $tcmd);
+    die "status $r" if $r and !$returnrc;
+    return $r;
 }
-sub target_cmd ($$;$$) { tcmd(undef,undef,'osstest',@_); }
-sub target_cmd_root ($$;$$) { tcmd(undef,undef,'root',@_); }
+sub target_cmd ($$;$$) { tcmd(undef,undef,0, 'osstest',@_); }
+sub target_cmd_root ($$;$$) { tcmd(undef,undef,0, 'root',@_); }
+sub target_cmd_root_rc ($$;$$) { tcmd(undef,undef,1, 'root',@_); }
 
 sub tcmdout {
     my $stdout= IO::File::new_tmpfile();
-    tcmd(undef,$stdout,@_);
+    tcmd(undef,$stdout,0,@_);
     $stdout->seek(0,0) or die "$stdout $!";
     my $r;
     { local ($/) = undef;
@@ -679,7 +685,7 @@  sub target_cmd_output_root ($$;$) { tcmdout('root',@_); }
 
 sub target_cmd_inputfh_root ($$$;$$) {
     my ($tho,$stdinfh,$tcmd,@rest) = @_;
-    tcmd($stdinfh,undef,'root',$tho,$tcmd,@rest);
+    tcmd($stdinfh,undef,0,'root',$tho,$tcmd,@rest);
 }
 
 sub poll_loop ($$$&) {