Bug 165554 - [WebCrypto] Migrate some tests from webkitSubtle to subtle
Summary: [WebCrypto] Migrate some tests from webkitSubtle to subtle
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jiewen Tan
URL:
Keywords:
Depends on:
Blocks: 160880
  Show dependency treegraph
 
Reported: 2016-12-07 15:02 PST by Jiewen Tan
Modified: 2016-12-12 17:33 PST (History)
3 users (show)

See Also:


Attachments
Patch (30.09 KB, patch)
2016-12-07 15:16 PST, Jiewen Tan
no flags Details | Formatted Diff | Diff
Patch for landing (30.09 KB, patch)
2016-12-12 15:30 PST, Jiewen Tan
jiewen_tan: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jiewen Tan 2016-12-07 15:02:03 PST
Replicate some webkitSubtle tests for subtle to increase test coverage for subtle.
Comment 1 Jiewen Tan 2016-12-07 15:16:09 PST
Created attachment 296427 [details]
Patch
Comment 2 Brent Fulgham 2016-12-08 08:29:28 PST
Comment on attachment 296427 [details]
Patch

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

I think you did a move, when you just meant to copy. r- unless I misunderstood the point of this patch.

> LayoutTests/ChangeLog:3
> +        [WebCrypto] Replicate some webkitSubtle tests for subtle

You say "replicate" in the patch notes, but you appear to be doing a 'svn mv', which gets rid of the old "webkitSubtle" version. Was this intentional?
Comment 3 Jiewen Tan 2016-12-08 12:25:02 PST
Comment on attachment 296427 [details]
Patch

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

>> LayoutTests/ChangeLog:3
>> +        [WebCrypto] Replicate some webkitSubtle tests for subtle
> 
> You say "replicate" in the patch notes, but you appear to be doing a 'svn mv', which gets rid of the old "webkitSubtle" version. Was this intentional?

Yes. For those two tests, the intention is to test CryptoKey interface instead of SubtleCrypto/WebkitSubtleCrypto. Therefore, it doesn't matter which SubtleCrypto interface is used to generate the key.
Comment 4 Brent Fulgham 2016-12-12 13:47:38 PST
Comment on attachment 296427 [details]
Patch

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

Looks good. Please revise the title as I suggest to make it clearer that you are "moving" some 'webkitSubtle' tests to use the new 'subtle' API.

>>> LayoutTests/ChangeLog:3
>>> +        [WebCrypto] Replicate some webkitSubtle tests for subtle
>> 
>> You say "replicate" in the patch notes, but you appear to be doing a 'svn mv', which gets rid of the old "webkitSubtle" version. Was this intentional?
> 
> Yes. For those two tests, the intention is to test CryptoKey interface instead of SubtleCrypto/WebkitSubtleCrypto. Therefore, it doesn't matter which SubtleCrypto interface is used to generate the key.

I suggest that you change the title to "Migrate some tests from webkitSubtle to subtle"

> LayoutTests/platform/ios-simulator-wk1/TestExpectations:1027
> +crypto/subtle/rsa-indexeddb.html

Do we know why these tests don't work on the simulator? Does the SDK lack some features we need? We should probably file some Radars if that is the case.
Comment 5 Jiewen Tan 2016-12-12 15:02:55 PST
Comment on attachment 296427 [details]
Patch

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

Thanks Brent for r+ my patch.

>>>> LayoutTests/ChangeLog:3
>>>> +        [WebCrypto] Replicate some webkitSubtle tests for subtle
>>> 
>>> You say "replicate" in the patch notes, but you appear to be doing a 'svn mv', which gets rid of the old "webkitSubtle" version. Was this intentional?
>> 
>> Yes. For those two tests, the intention is to test CryptoKey interface instead of SubtleCrypto/WebkitSubtleCrypto. Therefore, it doesn't matter which SubtleCrypto interface is used to generate the key.
> 
> I suggest that you change the title to "Migrate some tests from webkitSubtle to subtle"

Fixed.

>> LayoutTests/platform/ios-simulator-wk1/TestExpectations:1027
>> +crypto/subtle/rsa-indexeddb.html
> 
> Do we know why these tests don't work on the simulator? Does the SDK lack some features we need? We should probably file some Radars if that is the case.

I believe it is just indexedDB is not available in iOS WK1.
Comment 6 Jiewen Tan 2016-12-12 15:30:04 PST
Created attachment 296959 [details]
Patch for landing
Comment 7 Jiewen Tan 2016-12-12 17:33:33 PST
Committed r209749: <http://trac.webkit.org/changeset/209749>