Bug 136878

Summary: Introduce Promise A+ tests into WebKit
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Yusuke Suzuki 2014-09-16 18:35:22 PDT
To ensure WebKit Promise stability, introducing Promise A+ tests[1] into WebKit.
It's also used to test V8 Promise[2].
So the tests guarantee that WebKit Promise's spec validity and interchangeability to the other browser's Promise implementation.

[1]: https://github.com/promises-aplus/promises-tests
[2]: https://code.google.com/p/v8/source/browse#svn%2Ftrunk%2Ftest%2Fpromises-aplus
Comment 1 Yusuke Suzuki 2014-09-18 04:41:50 PDT
Created attachment 238306 [details]
Patch
Comment 2 Yusuke Suzuki 2014-09-18 04:46:00 PDT
Comment on attachment 238306 [details]
Patch

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

> LayoutTests/ChangeLog:23
> +        So marked the test as a SLOW test.

I'm not sure how to introduce the external code into WebKit. Added license files.
Sam, could you tell me the mannar to do it? I'll fix it :)
Comment 3 Yusuke Suzuki 2014-09-18 13:21:55 PDT
Comment on attachment 238306 [details]
Patch

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

> LayoutTests/ChangeLog:17
> +        So added SinonJS[4] and browserify's license files into the test directory.

As mentioned here, it uses browserify to transform Node.js style code into browser style code (using sync `require`).
Is it OK? I'd like to hear your opinion.
Comment 4 Sam Weinig 2014-09-30 17:13:52 PDT
Comment on attachment 238306 [details]
Patch

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

>> LayoutTests/ChangeLog:23
>> +        So marked the test as a SLOW test.
> 
> I'm not sure how to introduce the external code into WebKit. Added license files.
> Sam, could you tell me the mannar to do it? I'll fix it :)

How slow is it?
Comment 5 Sam Weinig 2014-10-01 12:35:26 PDT
(In reply to comment #4)
> (From update of attachment 238306 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=238306&action=review
> 
> >> LayoutTests/ChangeLog:23
> >> +        So marked the test as a SLOW test.
> > 
> > I'm not sure how to introduce the external code into WebKit. Added license files.
> > Sam, could you tell me the mannar to do it? I'll fix it :)
> 
> How slow is it?

Actually, can we split this into multiple tests so that each is not that slow? Otherwise, this all looks good.
Comment 6 Yusuke Suzuki 2014-10-01 12:36:28 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 238306 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=238306&action=review
> > 
> > >> LayoutTests/ChangeLog:23
> > >> +        So marked the test as a SLOW test.
> > > 
> > > I'm not sure how to introduce the external code into WebKit. Added license files.
> > > Sam, could you tell me the mannar to do it? I'll fix it :)
> > 
> > How slow is it?
> 
> Actually, can we split this into multiple tests so that each is not that slow? Otherwise, this all looks good.

Thank you for your review!
Maybe I think we can split it into several files. So I'll do so :)
Comment 7 Yusuke Suzuki 2014-10-02 07:59:24 PDT
Created attachment 239108 [details]
Patch
Comment 8 Yusuke Suzuki 2014-10-02 08:03:26 PDT
Comment on attachment 239108 [details]
Patch

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

I've updated the patch. In this patch, I've splitted the whole test into per-file tests.

> LayoutTests/ChangeLog:23
> +        So mark it as a Slow test.

I've splitted the whole tests into per file tests. So almost all tests become fast.
However, only tests-2-3-3 has a lot of test cases and it sometimes exceeds the time limit.
For example, in my environment (GTK debug build on Ubuntu 14.04), it takes about 13s.
So marked it as a Slow test.
Comment 9 WebKit Commit Bot 2014-10-03 19:41:49 PDT
Comment on attachment 239108 [details]
Patch

Clearing flags on attachment: 239108

Committed r174307: <http://trac.webkit.org/changeset/174307>
Comment 10 WebKit Commit Bot 2014-10-03 19:41:52 PDT
All reviewed patches have been landed.  Closing bug.