changeset 32980:8dc62c97a665

run-tests: do not prompt changes (-i) if a race condition is detected

The race condition is like:

  1. run-tests.py reads test-a.t as reference output, content A
  2. run-tests.py runs the test (which could be content B, another race
     condition fixed by the next patch, but assume it's content A here)
  3. something changes test-a.t to content C
  4. run-tests.py compares test output (content D) with content A
  5. with "-i", run-tests.py prompts diff(A, D), while the file has content
     C instead of A at this time

This patch detects the above case and tell the user to rerun the test if
they want to apply test changes.

--- a/tests/run-tests.py	Tue Jun 20 23:22:38 2017 -0700
+++ b/tests/run-tests.py	Wed Jun 21 01:05:20 2017 -0700
@@ -633,16 +633,19 @@
         self._testtmp = None
         self._chgsockdir = None
 
+        self._refout = self.readrefout()
+
+    def readrefout(self):
+        """read reference output"""
         # If we're not in --debug mode and reference output file exists,
         # check test output against it.
-        if debug:
-            self._refout = None # to match "out is None"
+        if self._debug:
+            return None # to match "out is None"
         elif os.path.exists(self.refpath):
-            f = open(self.refpath, 'rb')
-            self._refout = f.read().splitlines(True)
-            f.close()
+            with open(self.refpath, 'rb') as f:
+                return f.read().splitlines(True)
         else:
-            self._refout = []
+            return []
 
     # needed to get base class __repr__ running
     @property
@@ -1588,14 +1591,19 @@
 
             # 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
+                if test.readrefout() != expected:
+                    self.stream.write(
+                        'Reference output has changed (run again to prompt '
+                        'changes)')
+                else:
+                    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
             if not accepted:
                 self.faildata[test.name] = b''.join(lines)
 

--- a/tests/test-run-tests.t	Tue Jun 20 23:22:38 2017 -0700
+++ b/tests/test-run-tests.t	Wed Jun 21 01:05:20 2017 -0700
@@ -641,6 +641,33 @@
     $ echo 'saved backup bundle to $TESTTMP/foo.hg'
     saved backup bundle to $TESTTMP/*.hg (glob)<
 
+Race condition - test file was modified when test is running
+
+  $ TESTRACEDIR=`pwd`
+  $ export TESTRACEDIR
+  $ cat > test-race.t <<EOF
+  >   $ echo 1
+  >   $ echo "# a new line" >> $TESTRACEDIR/test-race.t
+  > EOF
+
+  $ rt -i test-race.t
+  
+  --- $TESTTMP/test-race.t
+  +++ $TESTTMP/test-race.t.err
+  @@ -1,2 +1,3 @@
+     $ echo 1
+  +  1
+     $ echo "# a new line" >> $TESTTMP/test-race.t
+  Reference output has changed (run again to prompt changes)
+  ERROR: test-race.t output changed
+  !
+  Failed test-race.t: output changed
+  # Ran 1 tests, 0 skipped, 1 failed.
+  python hash seed: * (glob)
+  [1]
+
+  $ rm test-race.t
+
 (reinstall)
   $ mv backup test-failure.t