From 8025b67f4a91c89c84e9425f55ca42755a7d5530 Mon Sep 17 00:00:00 2001 From: Nicholas Clark Date: Sun, 5 Jun 2011 17:06:14 +0200 Subject: [PATCH] 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. --- ext/IPC-Open3/t/IPC-Open3.t | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) 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())'; $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'; }; -- 2.7.4