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

Replace Syntax with TypeError in bluetoothuuid.rs #14429

Merged
merged 3 commits into from Dec 7, 2016
Merged

Replace Syntax with TypeError in bluetoothuuid.rs #14429

merged 3 commits into from Dec 7, 2016

Conversation

@zakorgy
Copy link
Contributor

zakorgy commented Dec 1, 2016 •
edited by larsbergstrom

  1. Replacing SyntaxError with TypeError in bluetoothuuid.rs due to the specification change.
    This indicates changes in the existing tests.
    The error strings are from the chromium implementation.
  2. We missed out a $ character from the end of the VALID_UUID_REGEX global variable.
  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes OR

This change is Reviewable
zakorgy added 2 commits Nov 21, 2016
@zakorgy
BluetoothUUID refactor
7e2a012
@zakorgy
Add end of text character to BluetoothUUID's regex string
Loading status checks…
4132d0f
@highfive
Copy link

highfive commented Dec 1, 2016

Heads up! This PR modifies the following files:

  • @fitzgen: components/script/dom/bluetoothuuid.rs
  • @KiChjang: components/script/dom/bluetoothuuid.rs
@zakorgy
Copy link
Contributor Author

zakorgy commented Dec 1, 2016

r? @jdm
@highfive highfive assigned jdm and unassigned wafflespeanut Dec 1, 2016
@jdm
Copy link
Member

jdm commented Dec 6, 2016

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

bors-servo commented Dec 6, 2016

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

bors-servo commented Dec 6, 2016

⌛ Testing commit 4132d0f with merge 21f027c...
bors-servo added a commit that referenced this pull request Dec 6, 2016
@bors-servo
Auto merge of #14429 - szeged:syntax-to-type-error, r=jdm
Loading status checks…
21f027c 
Replace Syntax with TypeError in bluetoothuuid.rs

<!-- Please describe your changes on the following line: -->
1. Replacing `SyntaxError` with `TypeError` in `bluetoothuuid.rs` due to the specification change.
This indicates changes in the existing tests.
The error strings are from the chromium implementation.
2. We missed out a `$` character from the end of the `VALID_UUID_REGEX` global variable.

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

<!-- Either: -->
- [x] There are tests for these changes OR

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

bors-servo commented Dec 6, 2016

💔 Test failed - mac-rel-wpt2
@wafflespeanut
Copy link
Member

wafflespeanut commented Dec 6, 2016

One failure was #14380, but there's two other...

  ▶ Unexpected subtest result in /_mozilla/mozilla/bluetooth/getDescriptor/blocklisted-descriptor.html:
  │ FAIL [expected PASS] The descriptor is blocklisted.
  └   → assert_throws: function "function () { throw e }" threw object "TypeError: Invalid Descriptor name : '07711111-6104-0970-7011-1107105110aaa'.
It must be a valid UUID alias (e.g. 0x1234), UUID (lowercase hex characters e.g. '00001234-0000-1000-8000-00805f9b34fb'),
or recognized standard name from https://developer.bluetooth.org/gatt/descriptors/Pages/DescriptorsHomePage.aspx
e.g. 'gatt.characteristic_presentation_format'." that is not a DOMException SecurityError: property "code" is equal to undefined, expected 18

  ▶ Unexpected subtest result in /_mozilla/mozilla/bluetooth/getDescriptors/blocklisted-descriptors-with-uuid.html:
  │ FAIL [expected PASS] The descriptor is blocklisted. Should reject with SecurityError.
  └   → assert_throws: function "function () { throw e }" threw object "TypeError: Invalid Descriptor name : '07711111-6104-0970-7011-1107105110aaa'.
It must be a valid UUID alias (e.g. 0x1234), UUID (lowercase hex characters e.g. '00001234-0000-1000-8000-00805f9b34fb'),
or recognized standard name from https://developer.bluetooth.org/gatt/descriptors/Pages/DescriptorsHomePage.aspx
e.g. 'gatt.characteristic_presentation_format'." that is not a DOMException SecurityError: property "code" is equal to undefined, expected 18
@dati91
Copy link
Contributor

dati91 commented Dec 6, 2016 •
edited

It seems like the regex fix brought out a typo where a uuid's last part is 13 char long instead of 12.
@dati91
Fix invalid descriptor uuid
Loading status checks…
1cb694b
@wafflespeanut
Copy link
Member

wafflespeanut commented Dec 6, 2016

@bors-servo r=jdm
@bors-servo
Copy link
Contributor

bors-servo commented Dec 6, 2016

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

bors-servo commented Dec 7, 2016

⌛ Testing commit 1cb694b with merge b54cfc9...
bors-servo added a commit that referenced this pull request Dec 7, 2016
@bors-servo
Auto merge of #14429 - szeged:syntax-to-type-error, r=jdm
Loading status checks…
b54cfc9 
Replace Syntax with TypeError in bluetoothuuid.rs

<!-- Please describe your changes on the following line: -->
1. Replacing `SyntaxError` with `TypeError` in `bluetoothuuid.rs` due to the specification change.
This indicates changes in the existing tests.
The error strings are from the chromium implementation.
2. We missed out a `$` character from the end of the `VALID_UUID_REGEX` global variable.

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

<!-- Either: -->
- [x] There are tests for these changes OR

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

bors-servo commented Dec 7, 2016

☀️ Test successful - arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev
@bors-servo bors-servo merged commit 1cb694b into servo:master Dec 7, 2016
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

6 participants
@zakorgy @highfive @jdm @bors-servo @wafflespeanut @dati91
You can’t perform that action at this time.