Skip to content

Commit 0e7df1e

Browse files
committed
Remove a bunch of redundant code from parallel_testsuite.py
All this infrastructure with the BufferedTestBase was added to work around that fact that we were previously using using a fake TestResult class so that is could be serialized. However in #25737 I converted to using TestResult as the superclass of BufferedParallelTestResult. There were only two members that needed to me removed to make it work. With that change landed we can now remove a log of the other code here.
1 parent db8daf9 commit 0e7df1e

File tree

1 file changed

+39
-135
lines changed

1 file changed

+39
-135
lines changed

test/parallel_testsuite.py

Lines changed: 39 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ def combine_results(self, result, buffered_results):
263263
# Extract only the test short names
264264
flaky_tests = [x.split('.')[-1] for x in flaky_tests]
265265

266-
# The next updateResult loop will print a *lot* of lines really fast. This
266+
# The next integrateResult loop will print a *lot* of lines really fast. This
267267
# will cause a Python exception being thrown when attempting to print to
268268
# stderr, if stderr is in nonblocking mode, like it is on Buildbot CI:
269269
# See https://github.com/buildbot/buildbot/issues/8659
@@ -272,11 +272,9 @@ def combine_results(self, result, buffered_results):
272272
os.set_blocking(sys.stderr.fileno(), True)
273273

274274
for r in results:
275-
# Merge information of flaky tests into the test result
276-
if r.test_result == 'success' and r.test_short_name() in flaky_tests:
277-
r.test_result = 'warnings'
278-
# And integrate the test result to the global test object
279-
r.updateResult(result)
275+
# Integrate the test result to the global test restult oject
276+
r.integrateResult(result)
277+
r.log_test_run_for_visualization(flaky_tests)
280278

