Fix mach test-wpt to make crash tests work
#29832
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@bors-servo try |
bors-servo
added a commit
that referenced
this pull request
Jun 2, 2023
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. -->
|
Test results for linux-wpt-layout-2020 from try job (#5155522379): Flaky unexpected result (1)
Stable unexpected results that are known to be intermittent (1)
Stable unexpected results (3)
|
|
Test results for linux-wpt-layout-2013 from try job (#5155522379): Flaky unexpected result (9)
Stable unexpected results that are known to be intermittent (22)
Stable unexpected results (12)
|
|
💔 Test failed - checks-github |
5 tasks
|
Hrm. It seems like this is causing more timeouts. I wonder what could be preventing the process from exiting in these tests... |
bors-servo
added a commit
that referenced
this pull request
Jun 3, 2023
Clear PositioningContext for speculative layouts <!-- Please describe your changes on the following line: --> Developed in collaboration with @mrobinson `try_layout` is used for laying out absolutely positioned descendants multiple times when min/max-{width, height} properties are set. When the same PositioningContext instance is used between successive attempts without clearing the accumulated descendants, we will generate multiple fragments which reference the same box, which then will lead to a double borrow error when layout is performed in parallel. --- <!-- 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 crash testing is currently not working in servo. New tests will be added once #29832 is fixed. <!-- 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. -->
mukilan
force-pushed
the
fix-crashtests
branch
from
bd94242
to
d9fcc49
Compare
|
🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#41513) with upstreamable changes. |
|
✍ Updated existing upstream WPT pull request (web-platform-tests/wpt#41513) title and body. |
|
@bors-servo try |
|
🔨 Triggering try run (#5892033956) with platform=all and layout=all |
|
Test results for linux-wpt-layout-2013 from try job (#5892033956): Flaky unexpected result (12)
Stable unexpected results that are known to be intermittent (13)
|
|
Test results for linux-wpt-layout-2020 from try job (#5892033956): Flaky unexpected result (8)
Stable unexpected results that are known to be intermittent (32)
Stable unexpected results (1)
|
|
|
4 tasks
jdm
approved these changes
Aug 18, 2023
|
🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync. |
mrobinson
force-pushed
the
fix-crashtests
branch
from
c4932d1
to
e206d3b
Compare
|
🛠 These changes could not be applied onto the latest upstream WPT. Servo's copy of the Web Platform Tests may be out of sync. |
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. Co-authored-by: Martin Robinson <mrobinson@igalia.com> Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Servo's panic hook writes backtraces to stdout. This patch changes it so they are written to stderr. The crash test executor for servo in WPT grouping formatter was also not capturing the output correctly for crashtests as the log events were being aggregated based on thread name which doesn't seem to match correctly in case of crashtests. This patch also fixes the log grouping logic to be based on test name. Co-authored-by: Martin Robinson <mrobinson@igalia.com> Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com> Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com> Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
mukilan
force-pushed
the
fix-crashtests
branch
from
e206d3b
to
92209d7
Compare
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#41513). |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
Looks like the two failures are:
The first was recently fixed, so I think we can remove that expectation. The second must be a flaky crash. For that I guess we simply need to remove the expectation and open an I-Intermittent bug to track it. |
Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
|
Thanks @mrobinson! I've removed the expectations for those tests and repurposed #30124 to track the intermittent CRASH |
|
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#41513). |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
I suspect that this failure in the float unit tests is unrelated to this change. The float unit tests use quickcheck to essentially "fuzz" the float layout algorithm. Sometimes the values passed stretch the limits of our floating point layout in Layout 2020 (see #29819). To confirm, I'm going to send this back to the MQ to see if this behavior is flaky. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
mrobinson
approved these changes
Aug 18, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The first commit fixes two issues related to crash tests:
The second commit addresses issues with generating and capturing backtrace and stderr for crash tests.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errors