Parse tombstones on android devices #27815
Conversation
Added parameter to allow wrappter to pass in a tombstone parser script, and parse any tombstones at end of each test, and save result to result viewer Tested with /font-access, tombstone is passed and saved
wpt-pr-bot
added
infra
wptrunner
There was a problem hiding this comment.
Apologies for the late review but I think I agree with @stephenmcgruer that we could make this patch reuse some of the existing infrastructure rather than being its own thing.
| @@ -615,6 +615,9 @@ def test_ended(self, test, results): | |||
| return | |||
| if self.timer is not None: | |||
| self.timer.cancel() | |||
|
|
|||
| self.browser.browser.maybe_parse_tombstone() | |||
There was a problem hiding this comment.
So this is basically checking for crashes?
I wonder why it couldn't be part of browser.check_crash in that case? There's already logic below (line 661) to handle this; it might be enough just to have the android browsers implement that?
There was a problem hiding this comment.
This is just to parse any new tombstones on the Android device. I want to keep this separate to check_crash, or log_crash, because tombstones could come from unrelated processes. Currently there is no reliable way (yet) to know which process ids are for the browser under test. At design stage, I planned to filter tombstones against process name. Turned out this may not work, due to 1) process name may change e.g. when package name changes, 2) sometimes a tombstone file could have multiple tombstones.
I actually checked check_crash when I was implementing this. Looks like it will touch test status, which is what I don't want to.
There was a problem hiding this comment.
It only actually does this for crashtests at the moment, because it was causing problems to unconditionally set the status to crash when a dump file was found. And you could always return False unconditionally if you never want the harness to react as if a crash has been found.
There was a problem hiding this comment.
Let me give this a try, thanks!
| cmd.extend(["--output-directory", self.output_directory]) | ||
| raw_output = subprocess.check_output(cmd) | ||
| for line in raw_output.splitlines(): | ||
| logger.process_output("TRACE", line, "logcat") |
There was a problem hiding this comment.
If we actually find a crash can we call logger.log_crash so it is logged as a crash?
There was a problem hiding this comment.
So I would say this is similar to the discussion for check_crash at above, and let's don't do this yet.
Added parameter to allow wrapper to pass in a tombstone
parser script, and parse any tombstones at end of each test,
and save result to result viewer
Tested with /font-access, tombstone is passed and saved