Skip to content

Commit

Permalink
Auto merge of #29832 - mukilan:fix-crashtests, r=<try>
Browse files Browse the repository at this point in the history
Fix `mach test-wpt` to make crash tests work

There are two issues related to crash tests:
1. test-wpt is unable to find existing crash tests even when called with --test-types=crashtests. The fix here is to add crashtests to the default test suite types to python/wpt/run.py
2. When running in headless mode, crashes in style threads don't cause servo to crash because the logic in constellation.rs currently calls handle_panic only when the top-level browsing context id is some value. Since style pool threads are shared, they always generate Panic messages with None as top-level browsing context id.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they fix bugs in testing logic

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
  • Loading branch information
bors-servo committed Jun 2, 2023
2 parents 0ea942f + bd94242 commit 08f2d99
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 16 deletions.
35 changes: 20 additions & 15 deletions components/constellation/constellation.rs
Expand Up @@ -1491,11 +1491,10 @@ where
// Panic a top level browsing context.
FromCompositorMsg::SendError(top_level_browsing_context_id, error) => {
debug!("constellation got SendError message");
if let Some(id) = top_level_browsing_context_id {
self.handle_panic(id, error, None);
} else {
if top_level_browsing_context_id.is_none() {
warn!("constellation got a SendError message without top level id");
}
self.handle_panic(top_level_browsing_context_id, error, None);
},
// Send frame tree to WebRender. Make it visible.
FromCompositorMsg::SelectBrowser(top_level_browsing_context_id) => {
Expand Down Expand Up @@ -2757,15 +2756,13 @@ where
.pipelines
.get(&pipeline_id)
.map(|pipeline| pipeline.top_level_browsing_context_id);
if let Some(top_level_browsing_context_id) = top_level_browsing_context_id {
let reason = format!("Send failed ({})", err);
self.handle_panic(top_level_browsing_context_id, reason, None);
}
let reason = format!("Send failed ({})", err);
self.handle_panic(top_level_browsing_context_id, reason, None);
}

fn handle_panic(
&mut self,
top_level_browsing_context_id: TopLevelBrowsingContextId,
top_level_browsing_context_id: Option<TopLevelBrowsingContextId>,
reason: String,
backtrace: Option<String>,
) {
Expand All @@ -2776,6 +2773,11 @@ where
process::exit(1);
}

let top_level_browsing_context_id = match top_level_browsing_context_id {
Some(id) => id,
None => return,
};

debug!(
"Panic handler for top-level browsing context {}: {}.",
top_level_browsing_context_id, reason
Expand Down Expand Up @@ -2857,13 +2859,16 @@ where
entry: LogEntry,
) {
debug!("Received log entry {:?}.", entry);
match (entry, top_level_browsing_context_id) {
(LogEntry::Panic(reason, backtrace), Some(top_level_browsing_context_id)) => {
self.handle_panic(top_level_browsing_context_id, reason, Some(backtrace));
},
(LogEntry::Panic(reason, _), _) |
(LogEntry::Error(reason), _) |
(LogEntry::Warn(reason), _) => {
if let LogEntry::Panic(ref reason, ref backtrace) = entry {
self.handle_panic(
top_level_browsing_context_id,
reason.clone(),
Some(backtrace.clone()),
);
}

match entry {
LogEntry::Panic(reason, _) | LogEntry::Error(reason) | LogEntry::Warn(reason) => {
// VecDeque::truncate is unstable
if WARNINGS_BUFFER_SIZE <= self.handled_warnings.len() {
self.handled_warnings.pop_front();
Expand Down
2 changes: 1 addition & 1 deletion python/wpt/run.py
Expand Up @@ -102,7 +102,7 @@ def run_tests(**kwargs):

if not kwargs.get("no_default_test_types"):
test_types = {
"servo": ["testharness", "reftest", "wdspec"],
"servo": ["testharness", "reftest", "wdspec", "crashtest"],
"servodriver": ["testharness", "reftest"],
}
product = kwargs.get("product") or "servo"
Expand Down

0 comments on commit 08f2d99

Please sign in to comment.