Re: [GUILT v3 09/31] Test suite: properly check the exit status of commands.

2014-05-18 Thread Per Cederqvist 
On Fri, May 16, 2014 at 5:45 PM, Jeff Sipek jef...@josefsipek.net wrote:
 On Fri, May 16, 2014 at 04:45:56PM +0200, Per Cederqvist wrote:
 The cmd and shouldfail functions checked the exit status of the
 replace_path function instead of the actual command that was running.
 (The $? construct checks the exit status of the last command in a
 pipeline, not the first command.)

 Updated t-032.sh, which used shouldfail instead of cmd in one
 place.  (The comment in the script makes it clear that the command is
 expected to succeed.)

 Signed-off-by: Per Cederqvist ced...@opera.com
 ---
  regression/scaffold | 17 +++--
  regression/t-032.sh |  2 +-
  2 files changed, 12 insertions(+), 7 deletions(-)

 diff --git a/regression/scaffold b/regression/scaffold
 index 5c8b73e..e4d7487 100644
 --- a/regression/scaffold
 +++ b/regression/scaffold
 @@ -51,18 +51,23 @@ function filter_dd
  function cmd
  {
   echo % $@
 - $@ 21 | replace_path  return 0
 - return 1
 + (
 + exec 31
 + rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`

 Wow.  This took a while to decipher :)

Ancien wisdom from the Csh Programming Considered Harmful
article: http://www.faqs.org/faqs/unix-faq/shell/csh-whynot/

These functions work only because of the set -e earlier in scaffold.
The final return statements are not actually reached. I don't like that.
So the next version of the patch series will print an explicit message
like % FAIL: The above command should succeed but failed. or
% FAIL: The above command should fail but succeeded. and do
an explicit exit 1 on failure. I think it makes it easier to debug issues.
(I recently spent a few hours trying to figure our why the test just silently
exited. Turns out the old git version I was running didn't like my .gitconfig,
so it exited with a non-zero exit code...)

 Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net

I'll let you re-check the next version of the code.

/ceder

 + exit $rv
 + )
 + return $?
  }

  # usage: shouldfail cmd..
  function shouldfail
  {
   echo % $@
 - (
 - $@ 21 || return 0
 - return 1
 - ) | replace_path
 + ! (
 + exec 31
 + rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
 + exit $rv
 + )
   return $?
  }

 diff --git a/regression/t-032.sh b/regression/t-032.sh
 index b1d5f19..bba401e 100755
 --- a/regression/t-032.sh
 +++ b/regression/t-032.sh
 @@ -28,7 +28,7 @@ shouldfail guilt import -P foo3 foo
  cmd guilt import -P foo2 foo

  # ok
 -shouldfail guilt import foo
 +cmd guilt import foo

  # duplicate patch name (implicit)
  shouldfail guilt import foo
 --
 1.8.3.1


 --
 Fact: 28.1% of all statistics are generated randomly.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GUILT v3 09/31] Test suite: properly check the exit status of commands.

2014-05-16 Thread Per Cederqvist 
The cmd and shouldfail functions checked the exit status of the
replace_path function instead of the actual command that was running.
(The $? construct checks the exit status of the last command in a
pipeline, not the first command.)

Updated t-032.sh, which used shouldfail instead of cmd in one
place.  (The comment in the script makes it clear that the command is
expected to succeed.)

Signed-off-by: Per Cederqvist ced...@opera.com
---
 regression/scaffold | 17 +++--
 regression/t-032.sh |  2 +-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/regression/scaffold b/regression/scaffold
index 5c8b73e..e4d7487 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -51,18 +51,23 @@ function filter_dd
 function cmd
 {
echo % $@
-   $@ 21 | replace_path  return 0
-   return 1
+   (
+   exec 31
+   rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
+   exit $rv
+   )
+   return $?
 }
 
 # usage: shouldfail cmd..
 function shouldfail
 {
echo % $@
-   (
-   $@ 21 || return 0
-   return 1
-   ) | replace_path
+   ! (
+   exec 31
+   rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
+   exit $rv
+   )
return $?
 }
 
diff --git a/regression/t-032.sh b/regression/t-032.sh
index b1d5f19..bba401e 100755
--- a/regression/t-032.sh
+++ b/regression/t-032.sh
@@ -28,7 +28,7 @@ shouldfail guilt import -P foo3 foo
 cmd guilt import -P foo2 foo
 
 # ok
-shouldfail guilt import foo
+cmd guilt import foo
 
 # duplicate patch name (implicit)
 shouldfail guilt import foo
-- 
1.8.3.1

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GUILT v3 09/31] Test suite: properly check the exit status of commands.

2014-05-16 Thread Jeff Sipek 
On Fri, May 16, 2014 at 04:45:56PM +0200, Per Cederqvist wrote:
 The cmd and shouldfail functions checked the exit status of the
 replace_path function instead of the actual command that was running.
 (The $? construct checks the exit status of the last command in a
 pipeline, not the first command.)
 
 Updated t-032.sh, which used shouldfail instead of cmd in one
 place.  (The comment in the script makes it clear that the command is
 expected to succeed.)
 
 Signed-off-by: Per Cederqvist ced...@opera.com
 ---
  regression/scaffold | 17 +++--
  regression/t-032.sh |  2 +-
  2 files changed, 12 insertions(+), 7 deletions(-)
 
 diff --git a/regression/scaffold b/regression/scaffold
 index 5c8b73e..e4d7487 100644
 --- a/regression/scaffold
 +++ b/regression/scaffold
 @@ -51,18 +51,23 @@ function filter_dd
  function cmd
  {
   echo % $@
 - $@ 21 | replace_path  return 0
 - return 1
 + (
 + exec 31
 + rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`

Wow.  This took a while to decipher :)

Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net


 + exit $rv
 + )
 + return $?
  }
  
  # usage: shouldfail cmd..
  function shouldfail
  {
   echo % $@
 - (
 - $@ 21 || return 0
 - return 1
 - ) | replace_path
 + ! (
 + exec 31
 + rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
 + exit $rv
 + )
   return $?
  }
  
 diff --git a/regression/t-032.sh b/regression/t-032.sh
 index b1d5f19..bba401e 100755
 --- a/regression/t-032.sh
 +++ b/regression/t-032.sh
 @@ -28,7 +28,7 @@ shouldfail guilt import -P foo3 foo
  cmd guilt import -P foo2 foo
  
  # ok
 -shouldfail guilt import foo
 +cmd guilt import foo
  
  # duplicate patch name (implicit)
  shouldfail guilt import foo
 -- 
 1.8.3.1
 

-- 
Fact: 28.1% of all statistics are generated randomly.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html