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

idlharness.js's assert_type_is needs to be taught about each interface #23346

Closed
inexorabletash opened this issue Apr 30, 2020 · 3 comments · Fixed by #23754
Closed

idlharness.js's assert_type_is needs to be taught about each interface #23346

inexorabletash opened this issue Apr 30, 2020 · 3 comments · Fixed by #23754

Comments

@inexorabletash
Copy link
Member

inexorabletash commented Apr 30, 2020

As noted in e.g. #23323 (comment) the idlharness assertion assert_type_is() needs to be explicitly made aware of interfaces like Float32Array and DataView. If an interface isn't listed, it leads to unexpected failures.

We likely haven't noticed this much because most idlharness tests are boilerplate and don't provide sample values. Even after the above PR, grepping for the error in blink's output, I found some geometry-related fails: " Unrecognized type" --> DOMRect, DOMRectReadOnly, DOMPointReadOnly, DOMMatrix

Can we update assert_type_is() to have value instanceof self[type] as the default case? (Honest question: I didn't look deeply enough to see what the downsides are)

cc: @foolip

@domenic
Copy link
Member

domenic commented May 1, 2020

This is similar to (but not quite the same as) #23329, in particular your reference to the built-in types like Float32Array.

Note that those types aren't interfaces, though.

@inexorabletash
Copy link
Member Author

Yeah, cc: @stephenmcgruer too

@stephenmcgruer
Copy link
Contributor

That seems like a reasonable default to me; I can own landing that change.

stephenmcgruer added a commit that referenced this issue May 28, 2020
* Add default case for non-member types in assert_type_is

Fixes #23346

* Update comment
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 1, 2020
… assert_type_is, a=testonly

Automatic update from web-platform-tests
Add default case for non-member types in assert_type_is (#23754)

* Add default case for non-member types in assert_type_is

Fixes web-platform-tests/wpt#23346

* Update comment
--

wpt-commits: d94c7aa1d9d3e311043729741df287a980df296a
wpt-pr: 23754
xeonchen pushed a commit to xeonchen/gecko that referenced this issue Jun 2, 2020
… assert_type_is, a=testonly

Automatic update from web-platform-tests
Add default case for non-member types in assert_type_is (#23754)

* Add default case for non-member types in assert_type_is

Fixes web-platform-tests/wpt#23346

* Update comment
--

wpt-commits: d94c7aa1d9d3e311043729741df287a980df296a
wpt-pr: 23754
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants