Bug 105927

Summary: Add tests for WebIDL type conversions
Product: WebKit Reporter: Erik Arvidsson <arv>
Component: Tools / TestsAssignee: Joshua Bell <jsbell>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, jsbell, roger_fong, sam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Erik Arvidsson 2013-01-02 09:34:35 PST
Maybe we could add an object to the testRunner whose sole purpose is to test the type conversions required by WebIDL

interface TestWebIDLConversions {
  long attribute testLong;
  unsigned long attribute testUnsignedLong;
  [EnforceRange] long attribute testEnforceRangeLong;
  ...
}
Comment 1 Sam Weinig 2013-01-02 10:46:20 PST
This seems like a great idea to me.
Comment 2 Adam Barth 2013-01-02 10:52:26 PST
We could also add them to Internals if that's easier.
Comment 3 Joshua Bell 2013-01-02 17:07:48 PST
Created attachment 181113 [details]
Patch
Comment 4 Joshua Bell 2013-01-02 17:08:28 PST
First stab at tests. Can land this, then extend it with EnforceRange support and fix long long conversion issues over in bug 96798
Comment 5 Erik Arvidsson 2013-01-02 18:56:45 PST
Comment on attachment 181113 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=181113&action=review

I was imagining having a sub object on window.internals so that we don't pollute it with a bunch of properties that most people do not care about.

> LayoutTests/fast/js/webidl-type-mapping.html:3
> +<script src="script-tests/webidl-type-mapping.js"></script>

Just inline the js here
Comment 6 Build Bot 2013-01-02 19:09:22 PST
Comment on attachment 181113 [details]
Patch

Attachment 181113 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15627729

New failing tests:
fast/js/webidl-type-mapping.html
Comment 7 Joshua Bell 2013-01-03 11:55:31 PST
Created attachment 181198 [details]
Patch
Comment 8 Joshua Bell 2013-01-03 11:56:39 PST
(In reply to comment #5)
> (From update of attachment 181113 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=181113&action=review
> 
> I was imagining having a sub object on window.internals so that we don't pollute it with a bunch of properties that most people do not care about.

Done.

> > LayoutTests/fast/js/webidl-type-mapping.html:3
> > +<script src="script-tests/webidl-type-mapping.js"></script>
> 
> Just inline the js here

Done.

(In reply to comment #6)
> (From update of attachment 181113 [details])
> Attachment 181113 [details] did not pass mac-ews (mac):
> Output: http://queues.webkit.org/results/15627729
> 
> New failing tests:
> fast/js/webidl-type-mapping.html

Apparently we're not consistent. I'm trying to get a WebKit build together to see what the difference is between JSC and V8.
Comment 9 Erik Arvidsson 2013-01-03 12:06:52 PST
Comment on attachment 181198 [details]
Patch

LGTM... Please get a reviewer to r+ this.
Comment 10 Joshua Bell 2013-01-03 15:53:00 PST
Created attachment 181243 [details]
Patch for landing
Comment 11 Joshua Bell 2013-01-03 15:56:44 PST
(In reply to comment #8)
> 
> Apparently we're not consistent. I'm trying to get a WebKit build together to see what the difference is between JSC and V8.

+/-Inf/NaN and even Number.MAX_VALUE conversions were different for [unsigned] long long in a handful of cases. So far as I can tell from the WebIDL spec those should all yield 0. For now I just disabled those tests.
Comment 12 WebKit Review Bot 2013-01-03 17:17:15 PST
Comment on attachment 181243 [details]
Patch for landing

Rejecting attachment 181243 [details] from commit-queue.

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
Full output: http://queues.webkit.org/results/15666201
Comment 13 WebKit Review Bot 2013-01-04 12:43:56 PST
Comment on attachment 181243 [details]
Patch for landing

Clearing flags on attachment: 181243

Committed r138836: <http://trac.webkit.org/changeset/138836>
Comment 14 WebKit Review Bot 2013-01-04 12:44:00 PST
All reviewed patches have been landed.  Closing bug.
Comment 16 Roger Fong 2013-01-04 15:43:37 PST
reopening until build is fixed
Comment 17 Joshua Bell 2013-01-04 15:59:49 PST
(In reply to comment #16)
> reopening until build is fixed

Landed what I think is the fix in: http://trac.webkit.org/changeset/138864