Skip to content

Add Bluetooth idlharness test#15080

Merged
bors-servo merged 1 commit intoservo:masterfrom
dati91:idlharness
Jan 27, 2017
Merged

Add Bluetooth idlharness test#15080
bors-servo merged 1 commit intoservo:masterfrom
dati91:idlharness

Conversation

@dati91
Copy link
Copy Markdown
Contributor

@dati91 dati91 commented Jan 17, 2017

Add idlharness wpt test file for WebBluetooth.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #__ (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 17, 2017
@dati91
Copy link
Copy Markdown
Contributor Author

dati91 commented Jan 17, 2017

r? @jdm
This was a long time debt from #13612 (comment).

Please not that this PR is not complete yet!

I broke it 2 separate patch because the 2. will crash servo... crashlog
I tried to investigate it, but the only thing i found is ConstellationControlMsg::ExitScriptThread message was sent right before we started the promise handle method. But I couldn't figure out what caused it (i mean I know its from a drop when the channel is destroyed, but what caused it).
I would appreciate any help with that!

@highfive highfive assigned jdm and unassigned emilio Jan 17, 2017
@jdm
Copy link
Copy Markdown
Member

jdm commented Jan 17, 2017

@dati91 Do you still see that crash if you rebase against master? I'm used to seeing stacks like that before servo/rust-mozjs#326 and #15041 merged.

@dati91
Copy link
Copy Markdown
Contributor Author

dati91 commented Jan 17, 2017

I tested on a70af60 revision.

@jdm
Copy link
Copy Markdown
Member

jdm commented Jan 19, 2017

The changes here look fine. Now that the cause for the crash has been discovered in #15085, I'd like to hold off on merging this until the crash is fixed.

@jdm jdm added S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. S-needs-squash Some (or all) of the commits in the PR should be combined. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jan 19, 2017
@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 20, 2017
@dati91
Copy link
Copy Markdown
Contributor Author

dati91 commented Jan 20, 2017

I also added the missing objects (intentionally missing to minimize the crash test case).
If the crash cause is resolved I still have to do a rebase as well to fix the expected error list if necessary.

@jdm jdm removed S-needs-squash Some (or all) of the commits in the PR should be combined. S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. labels Jan 23, 2017
@jdm
Copy link
Copy Markdown
Member

jdm commented Jan 25, 2017

@bors-servo: try

@bors-servo
Copy link
Copy Markdown
Contributor

⌛ Trying commit d0ce241 with merge 798fc6b...

bors-servo pushed a commit that referenced this pull request Jan 25, 2017
Add Bluetooth idlharness test

<!-- Please describe your changes on the following line: -->
Add idlharness wpt test file for WebBluetooth.

---
<!-- 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: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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/15080)
<!-- Reviewable:end -->
@bors-servo
Copy link
Copy Markdown
Contributor

💔 Test failed - linux-rel-wpt

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Jan 25, 2017
@highfive highfive removed the S-tests-failed The changes caused existing tests to fail. label Jan 26, 2017
@dati91
Copy link
Copy Markdown
Contributor Author

dati91 commented Jan 26, 2017

@jdm Updated the expected list and squashed everything.

@jdm
Copy link
Copy Markdown
Member

jdm commented Jan 26, 2017

@bors-servo: r+

@bors-servo
Copy link
Copy Markdown
Contributor

📌 Commit c5dd88f has been approved by jdm

@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 Jan 26, 2017
@bors-servo
Copy link
Copy Markdown
Contributor

⌛ Testing commit c5dd88f with merge d8f30e7...

bors-servo pushed a commit that referenced this pull request Jan 26, 2017
Add Bluetooth idlharness test

<!-- Please describe your changes on the following line: -->
Add idlharness wpt test file for WebBluetooth.

---
<!-- 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: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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/15080)
<!-- Reviewable:end -->
@bors-servo
Copy link
Copy Markdown
Contributor

💔 Test failed - linux-rel-wpt

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jan 26, 2017
@dati91
Copy link
Copy Markdown
Contributor Author

dati91 commented Jan 27, 2017

Hmm, this can't be related because I only added a test... :/

{"status": "FAIL", "stack": null, "subtest": null, "action": "test_result", "test": "/_mozilla/css/iframe/hide_and_show.html", "line": 7915, "message": "/_mozilla/css/iframe/hide_and_show.html 2008c2e9f2c032e2a0e85f2cb521cb025574cf82\n/_mozilla/css/iframe/hide_and_show_ref.html 5aca5847510aafc7accd66fc41a5309e5656f02c\nTesting 2008c2e9f2c032e2a0e85f2cb521cb025574cf82 == 5aca5847510aafc7accd66fc41a5309e5656f02c", "expected": "PASS"}

@jdm
Copy link
Copy Markdown
Member

jdm commented Jan 27, 2017

@bors-servo
Copy link
Copy Markdown
Contributor

⌛ Testing commit c5dd88f with merge f141ede...

@highfive highfive removed the S-tests-failed The changes caused existing tests to fail. label Jan 27, 2017
bors-servo pushed a commit that referenced this pull request Jan 27, 2017
Add Bluetooth idlharness test

<!-- Please describe your changes on the following line: -->
Add idlharness wpt test file for WebBluetooth.

---
<!-- 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: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- 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/15080)
<!-- Reviewable:end -->
@highfive highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 27, 2017
@bors-servo
Copy link
Copy Markdown
Contributor

@bors-servo bors-servo merged commit c5dd88f into servo:master Jan 27, 2017
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants