Commit 91ca7b8c authored by Brian Sheedy's avatar Brian Sheedy Committed by Commit Bot

Remove Telemetry Android minidump special casing

Enables the standard BrowserMinidumpTests on Android and removes the
Android-specific testGetStackTrace. Android now supports the standard
set of minidump-related methods, so it can use the same tests as other
platforms.

Also removes a special casing of Android in the GPU test code that was
also caused by Android not implementing the standard methods.

Bug: 1008075
Change-Id: Iaaa7f14ecee81be4ffa16c5db3418cbbe7de51f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1934842
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: Caleb Rouleau <crouleau@chromium.org>
Auto-Submit: Brian Sheedy <bsheedy@chromium.org>
Reviewed-by: default avatarCaleb Rouleau <crouleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718917}
parent 902a75dc
...@@ -6,7 +6,6 @@ import logging ...@@ -6,7 +6,6 @@ import logging
import re import re
import sys import sys
from telemetry.core import android_platform
from telemetry.testing import serially_executed_browser_test_case from telemetry.testing import serially_executed_browser_test_case
from telemetry.util import screenshot from telemetry.util import screenshot
from typ import json_results from typ import json_results
...@@ -164,7 +163,8 @@ class GpuIntegrationTest( ...@@ -164,7 +163,8 @@ class GpuIntegrationTest(
cls.StartBrowser() cls.StartBrowser()
def _RunGpuTest(self, url, test_name, *args): def _RunGpuTest(self, url, test_name, *args):
expected_results, should_retry_on_failure = self.GetExpectationsForTest()[:2] expected_results, should_retry_on_failure = (
self.GetExpectationsForTest()[:2])
try: try:
# TODO(nednguyen): For some reason the arguments are getting wrapped # TODO(nednguyen): For some reason the arguments are getting wrapped
# in another tuple sometimes (like in the WebGL extension tests). # in another tuple sometimes (like in the WebGL extension tests).
...@@ -199,14 +199,7 @@ class GpuIntegrationTest( ...@@ -199,14 +199,7 @@ class GpuIntegrationTest(
# stacks could slow down the tests' running time unacceptably. # stacks could slow down the tests' running time unacceptably.
# We also don't do this if the browser failed to startup. # We also don't do this if the browser failed to startup.
if self.browser is not None: if self.browser is not None:
# TODO(https://crbug.com/1008075): Remove this split once stack self.browser.LogSymbolizedUnsymbolizedMinidumps(logging.ERROR)
# symbolization is standardized across all platforms.
if isinstance(self.browser.platform,
android_platform.AndroidPlatform):
_, output = self.browser.GetStackTrace()
logging.error(output)
else:
self.browser.LogSymbolizedUnsymbolizedMinidumps(logging.ERROR)
# This failure might have been caused by a browser or renderer # This failure might have been caused by a browser or renderer
# crash, so restart the browser to make sure any state doesn't # crash, so restart the browser to make sure any state doesn't
# propagate to the next test iteration. # propagate to the next test iteration.
......
...@@ -6,7 +6,6 @@ from __future__ import print_function ...@@ -6,7 +6,6 @@ from __future__ import print_function
import logging import logging
import os import os
import re
import time import time
from telemetry.testing import tab_test_case from telemetry.testing import tab_test_case
...@@ -17,10 +16,9 @@ import py_utils ...@@ -17,10 +16,9 @@ import py_utils
class BrowserMinidumpTest(tab_test_case.TabTestCase): class BrowserMinidumpTest(tab_test_case.TabTestCase):
@decorators.Isolated @decorators.Isolated
# Android is currently hard coded to return None for minidump paths.
# Minidump symbolization doesn't work in ChromeOS local mode if the rootfs is # Minidump symbolization doesn't work in ChromeOS local mode if the rootfs is
# still read-only, so skip the test in that case. # still read-only, so skip the test in that case.
@decorators.Disabled('android', 'chromeos-local') @decorators.Disabled('chromeos-local')
def testSymbolizeMinidump(self): def testSymbolizeMinidump(self):
# Wait for the browser to restart fully before crashing # Wait for the browser to restart fully before crashing
self._LoadPageThenWait('var sam = "car";', 'sam') self._LoadPageThenWait('var sam = "car";', 'sam')
...@@ -57,51 +55,10 @@ class BrowserMinidumpTest(tab_test_case.TabTestCase): ...@@ -57,51 +55,10 @@ class BrowserMinidumpTest(tab_test_case.TabTestCase):
+ ''.join(all_unsymbolized_after_symbolize_paths)) + ''.join(all_unsymbolized_after_symbolize_paths))
self.assertTrue(len(all_unsymbolized_after_symbolize_paths) == 0) self.assertTrue(len(all_unsymbolized_after_symbolize_paths) == 0)
@decorators.Isolated
# The way Android handles crashes is through a different set of methods, so
# have an Android-specific test similar to testSymbolizeMinidump.
@decorators.Enabled('android')
def testGetStackTrace(self):
self._LoadPageThenWait('var sam = "car";', 'sam')
self._browser.tabs.New().Navigate('chrome://gpucrash', timeout=5)
_, output = self._browser.GetStackTrace()
# The output is a single string with multiple sections:
# 1. UI Dump
# 2. Logcat
# 3. Stack from Logcat
# 4. Tombstones
# 5. Crashpad stackwalk
# Each section is finished with 80 asterisks, so split based on that.
sections = output.split('*' * 80 + '\n')
# We will always get the UI dump and logcat sections. The logcat stack,
# tombstones, and Crashpad stack are dependent on the necessary tools being
# present, which they always should be. The Crashpad section actually having
# data is dependent on a Crashpad dump being found, but that should always
# be the case. So, expect 5 actual sections (6 total due to the way .split()
# works).
self.assertTrue(len(sections) == 6)
self.assertTrue(sections[2].startswith('Stack from Logcat'))
self.assertTrue(sections[3].startswith('Tombstones'))
# Since the crash is a simulated one from gl::Crash(), we expect that to
# show up in the symbolized stacks, but not the unsymbolized one.
crash_function = 'gl::Crash()'
self.assertFalse(crash_function in sections[1])
self.assertTrue(crash_function in sections[2])
self.assertTrue(crash_function in sections[3])
# Depending on symbol level, etc., the Crashpad stack trace might not
# have the actual crash function in it. So, accept either case as valid.
# Matches " 0 libchrome.so!<name omitted>", which is the case with certain
# GN args.
match = re.search(r'\n\s*0.*chrome\.so\!\<name omitted\>', sections[4])
self.assertTrue(crash_function in sections[4] or match is not None)
@decorators.Isolated @decorators.Isolated
# Minidump symbolization doesn't work in ChromeOS local mode if the rootfs is # Minidump symbolization doesn't work in ChromeOS local mode if the rootfs is
# still read-only, so skip the test in that case. # still read-only, so skip the test in that case.
@decorators.Disabled('android', 'chromeos-local') @decorators.Disabled('chromeos-local')
def testMultipleCrashMinidumps(self): def testMultipleCrashMinidumps(self):
# Wait for the browser to restart fully before crashing # Wait for the browser to restart fully before crashing
self._LoadPageThenWait('var cat = "dog";', 'cat') self._LoadPageThenWait('var cat = "dog";', 'cat')
......
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