Simplify the test for IPC::Open bug RT #72016.
authorNicholas Clark <nick@ccl4.org>
Sun, 5 Jun 2011 15:06:14 +0000 (17:06 +0200)
committerNicholas Clark <nick@ccl4.org>
Sat, 11 Jun 2011 06:48:13 +0000 (08:48 +0200)
The original bug was a request that errors be reported in the parent process,
with a TODO test, and then a patch that added the feature for the !DO_SPAWN
case, and removed the TODO. The *implication* of the bug report and the way the
original test was only TODO for the !DO_SPAWN case was that errors were
reported inconsistently between the two code paths of open3().

However, this is not the case - the DO_SPAWN path through open3() return a
(pseudo) PID (and no error) when asked to run a non-existent program. Hence
there is now a feature discrepancy between the alternative implementations,
which feels like a bug that should (ultimately) be addressed.

The original test could have expressed that more directly with one code path
and a TODO. The refactoring of bd29e8c290c68f4f failed to spot this, and
introduced new logic errors in the DO_SPAWN path - waitpid() should not be
called if $@ is set.

Set $pid outside the eval {} - this makes sure it is (re)set to undef if the
eval fails, instead of holding its previous (now bogus) value.

ext/IPC-Open3/t/IPC-Open3.t

index 09c44d7..79dd936 100644 (file)
@@ -133,7 +133,7 @@ EOF
 # for understanding of Config{'sh'} test see exec description in camel book
 my $cmd = 'print(scalar(<STDIN>))';
 $cmd = $Config{'sh'} =~ /sh/ ? "'$cmd'" : cmd_line($cmd);
-eval{$pid = open3 'WRITE', '>&STDOUT', 'ERROR', "$perl -e " . $cmd; };
+$pid = eval { open3 'WRITE', '>&STDOUT', 'ERROR', "$perl -e " . $cmd; };
 if ($@) {
        print "error $@\n";
        ++$test;
@@ -147,15 +147,13 @@ else {
 $TB->current_test($test);
 
 # RT 72016
-eval{$pid = open3 'WRITE', 'READ', 'ERROR', '/non/existent/program'; };
-if (IPC::Open3::DO_SPAWN) {
-    if ($@) {
-       cmp_ok(waitpid($pid, 0), '>', 0);
-    } else {
-       pass();
-    }
-} else {
-    isnt($@, '') or do {waitpid $pid, 0};
+{
+    local $::TODO = "$^O returns a pid and doesn't throw an exception"
+       if $^O eq 'MSWin32';
+    $pid = eval { open3 'WRITE', 'READ', 'ERROR', '/non/existent/program'; };
+    isnt($@, '',
+        'open3 of a non existent program fails with an exception in the parent')
+       or do {waitpid $pid, 0};
 }
 
 $pid = eval { open3 'WRITE', '', 'ERROR', '/non/existent/program'; };