changeset 21763:84cd5ee787ed

run-tests: hold iolock across diff/prompt when interactive

Otherwise diff may get separated from the corresponding prompt by
other threads. This required moving the interactive prompting from one
helper method to another.

--- a/tests/run-tests.py	Wed Jun 18 15:26:07 2014 -0500
+++ b/tests/run-tests.py	Wed Jun 18 20:51:49 2014 -0500
@@ -543,7 +543,9 @@
                 f.close()
 
             # The result object handles diff calculation for us.
-            self._result.addOutputMismatch(self, ret, out, self._refout)
+            if self._result.addOutputMismatch(self, ret, out, self._refout):
+                # change was accepted, skip failing
+                return
 
             if ret:
                 msg = 'output changed and ' + describe(ret)
@@ -1084,19 +1086,6 @@
             if not self._options.nodiff:
                 self.stream.write('\nERROR: %s output changed\n' % test)
 
-            if self._options.interactive:
-                iolock.acquire()
-                self.stream.write('Accept this change? [n] ')
-                answer = sys.stdin.readline().strip()
-                iolock.release()
-                if answer.lower() in ('y', 'yes'):
-                    if test.name.endswith('.t'):
-                        rename(test.errpath, test.path)
-                    else:
-                        rename(test.errpath, '%s.out' % test.path)
-                    self.failures.pop()
-                    return 1
-
             self.stream.write('!')
 
     def addError(self, *args, **kwargs):
@@ -1140,10 +1129,12 @@
     def addOutputMismatch(self, test, ret, got, expected):
         """Record a mismatch in test output for a particular test."""
 
+        accepted = False
+
+        iolock.acquire()
         if self._options.nodiff:
-            return
-
-        if self._options.view:
+            pass
+        elif self._options.view:
             os.system("%s %s %s" % (self._view, test.refpath, test.errpath))
         else:
             failed, lines = getdiff(expected, got,
@@ -1156,9 +1147,20 @@
                     self.stream.write(line)
                 self.stream.flush()
 
-        if ret or not self._options.interactive or \
-            not os.path.exists(test.errpath):
-            return
+            # handle interactive prompt without releasing iolock
+            if self._options.interactive:
+                self.stream.write('Accept this change? [n] ')
+                answer = sys.stdin.readline().strip()
+                if answer.lower() in ('y', 'yes'):
+                    if test.name.endswith('.t'):
+                        rename(test.errpath, test.path)
+                    else:
+                        rename(test.errpath, '%s.out' % test.path)
+                    accepted = True
+
+        iolock.release()
+
+        return accepted
 
     def startTest(self, test):
         super(TestResult, self).startTest(test)

--- a/tests/test-run-tests.t	Wed Jun 18 15:26:07 2014 -0500
+++ b/tests/test-run-tests.t	Wed Jun 18 20:51:49 2014 -0500
@@ -160,9 +160,9 @@
      $ echo babar
   -  rataxes
   +  babar
-  
+  Accept this change? [n] 
   ERROR: test-failure.t output changed
-  Accept this change? [n] !.
+  !.
   Failed test-failure.t: output changed
   # Ran 2 tests, 0 skipped, 0 warned, 1 failed.
   python hash seed: * (glob)
@@ -182,8 +182,6 @@
      $ echo babar
   -  rataxes
   +  babar
-  
-  ERROR: test-failure.t output changed
   Accept this change? [n] ..
   # Ran 2 tests, 0 skipped, 0 warned, 0 failed.