[PATCH] Python sub-process error checking

[Philip Martin <ph...@codematters.co.uk>]

Hello

The following patch makes the regression tests raise an error if a
process started by run_command crashes.  This results in better error
checking on Unix, a command that crashes will be detected
automatically rather than relying on the crash being detected by the
absence of some effect.  The disadvantage is that if a test is written
that relies on this new behaviour then it may not be effective on
Windows, it may pass when it should fail.

So, is this a good idea?


Better error checking on Unix platforms.  Based on an old patch
from William Uther <wi...@cs.cmu.edu>.

* subversion/tests/clients/cmdline/svntest/main.py (run_command): Use
  Popen3 on POSIX platforms to detect commands that crash.


Index: subversion/tests/clients/cmdline/svntest/main.py
===================================================================
--- subversion/tests/clients/cmdline/svntest/main.py	(revision 5578)
+++ subversion/tests/clients/cmdline/svntest/main.py	(working copy)
@@ -25,6 +25,9 @@
 import copy    # for deepcopy()
 import time    # for time()
 
+if os.name == 'posix':
+  import popen2
+
 from svntest import Failure
 from svntest import testcase
 from svntest import wc
@@ -229,11 +232,26 @@
     mode = 't'
 
   start = time.time()
-  infile, outfile, errfile = os.popen3(command + args, mode)
+  if os.name == 'posix':
+    child = popen2.Popen3(command + args, 1)
 
-  stdout_lines = outfile.readlines()
-  stderr_lines = errfile.readlines()
+    stdout_lines = child.fromchild.readlines()
+    stderr_lines = child.childerr.readlines()
+    result = child.wait()
+    if os.WIFSIGNALED(result):
+      raise Failure("Signal " + str(os.WTERMSIG(result)) + " from " \
+                    + os.path.basename(command) + args)
 
+    outfile = child.fromchild
+    infile = child.tochild
+    errfile = child.childerr
+
+  else:
+    infile, outfile, errfile = os.popen3(command + args, mode)
+
+    stdout_lines = outfile.readlines()
+    stderr_lines = errfile.readlines()
+
   outfile.close()
   infile.close()
   errfile.close()

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Python sub-process error checking

[kf...@collab.net]

Philip Martin <ph...@codematters.co.uk> writes:
> The following patch makes the regression tests raise an error if a
> process started by run_command crashes.  This results in better error
> checking on Unix, a command that crashes will be detected
> automatically rather than relying on the crash being detected by the
> absence of some effect.  The disadvantage is that if a test is written
> that relies on this new behaviour then it may not be effective on
> Windows, it may pass when it should fail.
> 
> So, is this a good idea?

Because of the concerns you mentioned, I think it's probably not a
good idea.  As annoying as it is, we just need to write our tests to
notice the lack of output (or other external symptom) of a crash
program, instead of noticing the crash itself.  Otherwise, people
developing on Unix would be tempted to write tests depending on the
behavior, and those tests would be no good on other platforms.

(If this were ever solved portably in Python, then we could switch.)

Not a veto, though.  If a majority of the other committers think this
is still a good idea, that's fine.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org