Skip to content

/ servo

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

Add Bluetooth idlharness test #15080

Merged
merged 1 commit into from Jan 27, 2017
Merged

Add Bluetooth idlharness test #15080

merged 1 commit into from Jan 27, 2017

Conversation

@dati91
Copy link
Contributor

dati91 commented Jan 17, 2017 •
edited by larsbergstrom

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
@dati91
Copy link
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
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
Contributor Author

dati91 commented Jan 17, 2017

I tested on a70af60 revision.
@jdm jdm mentioned this pull request Jan 17, 2017
Closed
@jdm
Copy link
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.
@dati91
Copy link
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
Copy link
Member

jdm commented Jan 25, 2017

@bors-servo: try
@bors-servo
Copy link
Contributor

bors-servo commented Jan 25, 2017

⌛ Trying commit d0ce241 with merge 798fc6b...
bors-servo added a commit that referenced this pull request Jan 25, 2017
@bors-servo
Auto merge of #15080 - dati91:idlharness, r=<try>
Loading status checks…
798fc6b 
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
Contributor

bors-servo commented Jan 25, 2017

💔 Test failed - linux-rel-wpt
@dati91
Add bluetooth idlharness test
Loading status checks…
c5dd88f
@dati91 dati91 force-pushed the dati91:idlharness branch from d0ce241 to c5dd88f Jan 26, 2017
@dati91
Copy link
Contributor Author

dati91 commented Jan 26, 2017

@jdm Updated the expected list and squashed everything.
@jdm
Copy link
Member

jdm commented Jan 26, 2017

@bors-servo: r+
@bors-servo
Copy link
Contributor

bors-servo commented Jan 26, 2017

📌 Commit c5dd88f has been approved by jdm
@bors-servo
Copy link
Contributor

bors-servo commented Jan 26, 2017

⌛ Testing commit c5dd88f with merge d8f30e7...
bors-servo added a commit that referenced this pull request Jan 26, 2017
@bors-servo
Auto merge of #15080 - dati91:idlharness, r=jdm
Loading status checks…
d8f30e7 
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
Contributor

bors-servo commented Jan 26, 2017

💔 Test failed - linux-rel-wpt
@dati91
Copy link
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
Member

jdm commented Jan 27, 2017

@bors-servo: retry
@bors-servo
Copy link
Contributor

bors-servo commented Jan 27, 2017

⌛ Testing commit c5dd88f with merge f141ede...
bors-servo added a commit that referenced this pull request Jan 27, 2017
@bors-servo
Auto merge of #15080 - dati91:idlharness, r=jdm
Loading status checks…
f141ede 
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
Contributor

bors-servo commented Jan 27, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev
@bors-servo bors-servo merged commit c5dd88f into servo:master Jan 27, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jdm jdm

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@dati91 @jdm @bors-servo @emilio @highfive
You can’t perform that action at this time.