Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added exception-handling to worker wpt tests. #17475

Merged
merged 1 commit into from Jun 23, 2017

Conversation

asajeffrey
Copy link
Member

@asajeffrey asajeffrey commented Jun 22, 2017

Added exception-handling to worker wpt tests.



This change is Reviewable

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jun 22, 2017
@asajeffrey
Copy link
Member Author

r? @emilio

@highfive highfive assigned emilio and unassigned metajack Jun 22, 2017
@asajeffrey
Copy link
Member Author

cc @jgraham

@emilio
Copy link
Member

emilio commented Jun 22, 2017

Looks reasonable to me, but I'd like @jdm to look at it and stamp it.

Thanks!

@@ -4,6 +4,10 @@
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script>
// The worker events races with the window's load event; if the worker events
// arrive first, the harness will detect the error event and fail the test.
setup({ allow_uncaught_exception: true });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this instead of calling preventDefault?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it's what all the other tests do:

$ grep -ir allow_uncaught_exception tests/wpt/web-platform-tests/workers/
tests/wpt/web-platform-tests/workers/WorkerGlobalScope_ErrorEvent_filename.htm:setup({ allow_uncaught_exception: true });
tests/wpt/web-platform-tests/workers/WorkerGlobalScope_ErrorEvent_lineno.htm:setup({ allow_uncaught_exception: true });
tests/wpt/web-platform-tests/workers/Worker_ErrorEvent_type.htm:setup({ allow_uncaught_exception: true });
tests/wpt/web-platform-tests/workers/interfaces/WorkerGlobalScope/onerror/propagate-to-window-onerror.html:  allow_uncaught_exception: true,
tests/wpt/web-platform-tests/workers/WorkerGlobalScope_ErrorEvent_colno.htm:setup({ allow_uncaught_exception: true });
tests/wpt/web-platform-tests/workers/constructors/SharedWorker/same-origin.html:setup({allow_uncaught_exception: true});
tests/wpt/web-platform-tests/workers/constructors/Worker/AbstractWorker.onerror.html:setup({allow_uncaught_exception:true});
tests/wpt/web-platform-tests/workers/constructors/Worker/same-origin.html:setup({allow_uncaught_exception: true});
tests/wpt/web-platform-tests/workers/Worker_ErrorEvent_message.htm:setup({ allow_uncaught_exception: true });
tests/wpt/web-platform-tests/workers/Worker_ErrorEvent_bubbles_cancelable.htm:setup({ allow_uncaught_exception: true });
tests/wpt/web-platform-tests/workers/WorkerGlobalScope_ErrorEvent_message.htm:setup({ allow_uncaught_exception: true });
tests/wpt/web-platform-tests/workers/Worker_ErrorEvent_filename.htm:setup({ allow_uncaught_exception: true });
tests/wpt/web-platform-tests/workers/Worker_ErrorEvent_lineno.htm:setup({ allow_uncaught_exception: true });
tests/wpt/web-platform-tests/workers/semantics/reporting-errors/004.html:setup({allow_uncaught_exception:true});
tests/wpt/web-platform-tests/workers/semantics/reporting-errors/002.html:setup({allow_uncaught_exception:true});
tests/wpt/web-platform-tests/workers/semantics/reporting-errors/001.html:setup({allow_uncaught_exception:true});
tests/wpt/web-platform-tests/workers/semantics/reporting-errors/003.html:setup({allow_uncaught_exception:true});
tests/wpt/web-platform-tests/workers/semantics/run-a-worker/003.html:setup({allow_uncaught_exception: true});

The odd one out is data-url.html, since some of the tests should raise an exception and some should not, so in that case I explicitly blocked the exception from propagating.

@jdm
Copy link
Member

jdm commented Jun 22, 2017

@bors-servo: r+

@bors-servo
Copy link
Contributor

📌 Commit 030e01c has been approved by jdm

@highfive highfive assigned jdm and unassigned emilio Jun 22, 2017
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jun 22, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 030e01c with merge 14e9bca0a7a94db80c98bc8bfc9bd5817d40a924...

@bors-servo
Copy link
Contributor

⛄ The build was interrupted to prioritize another pull request.

@bors-servo
Copy link
Contributor

⌛ Testing commit 030e01c with merge 9c08a7b...

bors-servo pushed a commit that referenced this pull request Jun 23, 2017
Added exception-handling to worker wpt tests.

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

Added exception-handling to worker wpt tests.

---
<!-- 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
- [X] These changes fix #15668 and #17460
- [X] These changes do not require tests because these are tests

<!-- 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. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17475)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: jdm
Pushing 9c08a7b to master...

@bors-servo bors-servo merged commit 030e01c into servo:master Jun 23, 2017
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jun 23, 2017
jdm pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 6, 2017
jdm pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 12, 2017
jakearchibald pushed a commit to jakearchibald/web-platform-tests that referenced this pull request Nov 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Intermittent OK in /workers/data-url.html
6 participants