Default to updating test status if there's a crash dump

Add a pair of new command line arguments
`--[no-]update-status-on-crash` which control whether to override the
status when a crash dump file is found. Previously we only did this
for specific test types due to some race conditions, but since it's
surprising to have a detected crash but not get the corresponding test
status, we want to try defaulting to it being enabled, but allow it to
be disabled for specific CI configurations where it causes problems.

Differential Revision: https://phabricator.services.mozilla.com/D258058

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1978363
gecko-commit: ce9a9f0eaa2acd247b818ccaab9c620feb074f39
gecko-reviewers: ErichDonGubler, Sasha

Author: jgraham

tools/wptrunner/wptrunner/testrunner.py

@@ -343,7 +343,7 @@ def __init__(self, suite_name, index, test_queue,
                  pause_after_test=False, pause_on_unexpected=False,
                  restart_on_unexpected=True, debug_info=None,
                  capture_stdio=True, restart_on_new_group=True, recording=None,
-                 max_restarts=5, max_restart_backoff=0):
+                 max_restarts=5, max_restart_backoff=0, update_status_on_crash=True):
         """Thread that owns a single TestRunner process and any processes required
         by the TestRunner (e.g. the Firefox binary).
 
@@ -381,6 +381,7 @@ def __init__(self, suite_name, index, test_queue,
         self.restart_on_new_group = restart_on_new_group
         self.max_restarts = max_restarts
         self.max_restart_backoff = max_restart_backoff
+        self.update_status_on_crash = update_status_on_crash
 
         assert recording is not None
         self.recording = recording
@@ -797,11 +798,11 @@ def test_ended(self, test, results):
         status = file_result.status
 
         if self.browser.check_crash(test.id) and status != "CRASH":
-            if test.test_type in ["crashtest", "wdspec"] or status == "EXTERNAL-TIMEOUT":
+            if self.update_status_on_crash:
                 self.logger.info("Found a crash dump file; changing status to CRASH")
                 status = "CRASH"
             else:
-                self.logger.warning(f"Found a crash dump; should change status from {status} to CRASH but this causes instability")
+                self.logger.warning(f"Found a crash dump; but keeping status {status}")
 
         # We have a couple of status codes that are used internally, but not exposed to the
         # user. These are used to indicate that some possibly-broken state was reached
@@ -1047,7 +1048,8 @@ def __init__(self, suite_name, test_queue_builder, test_implementations,
                  restart_on_new_group=True,
                  recording=None,
                  max_restarts=5,
-                 max_restart_backoff=0):
+                 max_restart_backoff=0,
+                 update_status_on_crash=False):
         self.suite_name = suite_name
         self.test_queue_builder = test_queue_builder
         self.test_implementations = test_implementations
@@ -1063,6 +1065,7 @@ def __init__(self, suite_name, test_queue_builder, test_implementations,
         assert recording is not None
         self.max_restarts = max_restarts
         self.max_restart_backoff = max_restart_backoff
+        self.update_status_on_crash = update_status_on_crash
 
         self.pool = set()
         self.stop_flag = None
@@ -1096,7 +1099,8 @@ def run(self, tests):
                                         self.restart_on_new_group,
                                         recording=self.recording,
                                         max_restarts=self.max_restarts,
-                                        max_restart_backoff=self.max_restart_backoff)
+                                        max_restart_backoff=self.max_restart_backoff,
+                                        update_status_on_crash=self.update_status_on_crash)
             manager.start()
             self.pool.add(manager)
         self.wait()

tools/wptrunner/wptrunner/wptcommandline.py

@@ -276,6 +276,11 @@ def create_parser(product_choices=None):
     config_group.add_argument("--no-suppress-handler-traceback", action="store_false",
                               dest="supress_handler_traceback",
                               help="Write the stacktrace for exceptions in server handlers")
+    config_group.add_argument("--update-status-on-crash", action="store_true", default=None,
+                              help="Update test status to CRASH if a crash dump is found")
+    config_group.add_argument("--no-update-status-on-crash", dest="update_status_on_crash",
+                              action="store_false",
+                              help="Don't update test status to CRASH if a crash dump is found")
     config_group.add_argument("--ws-extra", action="append",
                               help="Extra paths containing websockets handlers")
 
@@ -636,6 +641,9 @@ def check_args(kwargs):
             print("Binary path %s does not exist" % kwargs["binary"], file=sys.stderr)
             sys.exit(1)
 
+    if kwargs["update_status_on_crash"] is None:
+        kwargs["update_status_on_crash"] = True
+
     if kwargs["ssl_type"] is None:
         if None not in (kwargs["ca_cert_path"], kwargs["host_cert_path"], kwargs["host_key_path"]):
             kwargs["ssl_type"] = "pregenerated"

tools/wptrunner/wptrunner/wptrunner.py

@@ -333,6 +333,7 @@ def run_test_iteration(test_status, test_loader, test_queue_builder,
                           recording=recording,
                           max_restarts=kwargs["max_restarts"],
                           max_restart_backoff=kwargs["max_restart_backoff"],
+                          update_status_on_crash=kwargs["update_status_on_crash"]
                           ) as manager_group:
             try:
                 handle_interrupt_signals()