281279
# Generate the parallel test run visualization
282280
if EMTEST_VISUALIZE:
@@ -293,14 +291,10 @@ class BufferedParallelTestResult(unittest.TestResult):
293291
"""
294292
def __init__(self):
295293
super().__init__()
296-
self.buffered_result = None
297294
self.test_duration = 0
298295
self.test_result = 'errored'
299296
self.test_name = ''
300-
301-
@property
302-
def test(self):
303-
return self.buffered_result.test
297+
self.test = None
304298

305299
def test_short_name(self):
306300
# Given a test name e.g. "test_atomic_cxx (test_core.core0.test_atomic_cxx)"
@@ -310,179 +304,89 @@ def test_short_name(self):
310304
def addDuration(self, test, elapsed):
311305
self.test_duration = elapsed
312306

313-
def updateResult(self, result):
314-
result.startTest(self.test)
315-
self.buffered_result.updateResult(result)
316-
result.stopTest(self.test)
317-
result.core_time += self.test_duration
318-
self.log_test_run_for_visualization()
307+
def integrateResult(self, overall_results):
308+
"""This method get called on the main thread once the buffered result
309+
is received. It add the buffered result to the overall result."""
310+
# The exception info objects that we are adding here have already
311+
# been turned into strings so make _exc_info_to_string into a no-op.
312+
overall_results._exc_info_to_string = lambda x, _y: x
313+
# No need to worry about stdout/stderr buffering since are a not
314+
# actually running the test here, only setting the results.
315+
overall_results.buffer = False
316+
overall_results.startTest(self.test)
317+
if self.test_result == 'success':
318+
overall_results.addSuccess(self.test)
319+
elif self.test_result == 'failed':
320+
overall_results.addFailure(*self.failures[0])
321+
elif self.test_result == 'errored':
322+
overall_results.addError(*self.errors[0])
323+
elif self.test_result == 'skipped':
324+
overall_results.addSkip(*self.skipped[0])
325+
elif self.test_result == 'unexpected success':
326+
overall_results.addUnexpectedSuccess(*self.unexpectedSuccesses[0])
327+
elif self.test_result == 'expected failure':
328+
overall_results.addExpectedFailure(*self.expectedFailures[0])
329+
else:
330+
assert False, f'unhandled test result {self.test_result}'
331+
overall_results.stopTest(self.test)
332+
overall_results.core_time += self.test_duration
319333

320-
def log_test_run_for_visualization(self):
334+
def log_test_run_for_visualization(self, flaky_tests):
321335
if EMTEST_VISUALIZE and (self.test_result != 'skipped' or self.test_duration > 0.2):
336+
test_result = self.test_result
337+
if test_result == 'success' and self.test_short_name() in flaky_tests:
338+
test_result = 'warnings'
322339
profiler_logs_path = os.path.join(tempfile.gettempdir(), 'emscripten_toolchain_profiler_logs')
323340
os.makedirs(profiler_logs_path, exist_ok=True)
324341
profiler_log_file = os.path.join(profiler_logs_path, 'toolchain_profiler.pid_0.json')
325-
colors = {
342+
color = {
326343
'success': '#80ff80',
327344
'warnings': '#ffb040',
328345
'skipped': '#a0a0a0',
329346
'expected failure': '#ff8080',
330347
'unexpected success': '#ff8080',
331348
'failed': '#ff8080',
332349
'errored': '#ff8080',
333-
}
350+
}[test_result]
334351
# Write profiling entries for emprofile.py tool to visualize. This needs a unique identifier for each
335352
# block, so generate one on the fly.
336353
dummy_test_task_counter = os.path.getsize(profiler_log_file) if os.path.isfile(profiler_log_file) else 0
337354
# Remove the redundant 'test_' prefix from each test, since character space is at a premium in the visualized graph.
338355
test_name = utils.removeprefix(self.test_short_name(), 'test_')
339356
with open(profiler_log_file, 'a') as prof:
340-
prof.write(f',\n{{"pid":{dummy_test_task_counter},"op":"start","time":{self.start_time},"cmdLine":["{test_name}"],"color":"{colors[self.test_result]}"}}')
357+
prof.write(f',\n{{"pid":{dummy_test_task_counter},"op":"start","time":{self.start_time},"cmdLine":["{test_name}"],"color":"{color}"}}')
341358
prof.write(f',\n{{"pid":{dummy_test_task_counter},"op":"exit","time":{self.start_time + self.test_duration},"returncode":0}}')
342359

343360
def startTest(self, test):
344361
super().startTest(test)
362+
self.test = test
345363
self.test_name = str(test)
346364

347-
def stopTest(self, test):
348-
super().stopTest(test)
349-
# TODO(sbc): figure out a way to display this duration information again when
350-
# these results get passed back to the TextTestRunner/TextTestResult.
351-
self.buffered_result.duration = self.test_duration
352-
353365
def addSuccess(self, test):
354366
super().addSuccess(test)
355-
self.buffered_result = BufferedTestSuccess(test)
356367
self.test_result = 'success'
357368

358369
def addExpectedFailure(self, test, err):
359370
super().addExpectedFailure(test, err)
360-
self.buffered_result = BufferedTestExpectedFailure(test, err)
361371
self.test_result = 'expected failure'
362372

363373
def addUnexpectedSuccess(self, test):
364374
super().addUnexpectedSuccess(test)
365-
self.buffered_result = BufferedTestUnexpectedSuccess(test)
366375
self.test_result = 'unexpected success'
367376

368377
def addSkip(self, test, reason):
369378
super().addSkip(test, reason)
370-
self.buffered_result = BufferedTestSkip(test, reason)
371379
self.test_result = 'skipped'
372380

373381
def addFailure(self, test, err):
374382
super().addFailure(test, err)
375-
self.buffered_result = BufferedTestFailure(test, err)
376383
self.test_result = 'failed'
377384

378385
def addError(self, test, err):
379386
super().addError(test, err)
380-
self.buffered_result = BufferedTestError(test, err)
381387
self.test_result = 'errored'
382388

383389

384-
class BufferedTestBase:
385-
"""Abstract class that holds test result data, split by type of result."""
386-
def __init__(self, test, err=None):
387-
self.test = test
388-
if err:
389-
exctype, value, tb = err
390-
self.error = exctype, value, FakeTraceback(tb)
391-
392-
def updateResult(self, result):
393-
assert False, 'Base class should not be used directly'
394-
395-
396-
class BufferedTestSuccess(BufferedTestBase):
397-
def updateResult(self, result):
398-
result.addSuccess(self.test)
399-
400-
401-
class BufferedTestSkip(BufferedTestBase):
402-
def __init__(self, test, reason):
403-
self.test = test
404-
self.reason = reason
405-
406-
def updateResult(self, result):
407-
result.addSkip(self.test, self.reason)
408-
409-
410-
def fixup_fake_exception(fake_exc):
411-
ex = fake_exc[2]
412-
if ex.tb_frame.f_code.positions is None:
413-
return
414-
while ex is not None:
415-
# .co_positions is supposed to be a function that returns an enumerable
416-
# to the list of code positions. Create a function object wrapper around
417-
# the data
418-
def make_wrapper(rtn):
419-
return lambda: rtn
420-
ex.tb_frame.f_code.co_positions = make_wrapper(ex.tb_frame.f_code.positions)
421-
ex = ex.tb_next
422-
423-
424-
class BufferedTestFailure(BufferedTestBase):
425-
def updateResult(self, result):
426-
fixup_fake_exception(self.error)
427-
result.addFailure(self.test, self.error)
428-
429-
430-
class BufferedTestExpectedFailure(BufferedTestBase):
431-
def updateResult(self, result):
432-
fixup_fake_exception(self.error)
433-
result.addExpectedFailure(self.test, self.error)
434-
435-
436-
class BufferedTestError(BufferedTestBase):
437-
def updateResult(self, result):
438-
fixup_fake_exception(self.error)
439-
result.addError(self.test, self.error)
440-
441-
442-
class BufferedTestUnexpectedSuccess(BufferedTestBase):
443-
def updateResult(self, result):
444-
fixup_fake_exception(self.error)
445-
result.addUnexpectedSuccess(self.test)
446-
447-
448-
class FakeTraceback:
449-
"""A fake version of a traceback object that is picklable across processes.
450-
451-
Python's traceback objects contain hidden stack information that isn't able
452-
to be pickled. Further, traceback objects aren't constructable from Python,
453-
so we need a dummy object that fulfills its interface.
454-
455-
The fields we expose are exactly those which are used by
456-
unittest.TextTestResult to show a text representation of a traceback. Any
457-
other use is not intended.
458-
"""
459-
460-
def __init__(self, tb):
461-
self.tb_frame = FakeFrame(tb.tb_frame)
462-
self.tb_lineno = tb.tb_lineno
463-
self.tb_next = FakeTraceback(tb.tb_next) if tb.tb_next is not None else None
464-
self.tb_lasti = tb.tb_lasti
465-
466-
467-
class FakeFrame:
468-
def __init__(self, f):
469-
self.f_code = FakeCode(f.f_code)
470-
# f.f_globals is not picklable, not used in stack traces, and needs to be iterable
471-
self.f_globals = []
472-
473-
474-
class FakeCode:
475-
def __init__(self, co):
476-
self.co_filename = co.co_filename
477-
self.co_name = co.co_name
478-
# co.co_positions() returns an iterator. Flatten it to a list so that it can
479-
# be pickled to the parent process
480-
if hasattr(co, 'co_positions'):
481-
self.positions = list(co.co_positions())
482-
else:
483-
self.positions = None
484-
485-
486390
def num_cores():
487391
if NUM_CORES:
488392
return int(NUM_CORES)

0 commit comments

Comments
 (0)