D1830: test-run-tests: stabilize the test (issue5735)

[quark (Jun Wu)]

quark created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Previously there is a race condition because things happen in this order:
  
  1. Check shouldStop
  2. If shouldStop is false, print the diff
  3. Call fail() -> set shouldStop
  
  The check and set should really happen in a same critical section.
  
  This patch adds a lock to address the issue.

TEST PLAN
  Run `run-tests.py -l test-run-tests.t` 20 times on gcc112 and the race
  condition does not reproduce.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1830

AFFECTED FILES
  tests/run-tests.py

CHANGE DETAILS

diff --git a/tests/run-tests.py b/tests/run-tests.py
--- a/tests/run-tests.py
+++ b/tests/run-tests.py
@@ -670,6 +670,7 @@
 
     def __init__(self, path, outputdir, tmpdir, keeptmpdir=False,
                  debug=False,
+                 first=False,
                  timeout=None,
                  startport=None, extraconfigopts=None,
                  py3kwarnings=False, shell=None, hgcommand=None,
@@ -722,6 +723,7 @@
         self._threadtmp = tmpdir
         self._keeptmpdir = keeptmpdir
         self._debug = debug
+        self._first = first
         self._timeout = timeout
         self._slowtimeout = slowtimeout
         self._startport = startport
@@ -906,9 +908,13 @@
                         f.write(line)
 
             # The result object handles diff calculation for us.
-            if self._result.addOutputMismatch(self, ret, out, self._refout):
-                # change was accepted, skip failing
-                return
+            with firstlock:
+                if self._result.addOutputMismatch(self, ret, out, self._refout):
+                    # change was accepted, skip failing
+                    return
+                if self._first:
+                    global firsterror
+                    firsterror = True
 
             if ret:
                 msg = 'output changed and ' + describe(ret)
@@ -1637,6 +1643,8 @@
         return TTest.ESCAPESUB(TTest._escapef, s)
 
 iolock = threading.RLock()
+firstlock = threading.RLock()
+firsterror = False
 
 class TestResult(unittest._TextTestResult):
     """Holds results when executing via unittest."""
@@ -1722,7 +1730,7 @@
 
     def addOutputMismatch(self, test, ret, got, expected):
         """Record a mismatch in test output for a particular test."""
-        if self.shouldStop:
+        if self.shouldStop or firsterror:
             # don't print, some other test case already failed and
             # printed, we're just stale and probably failed due to our
             # temp dir getting cleaned up.
@@ -2700,6 +2708,7 @@
         t = testcls(refpath, self._outputdir, tmpdir,
                     keeptmpdir=self.options.keep_tmpdir,
                     debug=self.options.debug,
+                    first=self.options.first,
                     timeout=self.options.timeout,
                     startport=self._getport(count),
                     extraconfigopts=self.options.extra_config_opt,



To: quark, #hg-reviewers
Cc: mercurial-devel