From: Nicholas Clark <nick@ccl4.org>
Date: Sun, 5 Jun 2011 15:06:14 +0000 (+0200)
Subject: Simplify the test for IPC::Open bug RT #72016.
X-Git-Tag: accepted/trunk/20130322.191538~3855
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=8025b67f4a91c89c84e9425f55ca42755a7d5530;p=platform%2Fupstream%2Fperl.git

Simplify the test for IPC::Open bug RT #72016.

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.
---

diff --git a/ext/IPC-Open3/t/IPC-Open3.t b/ext/IPC-Open3/t/IPC-Open3.t
index 09c44d7..79dd936 100644
--- a/ext/IPC-Open3/t/IPC-Open3.t
+++ b/ext/IPC-Open3/t/IPC-Open3.t
@@ -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'; };