Skip to content

Commit

Permalink
Bug 1833759 - Catch xpcshell test crash before setting up the crash r…
Browse files Browse the repository at this point in the history
…eporter. r=gbrown

Differential Revision: https://phabricator.services.mozilla.com/D179635
  • Loading branch information
arai-a committed Jul 7, 2023
1 parent 856b4c1 commit e3a18b3
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 6 deletions.
8 changes: 8 additions & 0 deletions testing/xpcshell/head.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,14 @@ try {
);
crashReporter.UpdateCrashEventsDir();
crashReporter.minidumpPath = do_get_minidumpdir();

// Tell the test harness that the crash reporter is set up, and any crash
// after this point is supposed to be caught by the crash reporter.
//
// Due to the limitation on the remote xpcshell test, the process return
// code does not represent the process crash. Any crash before this point
// can be caught by the absence of this log.
_testLogger.logData("crash_reporter_init");
}
} catch (e) {}

Expand Down
62 changes: 56 additions & 6 deletions testing/xpcshell/runxpcshelltests.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@

EXPECTED_LOG_ACTIONS = set(
[
"crash_reporter_init",
"test_status",
"log",
]
Expand Down Expand Up @@ -156,7 +157,13 @@ def markGotSIGINT(signum, stackFrame):

class XPCShellTestThread(Thread):
def __init__(
self, test_object, retry=True, verbose=False, usingTSan=False, **kwargs
self,
test_object,
retry=True,
verbose=False,
usingTSan=False,
usingCrashReporter=False,
**kwargs
):
Thread.__init__(self)
self.daemon = True
Expand All @@ -165,6 +172,7 @@ def __init__(
self.retry = retry
self.verbose = verbose
self.usingTSan = usingTSan
self.usingCrashReporter = usingCrashReporter

self.appPath = kwargs.get("appPath")
self.xrePath = kwargs.get("xrePath")
Expand Down Expand Up @@ -216,6 +224,7 @@ def __init__(
# Context for output processing
self.output_lines = []
self.has_failure_output = False
self.saw_crash_reporter_init = False
self.saw_proc_start = False
self.saw_proc_end = False
self.command = None
Expand Down Expand Up @@ -721,6 +730,10 @@ def process_line(self, line_string):
self.report_message(line_string)
return

if line_object["action"] == "crash_reporter_init":
self.saw_crash_reporter_init = True
return

action = line_object["action"]

self.has_failure_output = (
Expand Down Expand Up @@ -891,7 +904,32 @@ def run_test(self):
return_code_ok = return_code == 0 or (
self.usingTSan and return_code == TSAN_EXIT_CODE_WITH_RACES
)
passed = (not self.has_failure_output) and return_code_ok

# Due to the limitation on the remote xpcshell test, the process
# return code does not represent the process crash.
# If crash_reporter_init log has not been seen and the return code
# is 0, it means the process crashed before setting up the crash
# reporter.
#
# NOTE: Crash reporter is not enabled on some configuration, such
# as ASAN and TSAN. Those configuration shouldn't be using
# remote xpcshell test, and the crash should be caught by
# the process return code.
# NOTE: self.saw_crash_reporter_init is False also when adb failed
# to launch process, and in that case the return code is
# not 0.
# (see launchProcess in remotexpcshelltests.py)
ended_before_crash_reporter_init = (
return_code_ok
and self.usingCrashReporter
and not self.saw_crash_reporter_init
)

passed = (
(not self.has_failure_output)
and not ended_before_crash_reporter_init
and return_code_ok
)

status = "PASS" if passed else "FAIL"
expected = "PASS" if expect_pass else "FAIL"
Expand All @@ -900,8 +938,15 @@ def run_test(self):
if self.timedout:
return

if status != expected:
if self.retry:
if status != expected or ended_before_crash_reporter_init:
if ended_before_crash_reporter_init:
self.log.test_end(
name,
"CRASH",
expected=expected,
message="Test ended before setting up the crash reporter",
)
elif self.retry:
self.log.test_end(
name,
status,
Expand All @@ -912,8 +957,8 @@ def run_test(self):
if self.verboseIfFails and not self.verbose:
self.log_full_output()
return

self.log.test_end(name, status, expected=expected, message=message)
else:
self.log.test_end(name, status, expected=expected, message=message)
self.log_full_output()

self.failCount += 1
Expand Down Expand Up @@ -1871,6 +1916,10 @@ def runTests(self, options, testClass=XPCShellTestThread, mobileArgs=None):
# that has an effect on interpretation of the process return value.
usingTSan = "tsan" in self.mozInfo and self.mozInfo["tsan"]

usingCrashReporter = (
"crashreporter" in self.mozInfo and self.mozInfo["crashreporter"]
)

# create a queue of all tests that will run
tests_queue = deque()
# also a list for the tests that need to be run sequentially
Expand Down Expand Up @@ -1899,6 +1948,7 @@ def runTests(self, options, testClass=XPCShellTestThread, mobileArgs=None):
test_object,
verbose=self.verbose or test_object.get("verbose") == "true",
usingTSan=usingTSan,
usingCrashReporter=usingCrashReporter,
mobileArgs=mobileArgs,
**kwargs,
)
Expand Down

0 comments on commit e3a18b3

Please sign in to comment.