Eliminate last uses of assert_throws. #21762
Conversation
wpt-pr-bot
requested review from
agouaillard,
alvestrand,
andrenatal,
andypaicu,
annevk,
asutherland,
domenic and
domfarolino
wpt-pr-bot
requested review from
TakayoshiKochi,
titzer,
wanderview,
yoavweiss,
youennf,
yutakahirano,
zcorpan and
zqzhang
and removed request for
marcoscaceres,
mfalken,
odinho,
plehegar,
rniwa and
sideshowbarker
|
Looking at the Firefox stability failures:
so that should not block this landing. Looking at the experimental wpt.fyi results:
I'll see about fixing the import-map test. |
bzbarsky
force-pushed
the
remove-assert-throws
branch
from
f0bf9d0
to
eeada6d
Compare
| assert_throws(new SyntaxError(), () => { i.states.add(''); }); | ||
| assert_throws('InvalidCharacterError', () => { i.states.add('a\tb'); }); | ||
| assert_throws_js(TypeError, () => { i.states.supports('foo'); }); | ||
| assert_throws_dom('SyntaxError', () => { i.states.add(''); }); |
There was a problem hiding this comment.
It might be worth pointing out that this also fixes correctness of test as new SyntaxError() seems wrong. (Same for some other files.)
There was a problem hiding this comment.
Pointing out in the commit message? But yes, in general that was the whole point of the assert_throws changes...
There was a problem hiding this comment.
I thought the main point was asserting the correct global, not also changing some incorrect JS SyntaxError to "SyntaxError" DOMException.
There was a problem hiding this comment.
Fwiw, the main point for me when I started this was catching places where people threw bogus DOMException TypeErrors instead of JS TypeErrors. The JS-vs-DOM SyntaxError is basically the same thing.
There was a problem hiding this comment.
Oh, I didn't realize the state of affairs was that bad. Wow, well I'm even more glad you fixed this then, thanks so much!
The one remaining use inside promise_rejects will go away soon when promise_rejects goes away.
bzbarsky
force-pushed
the
remove-assert-throws
branch
from
eeada6d
to
5b62147
Compare
|
Have we checked if webkit is ready for the removal? @rniwa ? |
|
So I should go ahead and admin merge this? |
|
@stephenmcgruer are we ready for this change in Chromium? |
|
Not exactly, but we can merge it without breaking us. We keep a separate copy of testharness.js for internal tests, and roll that manually, so we just can't roll till we migrate internal tests. Currently we have ~1200 https://crbug.com/1051932 tracks Chromium |
|
Currently in WebKit we are not running automated imports of the imported WPT tests. |
The one remaining use inside promise_rejects will go away soon when
promise_rejects goes away.