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

Update webidl2.js to v8.1.0 #8063

Merged

Conversation

gsnedders
Copy link
Member

@gsnedders gsnedders commented Nov 3, 2017

THIS MUST BE MERGED WITH "CREATE MERGE COMMIT" WITH HISTORY AS-IS AND NOT SQUASHED OR REBASED (failure to do this will make future updates painful).


This change is Reviewable

8a7ff70664 chore(package): bump version number to 8.1.0
f26e320e49 feat: always add rhs property (web-platform-tests#110)
8fc7b2ea19 docs(README): Removed redundant sections
649e457b78 chore(CHANGELOG): regenerate
c1af872481 chore(package): bump version number to 8.0.1
407aaefc9f Remove m postfix from all_ws() (web-platform-tests#108)
8f7c37ea23 chore(package): bump version number to 8.0
6f86663dbc feat: support mixins + includes statements (web-platform-tests#105)
d1f2352275 docs(README): fix typo
0ecc48607c BREAKING CHANGE: drop creator support (web-platform-tests#101)
4f1aab4e77 chore: drop Node 6 support (web-platform-tests#102)
af08834028 style: Normalize some whitespace to pass wpt's lint (web-platform-tests#99)
e938ba5eea chore(package): bump version number to 7.0
cb85ff1a02 BREAKING CHANGE: argument type should be a string
101d9067ae chore(package): bump version number
67af674490 docs(README): extended attribute type
b4013a14cc feat: give extended attributes a type
d15fcb28a9 Yet another innocuous change for tests
dd01a2ba48 Another innocuous change for tests
706d6251d5 Innocuous change to trigger a cvs.w3.org refresh
31be15dd81 chore(package): update jsondiffpatch
ca6c08ccd8 chore(CHANGELOG): regenerate
8ce266fc02 Use ES2015 syntax for tests (web-platform-tests#88)
7ffef282e1 chore(package): bump version number
9d2071443c BREAKING CHANGE: ret enum value as object (web-platform-tests#87)
8360040a9d chore(package): bump version number
fa4b9a3ef1 BREAKING CHANGE: Use ES2015 syntax (web-platform-tests#84)
4252cef22e chore(package): update deps
d6b17d84a1 chore(package): bump version number
174e47ea6d Check duplicated names (web-platform-tests#80)
981743ad2e remove legacycaller (web-platform-tests#79)
99100d174b Add "sequence" property to IDL Type AST definition (web-platform-tests#76)
23fc13f951 chore(CHANGELOG): regenerate
6b53eb357b chore(package): bump version
dbce4340c7 feat: support TypeWithExtendedAttributes on generics (web-platform-tests#75)
c5facac405 chore(CHANGELOG): regenerate
0077262a8f chore(CHANGELOG): regenerate
bbbab23b3d BREAKING CHANGE: remove serializers (closes web-platform-tests#73) (web-platform-tests#74)
5d7a972728 docs(README): add namespaces (web-platform-tests#70)
1ad592c3bb chore(package): bump version
056f0cbaf8 chore(.travis): test for latest LTS/stable node versions (web-platform-tests#69)
c6ef83e72f chore(pacakge): bump version
f5d777c7ab fix extattr whitespace error (web-platform-tests#68)
112e8feb16 chore(package): bump version + update deps
8ec814b9f3 BREAKING CHANGE: deprecate arrays and exceptions (web-platform-tests#67)

git-subtree-dir: resources/webidl2
git-subtree-split: 8a7ff70664ef54fbca77d9bda660865a369e9ce6
@gsnedders
Copy link
Member Author

gsnedders commented Nov 3, 2017

Note the first commit here (the squash commit) deletes all of WPT and reintroduces webidl2 at the root of the repo, as git-subtree expects (there should essentially be a linear history of squash commits, occasionally merged into master with the files moved). The previous squash commit has everything at resources/webidl2 because it comes from @jgraham squash-and-merging the previous update at #5665, and this confuses git-subtree massively and I just wasted two hours trying to figure out how to do this. :(

@foolip
Copy link
Member

foolip commented Nov 3, 2017

We don't have any tools to judge how much this would break, if anything, so I'm going to try applying the changes to Chromium and seeing if any tests fail there.

@foolip foolip closed this Nov 3, 2017
@foolip foolip reopened this Nov 3, 2017
@ghost
Copy link

ghost commented Nov 3, 2017

Build ERRORED

Started: 2017-11-20 19:05:35
Finished: 2017-11-20 19:54:06

Failing Jobs

  • firefox:nightly
  • MicrosoftEdge:14.14393

View more information about this build on:

@foolip
Copy link
Member

foolip commented Nov 4, 2017

@foolip
Copy link
Member

foolip commented Nov 4, 2017

I just wasted two hours trying to figure out how to do this. :(

Do you have instructions for the next time we do this, to not put it in a weird state again?

Copy link
Member

@foolip foolip left a comment

Choose a reason for hiding this comment

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

@marcoscaceres
Copy link
Contributor

Just wondering, but how come we don’t “npm install” this instead? Or is adding a dependency on node/npm too much?

@foolip
Copy link
Member

foolip commented Nov 4, 2017

Just wondering, but how come we don’t “npm install” this instead? Or is adding a dependency on node/npm too much?

Replacing all of our python infrastructure with node would be kinda nice, but just to clone a repo it'd be a bit of a gratuitous dependency I think. We'd still have to upgrade it once in a while, and we could have a 3-line script for it. Then, figuring out what will break will be the bulk of the work.

@marcoscaceres
Copy link
Contributor

Replacing all of our python infrastructure with node would be kinda nice, but just to clone a repo it'd be a bit of a gratuitous dependency I think.

Absolutely. I was only wondering just for js dependencies that are on npm, and not suggesting we replace any existing python infrastructure.

@foolip
Copy link
Member

foolip commented Nov 4, 2017

Absolutely. I was only wondering just for js dependencies that are on npm, and not suggesting we replace any existing python infrastructure.

Sure, that was my half-serious suggestion :)

@gsnedders
Copy link
Member Author

@foolip:

Do you have instructions for the next time we do this, to not put it in a weird state again?

Yes. Don't squash-and-merge or rebase-and-merge, always push the history as git-subtree creates it (either manually or with GitHub "create merge commit"). And then it should just be git subtree pull --prefix=resources/webidl2 https://github.com/w3c/webidl2.js.git v8.1.0.

@jgraham
Copy link
Contributor

jgraham commented Nov 4, 2017

Replacing all of our python infrastructure with node would be kinda nice

No it wouldn't.

@foolip
Copy link
Member

foolip commented Nov 4, 2017

Replacing all of our python infrastructure with node would be kinda nice

No it wouldn't.

To be clear, I don't actually think we should do that. The bit that would have been nice is one fewer language to understand for contributors.

@marcoscaceres
Copy link
Contributor

To be clear, I don't actually think we should do that. The bit that would have been nice is one fewer language to understand for contributors.

Agree. I've learned Python maybe 3 or 4 times in the last 15 years, but use it so rarely that I always end up forgetting it. I imagine it's the same for most people doing web stuff, with JS being a more natural fit and Node being much more capable nowadays.

At least we should make an to not add new stuff in Python if we can avoid it.

@foolip
Copy link
Member

foolip commented Nov 4, 2017

I don't know about that, at this point wpt infra is entirely Python, and doing some new things in another language would end up fairly messy I think, unless very isolated. To really switch away from Python would be a very large undertaking, and not a very good use of time at this point I think. Although, I do know Python somewhat well and mostly like it, so I may not be representative.

@gsnedders
Copy link
Member Author

When Python is the scripting language used by three out of four major browser engines, there's a somewhat strong argument against moving away from it.

As for the regressions:

  • We have a number of WebIDL fragments that fail to parse (css/cssom-view/interfaces.html, css/cssom/interfaces.html, gamepad/idlharness.html, html/dom/interfaces.html, html/dom/interfaces.worker.html, mediacapture-streams/MediaDevices-IDL-all.html, mediacapture-streams/MediaDevices-IDL-enumerateDevices.html, selection/interfaces.html, webaudio/the-audio-api/the-delaynode-interface/idl-test.html, webaudio/the-audio-api/the-gainnode-interface/idl-test.html, webrtc/interfaces.https.html). @marcoscaceres can you have a quick look at some of these and figure out whether the WebIDL in the spec doesn't conform, whether the test contains the wrong WebIDL, or whether there's a bug of webidl2.js? I suspect you can do that much quicker than me!
  • We have loads of failures like FAIL XMLHttpRequest interface: constant UNSENT on interface object assert_equals: property has wrong value expected (string) "0" but got (number) 0. This is the breaking change from BREAKING CHANGE: argument + default types should be string w3c/webidl2.js#95.

@foolip
Copy link
Member

foolip commented Nov 13, 2017

What part of the syntax is problematic in the above mentioned issues?

The first one must be because of array syntax in https://github.com/w3c/web-platform-tests/blob/a24dde9c8969dab3dd422b25902198faf4963823/gamepad/idlharness.html#L55

@gsnedders, do you have an exhaustive list?

@foolip
Copy link
Member

foolip commented Nov 13, 2017

I'm fixing the gamepad one in #8168

foolip added a commit that referenced this pull request Nov 13, 2017
This is causing a parse error with upgraded webidl2.js:
#8063
foolip added a commit that referenced this pull request Nov 13, 2017
The serializer is blocking #8063

ufrag was renamed in w3c/webrtc-pc#1479
@tobie
Copy link
Contributor

tobie commented Nov 13, 2017

Opened whatwg/webidl#477 for a proposed solution to this issue.

foolip added a commit that referenced this pull request Nov 14, 2017
The serializer is blocking #8063

ufrag was renamed in w3c/webrtc-pc#1479
foolip added a commit that referenced this pull request Nov 14, 2017
This is causing a parse error with upgraded webidl2.js:
#8063
@gsnedders
Copy link
Member Author

@foolip so only that remains to be fixed?

@gsnedders
Copy link
Member Author

@foolip If that's all of them now resolved, let's try one more CQ run to make sure nothing has regressed again?

@foolip
Copy link
Member

foolip commented Nov 20, 2017

@foolip
Copy link
Member

foolip commented Nov 20, 2017

OK, all the previously noticed issues are resolved now, only this thing remains:
https://storage.googleapis.com/chromium-layout-test-archives/linux_chromium_rel_ng/591895/layout-test-results/results.html

--- /b/s/w/ioeX1Xzh/layout-test-results/http/tests/w3c/webperf/approved/navigation-timing/html/idlharness-expected.txt
+++ /b/s/w/ioeX1Xzh/layout-test-results/http/tests/w3c/webperf/approved/navigation-timing/html/idlharness-actual.txt
@@ -55,22 +55,22 @@
 PASS PerformanceNavigation interface object name
 PASS PerformanceNavigation interface: existence and properties of interface prototype object
 PASS PerformanceNavigation interface: existence and properties of interface prototype object's "constructor" property
-PASS PerformanceNavigation interface: constant TYPE_NAVIGATE on interface object
-PASS PerformanceNavigation interface: constant TYPE_NAVIGATE on interface prototype object
-PASS PerformanceNavigation interface: constant TYPE_RELOAD on interface object
-PASS PerformanceNavigation interface: constant TYPE_RELOAD on interface prototype object
-PASS PerformanceNavigation interface: constant TYPE_BACK_FORWARD on interface object
-PASS PerformanceNavigation interface: constant TYPE_BACK_FORWARD on interface prototype object
-PASS PerformanceNavigation interface: constant TYPE_RESERVED on interface object
-PASS PerformanceNavigation interface: constant TYPE_RESERVED on interface prototype object
+FAIL PerformanceNavigation interface: constant TYPE_NAVIGATE on interface object assert_equals: property has wrong value expected (string) "0" but got (number) 0
+FAIL PerformanceNavigation interface: constant TYPE_NAVIGATE on interface prototype object assert_equals: property has wrong value expected (string) "0" but got (number) 0
+FAIL PerformanceNavigation interface: constant TYPE_RELOAD on interface object assert_equals: property has wrong value expected (string) "1" but got (number) 1
+FAIL PerformanceNavigation interface: constant TYPE_RELOAD on interface prototype object assert_equals: property has wrong value expected (string) "1" but got (number) 1
+FAIL PerformanceNavigation interface: constant TYPE_BACK_FORWARD on interface object assert_equals: property has wrong value expected (string) "2" but got (number) 2
+FAIL PerformanceNavigation interface: constant TYPE_BACK_FORWARD on interface prototype object assert_equals: property has wrong value expected (string) "2" but got (number) 2
+FAIL PerformanceNavigation interface: constant TYPE_RESERVED on interface object assert_equals: property has wrong value expected (string) "255" but got (number) 255
+FAIL PerformanceNavigation interface: constant TYPE_RESERVED on interface prototype object assert_equals: property has wrong value expected (string) "255" but got (number) 255
 PASS PerformanceNavigation interface: attribute type
 PASS PerformanceNavigation interface: attribute redirectCount
 PASS PerformanceNavigation must be primary interface of window.performance.navigation
 PASS Stringification of window.performance.navigation
-PASS PerformanceNavigation interface: window.performance.navigation must inherit property "TYPE_NAVIGATE" with the proper type
-PASS PerformanceNavigation interface: window.performance.navigation must inherit property "TYPE_RELOAD" with the proper type
-PASS PerformanceNavigation interface: window.performance.navigation must inherit property "TYPE_BACK_FORWARD" with the proper type
-PASS PerformanceNavigation interface: window.performance.navigation must inherit property "TYPE_RESERVED" with the proper type
+FAIL PerformanceNavigation interface: window.performance.navigation must inherit property "TYPE_NAVIGATE" with the proper type assert_equals: expected (string) "0" but got (number) 0
+FAIL PerformanceNavigation interface: window.performance.navigation must inherit property "TYPE_RELOAD" with the proper type assert_equals: expected (string) "1" but got (number) 1
+FAIL PerformanceNavigation interface: window.performance.navigation must inherit property "TYPE_BACK_FORWARD" with the proper type assert_equals: expected (string) "2" but got (number) 2
+FAIL PerformanceNavigation interface: window.performance.navigation must inherit property "TYPE_RESERVED" with the proper type assert_equals: expected (string) "255" but got (number) 255
 PASS PerformanceNavigation interface: window.performance.navigation must inherit property "type" with the proper type
 PASS PerformanceNavigation interface: window.performance.navigation must inherit property "redirectCount" with the proper type
 FAIL Performance interface: existence and properties of interface object assert_equals: prototype of self's property "Performance" is not Function.prototype expected function "function () { [native code] }" but got function "function EventTarget() { [native code] }"

@gsnedders, can you tell if the old or the new state is the correct one?

@foolip
Copy link
Member

foolip commented Nov 20, 2017

Hmm, after a quick look at https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/tests/w3c/webperf/approved/navigation-timing/html/idlharness.html I'm pretty sure this is a regression. These shouldn't be strings, they should be numbers.

@gsnedders
Copy link
Member Author

@foolip That to me looks like something's wrong on the Chromium side; loading http://web-platform.test:8000/navigation-timing/idlharness.html locally WFM. (Also should probably get rid of those copies of the tests in the Chromium tree.) Did you update idlharness.js when doing the import to run that CQ? 881cd41 changes it in the exact way that would fix this, and last I knew idlharness.js (and testharness.js) weren't auto-imported it.

@foolip
Copy link
Member

foolip commented Nov 21, 2017

@gsnedders I only updated the copies of things inside LayoutTests/external/wpt, I'll try also syncing the other idlharness.js.

@foolip
Copy link
Member

foolip commented Nov 21, 2017

@gsnedders
Copy link
Member Author

@foolip so does this look okay to you then?

@foolip
Copy link
Member

foolip commented Nov 21, 2017

Sure, it might break our import but I'm on rotation now and can handle it.

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.

7 participants