Commit 3c61465a authored by Jonathan Ross's avatar Jonathan Ross Committed by Commit Bot

Revert "Remove expected leftover minidumps"

This reverts commit b0fc3d10.

Reason for revert: Pixel_Video_Context_Loss_VP9 has begun failing on all Android platforms

Original change's description:
> Remove expected leftover minidumps
> 
> Removes expected leftover minidumps from several tests that
> intentionally crash a Chrome process. This is so that these
> intentional crashes are not caught as failures once pre/post test
> minidump checking is enabled with crrev.com/c/2090590.
> 
> Bug: 1056235
> Change-Id: Iad026563b3a7acfc6e2078ffec36a000ed4b2dec
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2080863
> Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
> Reviewed-by: Kenneth Russell <kbr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#748433}

TBR=kbr@chromium.org,bsheedy@chromium.org

Change-Id: Ie3903e7c61b4731ffa6ede93866d4105e1c0aec9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1056235
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2096775Reviewed-by: default avatarJonathan Ross <jonross@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748667}
parent 782b48e7
......@@ -172,13 +172,9 @@ class GpuIntegrationTest(
# generator?
if len(args) == 1 and isinstance(args[0], tuple):
args = args[0]
expected_crashes = self.GetExpectedCrashes(args)
self.RunActualGpuTest(url, *args)
except Exception:
if ResultType.Failure in expected_results or should_retry_on_failure:
# We don't check the return value here since we'll be raising the
# caught exception already.
self._ClearExpectedCrashes(expected_crashes)
if should_retry_on_failure:
logging.exception('Exception while running flaky test %s', test_name)
# For robustness, shut down the browser and restart it
......@@ -210,16 +206,9 @@ class GpuIntegrationTest(
self._RestartBrowser('unexpected test failure')
raise
else:
# We always want to clear any expected crashes, but we don't bother
# failing the test if it's expected to fail.
actual_and_expected_crashes_match = self._ClearExpectedCrashes(
expected_crashes)
if ResultType.Failure in expected_results:
logging.warning(
'%s was expected to fail, but passed.\n', test_name)
else:
if not actual_and_expected_crashes_match:
raise RuntimeError('Actual and expected crashes did not match')
def _IsIntel(self, vendor_id):
return vendor_id == 0x8086
......@@ -249,59 +238,6 @@ class GpuIntegrationTest(
return True
return False
def _ClearExpectedCrashes(self, expected_crashes):
"""Clears any expected crash minidumps so they're not caught later.
Args:
expected_crashes: A dictionary mapping crash types as strings to the
number of expected crashes of that type.
Returns:
True if the actual number of crashes matched the expected number,
otherwise False.
"""
# We can't get crashes if we don't have a browser.
if self.browser is None:
return True
# TODO(crbug.com/1006331): Properly match type once we have a way of
# checking the crashed process type without symbolizing the minidump.
total_expected_crashes = sum(expected_crashes.values())
# The Telemetry-wide cleanup will handle any remaining minidumps, so early
# return here since we don't expect any, which saves us a bit of work.
if total_expected_crashes == 0:
return True
unsymbolized_minidumps = self.browser.GetAllUnsymbolizedMinidumpPaths()
total_unsymbolized_minidumps = len(unsymbolized_minidumps)
if total_expected_crashes == total_unsymbolized_minidumps:
for path in unsymbolized_minidumps:
self.browser.IgnoreMinidump(path)
return True
logging.error(
'Found %d unsymbolized minidumps when we expected %d. Expected '
'crash breakdown: %s', total_unsymbolized_minidumps,
total_expected_crashes, expected_crashes)
return False
def GetExpectedCrashes(self, args):
"""Returns which crashes, per process type, to expect for the current test.
Should be overridden by child classes to actually return valid data if
available.
Args:
args: The list passed to _RunGpuTest()
Returns:
A dictionary mapping crash types as strings to the number of expected
crashes of that type. Examples include 'gpu' for the GPU process,
'renderer' for the renderer process, and 'browser' for the browser
process.
"""
del args
return {}
@classmethod
def GenerateGpuTests(cls, options):
"""Subclasses must implement this to yield (test_name, url, args)
......
......@@ -116,21 +116,6 @@ class PixelIntegrationTest(
# test makes assertions about the active GPU.
time.sleep(4)
def GetExpectedCrashes(self, args):
"""Returns which crashes, per process type, to expect for the current test.
Args:
args: The list passed to _RunGpuTest()
Returns:
A dictionary mapping crash types as strings to the number of expected
crashes of that type. Examples include 'gpu' for the GPU process,
'renderer' for the renderer process, and 'browser' for the browser
process.
"""
# args[0] is the PixelTestPage for the current test.
return args[0].expected_per_process_crashes
def _RunSkiaGoldBasedPixelTest(self, do_page_action, page):
"""Captures and compares a test image using Skia Gold.
......
......@@ -30,9 +30,6 @@ _FOUR_COLOR_VIDEO_240x135_EXPECTED_COLORS = [
]
CRASH_TYPE_GPU = 'gpu'
class PixelTestPage(object):
"""A wrapper class mimicking the functionality of the PixelTestsStorySet
from the old-style GPU tests.
......@@ -40,8 +37,7 @@ class PixelTestPage(object):
def __init__(self, url, name, test_rect, tolerance=2, browser_args=None,
expected_colors=None, gpu_process_disabled=False,
optional_action=None, restart_browser_after_test=False,
other_args=None, grace_period_end=None,
expected_per_process_crashes=None):
other_args=None, grace_period_end=None):
super(PixelTestPage, self).__init__()
self.url = url
self.name = name
......@@ -79,10 +75,6 @@ class PixelTestPage(object):
# be triaged without turning the bots red.
# This should be a datetime.date object.
self.grace_period_end = grace_period_end
# This lets the test runner know that one or more crashes are expected as
# part of the test. Should be a map of process type (str) to expected number
# of crashes (int).
self.expected_per_process_crashes = expected_per_process_crashes or {}
def CopyWithNewBrowserArgsAndSuffix(self, browser_args, suffix):
return PixelTestPage(
......@@ -476,10 +468,7 @@ class PixelTestPages(object):
base_name + '_Video_Context_Loss_MP4',
test_rect=[0, 0, 240, 135],
tolerance=tolerance,
expected_colors=_FOUR_COLOR_VIDEO_240x135_EXPECTED_COLORS,
expected_per_process_crashes={
CRASH_TYPE_GPU: 1,
}),
expected_colors=_FOUR_COLOR_VIDEO_240x135_EXPECTED_COLORS),
# The VP9 test clip is primarily software decoded on bots.
PixelTestPage(
......@@ -488,10 +477,7 @@ class PixelTestPages(object):
base_name + '_Video_Context_Loss_VP9',
test_rect=[0, 0, 240, 135],
tolerance=tolerance_vp9,
expected_colors=_FOUR_COLOR_VIDEO_240x135_EXPECTED_COLORS,
expected_per_process_crashes={
CRASH_TYPE_GPU: 1,
}),
expected_colors=_FOUR_COLOR_VIDEO_240x135_EXPECTED_COLORS),
PixelTestPage(
'pixel_video_backdrop_filter.html',
......
......@@ -147,10 +147,6 @@ class BrowserMinidumpTest(tab_test_case.TabTestCase):
self.assertEquals(after_symbolize_all_unsymbolized_paths,
[first_crash_path])
# Explicitly ignore the remaining minidump so that it isn't detected during
# teardown by the test runner.
self._browser.IgnoreMinidump(first_crash_path)
def _LoadPageThenWait(self, script, value):
# We are occasionally seeing these tests fail on the first load and
# call to GetMostRecentMinidumpPath, where the directory is coming up empty.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment