Bug 153323

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.