| Summary: | Modern IDB: Add -private.html variants of crypto/subtle IndexedDB tests | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Brady Eidson <beidson> | ||||||
| Component: | WebCore Misc. | Assignee: | Brady Eidson <beidson> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | achristensen, ap, commit-queue | ||||||
| Priority: | P2 | ||||||||
| Version: | Safari 9 | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 149117 | ||||||||
| Attachments: |
|
||||||||
|
Description
Brady Eidson
2016-01-21 14:28:09 PST
Created attachment 270555 [details]
Patch v1
No additional changes needed. Confirmed via testing that the -private.html versions are using the in-memory backing store, and are also passing.
Comment on attachment 270555 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=270555&action=review > LayoutTests/resources/js-test-pre.js:5 > + // If the test file URL ends in "-private.html", enable private browsing. I would really really prefer if we didn't add more magic paths and filenames. (In reply to comment #2) > Comment on attachment 270555 [details] > Patch v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=270555&action=review > > > LayoutTests/resources/js-test-pre.js:5 > > + // If the test file URL ends in "-private.html", enable private browsing. > > I would really really prefer if we didn't add more magic paths and filenames. Already established practice at this point; js-test.js already does this. Instead of making the change to js-test-pre.js, I could just make these tests include js-test.js also. It seems more undesirable to me to have the source of the two copies of the test *not* be identical. Do you have any other suggestions? > Already established practice at this point Yes, and I'd like to reduce that, not add more of such abuse. > It seems more undesirable to me to have the source of the two copies of the test *not* be identical. Why? These copies test different things, so the more apparent the difference is the better. Putting it directly into .html is vastly preferable to hiding it inside a helper script. (In reply to comment #4) > > Already established practice at this point > > Yes, and I'd like to reduce that, not add more of such abuse. I don't even mean "magic filenames." I mean *this specific* magic filename is *already* in wide use. > > It seems more undesirable to me to have the source of the two copies of the test *not* be identical. > > Why? Because the tests are expected to be identical. > These copies test different things, They copy identical JS APIs. The fact that the storage backend is different is irrelevant to the text of the test. > Putting it directly into .html is vastly preferable to hiding > it inside a helper script. This is a statement of opinion presented as fact. I happen to disagree. (In reply to comment #5) > (In reply to comment #4) > > > These copies test different things, > > They copy identical JS APIs. > s/copy/test/ > I don't even mean "magic filenames." I mean *this specific* magic filename is *already* in wide use. That is unfortunate, and this patch makes it even more unfortunate. Let's undo what has been done, instead of continuing the trend. > This is a statement of opinion presented as fact. I happen to disagree. Please feel free to take the discussion to webkit-dev. I strongly believe that this is the wrong approach. The logic seems super straightforward: anyone who will want to find what the difference between these tests is will do a diff, and will be super confused. We should avoid confusing WebKit engineers. (In reply to comment #7) > > This is a statement of opinion presented as fact. I happen to disagree. > > Please feel free to take the discussion to webkit-dev. I strongly believe > that this is the wrong approach. I strongly believe that it is less wrong than the suggested alternative, but I also don't need to resolve this right now, so I'll punt until a later date. Created attachment 280516 [details]
Patch
Comment on attachment 280516 [details] Patch Clearing flags on attachment: 280516 Committed r201685: <http://trac.webkit.org/changeset/201685> All reviewed patches have been landed. Closing bug. |