| yuja |
| hg-reviewers |
Previously there is a race condition because things happen in this order:
The check and set should really happen in a same critical section.
This patch adds a lock to address the issue.
Run run-tests.py -l test-run-tests.t 20 times on gcc112 and the race
condition does not reproduce.
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
As a note (re your test plan): there's a --runs-per-test flag on run-tests for exactly this kind of "did I fix a flaky test" debugging. I often set it to 100. :)
| classes are instantiated automatically by scanning modules.) | classes are instantiated automatically by scanning modules.) | ||||
| """ | """ | ||||
| # Status code reserved for skipped tests (used by hghave). | # Status code reserved for skipped tests (used by hghave). | ||||
| SKIPPED_STATUS = 80 | SKIPPED_STATUS = 80 | ||||
| def __init__(self, path, outputdir, tmpdir, keeptmpdir=False, | def __init__(self, path, outputdir, tmpdir, keeptmpdir=False, | ||||
| debug=False, | debug=False, | ||||
| first=False, | |||||
| timeout=None, | timeout=None, | ||||
| startport=None, extraconfigopts=None, | startport=None, extraconfigopts=None, | ||||
| py3kwarnings=False, shell=None, hgcommand=None, | py3kwarnings=False, shell=None, hgcommand=None, | ||||
| slowtimeout=None, usechg=False, | slowtimeout=None, usechg=False, | ||||
| useipv6=False): | useipv6=False): | ||||
| """Create a test from parameters. | """Create a test from parameters. | ||||
| path is the full path to the file defining the test. | path is the full path to the file defining the test. | ||||
| self._testdir = os.path.dirname(path) | self._testdir = os.path.dirname(path) | ||||
| self._outputdir = outputdir | self._outputdir = outputdir | ||||
| self._tmpname = os.path.basename(path) | self._tmpname = os.path.basename(path) | ||||
| self.errpath = os.path.join(self._outputdir, b'%s.err' % self.bname) | self.errpath = os.path.join(self._outputdir, b'%s.err' % self.bname) | ||||
| self._threadtmp = tmpdir | self._threadtmp = tmpdir | ||||
| self._keeptmpdir = keeptmpdir | self._keeptmpdir = keeptmpdir | ||||
| self._debug = debug | self._debug = debug | ||||
| self._first = first | |||||
| self._timeout = timeout | self._timeout = timeout | ||||
| self._slowtimeout = slowtimeout | self._slowtimeout = slowtimeout | ||||
| self._startport = startport | self._startport = startport | ||||
| self._extraconfigopts = extraconfigopts or [] | self._extraconfigopts = extraconfigopts or [] | ||||
| self._py3kwarnings = py3kwarnings | self._py3kwarnings = py3kwarnings | ||||
| self._shell = _bytespath(shell) | self._shell = _bytespath(shell) | ||||
| self._hgcommand = hgcommand or b'hg' | self._hgcommand = hgcommand or b'hg' | ||||
| self._usechg = usechg | self._usechg = usechg | ||||
| if (ret != 0 or out != self._refout) and not self._skipped \ | if (ret != 0 or out != self._refout) and not self._skipped \ | ||||
| and not self._debug: | and not self._debug: | ||||
| f = open(self.errpath, 'wb') | f = open(self.errpath, 'wb') | ||||
| for line in out: | for line in out: | ||||
| f.write(line) | f.write(line) | ||||
| f.close() | f.close() | ||||
| # The result object handles diff calculation for us. | # The result object handles diff calculation for us. | ||||
| with firstlock: | |||||
| if self._result.addOutputMismatch(self, ret, out, self._refout): | if self._result.addOutputMismatch(self, ret, out, self._refout): | ||||
| # change was accepted, skip failing | # change was accepted, skip failing | ||||
| return | return | ||||
| if self._first: | |||||
| global firsterror | |||||
| firsterror = True | |||||
| if ret: | if ret: | ||||
| msg = 'output changed and ' + describe(ret) | msg = 'output changed and ' + describe(ret) | ||||
| else: | else: | ||||
| msg = 'output changed' | msg = 'output changed' | ||||
| self.fail(msg) | self.fail(msg) | ||||
| elif ret: | elif ret: | ||||
| def _escapef(m): | def _escapef(m): | ||||
| return TTest.ESCAPEMAP[m.group(0)] | return TTest.ESCAPEMAP[m.group(0)] | ||||
| @staticmethod | @staticmethod | ||||
| def _stringescape(s): | def _stringescape(s): | ||||
| return TTest.ESCAPESUB(TTest._escapef, s) | return TTest.ESCAPESUB(TTest._escapef, s) | ||||
| iolock = threading.RLock() | iolock = threading.RLock() | ||||
| firstlock = threading.RLock() | |||||
| firsterror = False | |||||
| class TestResult(unittest._TextTestResult): | class TestResult(unittest._TextTestResult): | ||||
| """Holds results when executing via unittest.""" | """Holds results when executing via unittest.""" | ||||
| # Don't worry too much about accessing the non-public _TextTestResult. | # Don't worry too much about accessing the non-public _TextTestResult. | ||||
| # It is relatively common in Python testing tools. | # It is relatively common in Python testing tools. | ||||
| def __init__(self, options, *args, **kwargs): | def __init__(self, options, *args, **kwargs): | ||||
| super(TestResult, self).__init__(*args, **kwargs) | super(TestResult, self).__init__(*args, **kwargs) | ||||
| if reason not in ('not retesting', "doesn't match keyword"): | if reason not in ('not retesting', "doesn't match keyword"): | ||||
| self.stream.write('i') | self.stream.write('i') | ||||
| else: | else: | ||||
| self.testsRun += 1 | self.testsRun += 1 | ||||
| self.stream.flush() | self.stream.flush() | ||||
| def addOutputMismatch(self, test, ret, got, expected): | def addOutputMismatch(self, test, ret, got, expected): | ||||
| """Record a mismatch in test output for a particular test.""" | """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 | # don't print, some other test case already failed and | ||||
| # printed, we're just stale and probably failed due to our | # printed, we're just stale and probably failed due to our | ||||
| # temp dir getting cleaned up. | # temp dir getting cleaned up. | ||||
| return | return | ||||
| accepted = False | accepted = False | ||||
| lines = [] | lines = [] | ||||
| tmpdir = os.path.join(self._hgtmp, b'child%d' % count) | tmpdir = os.path.join(self._hgtmp, b'child%d' % count) | ||||
| # extra keyword parameters. 'case' is used by .t tests | # extra keyword parameters. 'case' is used by .t tests | ||||
| kwds = dict((k, testdesc[k]) for k in ['case'] if k in testdesc) | kwds = dict((k, testdesc[k]) for k in ['case'] if k in testdesc) | ||||
| t = testcls(refpath, self._outputdir, tmpdir, | t = testcls(refpath, self._outputdir, tmpdir, | ||||
| keeptmpdir=self.options.keep_tmpdir, | keeptmpdir=self.options.keep_tmpdir, | ||||
| debug=self.options.debug, | debug=self.options.debug, | ||||
| first=self.options.first, | |||||
| timeout=self.options.timeout, | timeout=self.options.timeout, | ||||
| startport=self._getport(count), | startport=self._getport(count), | ||||
| extraconfigopts=self.options.extra_config_opt, | extraconfigopts=self.options.extra_config_opt, | ||||
| py3kwarnings=self.options.py3k_warnings, | py3kwarnings=self.options.py3k_warnings, | ||||
| shell=self.options.shell, | shell=self.options.shell, | ||||
| hgcommand=self._hgcommand, | hgcommand=self._hgcommand, | ||||
| usechg=bool(self.options.with_chg or self.options.chg), | usechg=bool(self.options.with_chg or self.options.chg), | ||||
| useipv6=useipv6, **kwds) | useipv6=useipv6, **kwds) | ||||