Remove potential hang+timeout from _nocompile tests
If output buffers got full while running the tests, the
script would hang, polling the process forever and it would never
return a return_code from proc.poll().
This might have caused some random timouts but if so, it is
unclear what 64 KB output filled up the buffers, unless the buffers
were smaller than local testing would indicate.
This stores stdout and stderr in temporary files and reads those
files back into memory after the process has finished.
Bug: 882852
Change-Id: I4e6ace2bb783fadcde3f43643ccad03c58ef519a
Reviewed-on: https://chromium-review.googlesource.com/c/1425736
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625603}diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn
index 5959251..26813f7 100644
--- a/content/test/BUILD.gn
+++ b/content/test/BUILD.gn
@@ -2202,8 +2202,7 @@
}
}
-# Disabled on jumbo builds because of timeouts (crbug.com/882852).
-if (enable_nocompile_tests && !use_jumbo_build) {
+if (enable_nocompile_tests) {
nocompile_test("content_nocompile_tests") {
sources = [
"../public/browser/browser_task_traits_unittest.nc",
diff --git a/tools/nocompile_driver.py b/tools/nocompile_driver.py
index 33bb4efa..fedb351 100755
--- a/tools/nocompile_driver.py
+++ b/tools/nocompile_driver.py
@@ -20,6 +20,7 @@
import select
import subprocess
import sys
+import tempfile
import time
@@ -89,7 +90,7 @@
assert type(sourcefile_path) is str
assert type(cflags) is list
for flag in cflags:
- assert(type(flag) is str)
+ assert type(flag) is str
assert type(resultfile_path) is str
@@ -180,11 +181,12 @@
return test_configs
-def StartTest(compiler, sourcefile_path, cflags, config):
+def StartTest(compiler, sourcefile_path, tempfile_dir, cflags, config):
"""Start one negative compile test.
Args:
sourcefile_path: The path to the source file.
+ tempfile_dir: A directory to store temporary data from tests.
cflags: An array of strings with all the CFLAGS to give to gcc.
config: A dictionary describing the test. See ExtractTestConfigs
for a description of the config format.
@@ -221,12 +223,15 @@
cmdline.append('-D%s' % name)
cmdline.extend(['-o', '/dev/null', '-c', '-x', 'c++',
sourcefile_path])
+ test_stdout = tempfile.TemporaryFile(dir=tempfile_dir)
+ test_stderr = tempfile.TemporaryFile(dir=tempfile_dir)
- process = subprocess.Popen(cmdline, stdout=subprocess.PIPE,
- stderr=subprocess.PIPE)
+ process = subprocess.Popen(cmdline, stdout=test_stdout, stderr=test_stderr)
now = time.time()
return {'proc': process,
'cmdline': ' '.join(cmdline),
+ 'stdout': test_stdout,
+ 'stderr': test_stderr,
'name': name,
'suite_name': config['suite_name'],
'terminate_timeout': now + NCTEST_TERMINATE_TIMEOUT_SEC,
@@ -301,6 +306,18 @@
suite_name, timings['started'], timings['results_processed'], total_secs,
extract_secs, compile_secs, process_secs))
+def ExtractTestOutputAndCleanup(test):
+ """Test output is in temp files. Read those and delete them.
+ Returns: A tuple (stderr, stdout).
+ """
+ outputs = [None, None]
+ for i, stream_name in ((0, "stdout"), (1, "stderr")):
+ stream = test[stream_name]
+ stream.seek(0)
+ outputs[i] = stream.read()
+ stream.close()
+
+ return outputs
def ProcessTestResult(resultfile, resultlog, test):
"""Interprets and logs the result of a test started by StartTest()
@@ -310,11 +327,9 @@
resultlog: File object for the log file.
test: The dictionary from StartTest() to process.
"""
- # Snap a copy of stdout and stderr into the test dictionary immediately
- # cause we can only call this once on the Popen object, and lots of stuff
- # below will want access to it.
proc = test['proc']
- (stdout, stderr) = proc.communicate()
+ proc.wait()
+ (stdout, stderr) = ExtractTestOutputAndCleanup(test)
if test['aborted_at'] != 0:
FailTest(resultfile, test, "Compile timed out. Started %f ended %f." %
@@ -369,10 +384,12 @@
# If we don't make progress for too long, assume the code is just dead.
assert busy_loop_timeout > time.time()
- # Select on the output pipes.
+ # Select on the output files to block until we have something to
+ # do. We ignore the return value from select and just poll all
+ # processes.
read_set = []
for test in executing_tests.values():
- read_set.extend([test['proc'].stderr, test['proc'].stdout])
+ read_set.extend([test['stdout'], test['stderr']])
select.select(read_set, [], read_set, NCTEST_TERMINATE_TIMEOUT_SEC)
# Now attempt to process results.
@@ -389,6 +406,12 @@
proc.kill()
test['aborted_at'] = now
+ if len(finished_tests) == 0:
+ # We had output from some process but no process had
+ # finished. To avoid busy looping while waiting for a process to
+ # finish, insert a small 100 ms delay here.
+ time.sleep(0.1)
+
for test in finished_tests:
del executing_tests[test['name']]
return finished_tests
@@ -438,6 +461,7 @@
test = StartTest(
compiler,
sourcefile_path,
+ os.path.dirname(resultfile_path),
cflags,
{ 'name': 'NCTEST_SANITY',
'suite_name': suite_name,
@@ -455,7 +479,8 @@
if config['name'].startswith('DISABLED_'):
PassTest(resultfile, resultlog, config)
else:
- test = StartTest(compiler, sourcefile_path, cflags, config)
+ test = StartTest(compiler, sourcefile_path,
+ os.path.dirname(resultfile_path), cflags, config)
assert test['name'] not in executing_tests
executing_tests[test['name']] = test
@@ -468,8 +493,9 @@
finished_tests = sorted(finished_tests, key=lambda test: test['name'])
for test in finished_tests:
if test['name'] == 'NCTEST_SANITY':
- stdout, stderr = test['proc'].communicate()
- return_code = test['proc'].poll()
+ test['proc'].wait()
+ (stdout, stderr) = ExtractTestOutputAndCleanup(test)
+ return_code = test['proc'].returncode
if return_code != 0:
sys.stdout.write(stdout)
sys.stderr.write(stderr)
