RESOLVED FIXED 30640
Combine local and session storage tests into one and use script-tests properly 
https://bugs.webkit.org/show_bug.cgi?id=30640
Summary Combine local and session storage tests into one and use script-tests properly
Jeremy Orlow
Reported 2009-10-21 11:27:28 PDT
In https://bugs.webkit.org/show_bug.cgi?id=30535 Darin pointed out that I'm not using "script-tests" correctly. Apparently the tool assumes that all layout tests that correspond to a test in a script-tests directory is under its control. The dom storage layout tests require a small modification to the .html in order to have session storage and local storage tests share the same JavaScript file. I think the best solution for now is to move them back to a resources directory. Darin has suggested that maybe we shouldn't have seprate tests for session storage and local storage, but I'm not positive whether this is the best route to take yet since there are some places where session and local storage diverge (like in quota support).
Attachments
Patch v1 (22.20 KB, patch)
2009-10-21 11:30 PDT, Jeremy Orlow 		no flags
DetailsFormatted DiffDiff
Patch v1 (62.95 KB, patch)
2009-10-21 13:46 PDT, Jeremy Orlow 		no flags
DetailsFormatted DiffDiff
Patch v1 (60.91 KB, patch)
2009-10-21 13:48 PDT, Jeremy Orlow 		darin: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Jeremy Orlow
Comment 1 2009-10-21 11:30:26 PDT
Created attachment 41590 [details] Patch v1
Ojan Vafai
Comment 2 2009-10-21 11:55:47 PDT
Why doesn't using a TEMPLATE.html file in domstorage/script-tests work for this?
Jeremy Orlow
Comment 3 2009-10-21 12:10:56 PDT
(In reply to comment #2) > Why doesn't using a TEMPLATE.html file in domstorage/script-tests work for > this? I have no idea. Where's documentation on how to use this script-tests stuff?
Jeremy Orlow
Comment 4 2009-10-21 12:14:27 PDT
(In reply to comment #3) > (In reply to comment #2) > > Why doesn't using a TEMPLATE.html file in domstorage/script-tests work for > > this? > > I have no idea. > > Where's documentation on how to use this script-tests stuff? Oh...well if the template is tied to the script-tests directory it's not going to work because the customization is passing in either localStorage or sessionStorage. If the template is tied to the directory where the HTML exists then we might be able to make this work. The thing is that the directory structure is as follows: domstorage/script-tests domstorage/localstorage domstorage/sessionstorage The latter 2 directories are where the .html files exist. The html files pass in either "localStorage" or "sessionStorage" to the runTest function in the .js files. I suspect that this is different enough from the rest of the script tests that we can't trivially re-use the infrastructure. And, unless this is going to be useful elsewhere, I don't see a point to adapting the infrastructure to this use case.
Jeremy Orlow
Comment 5 2009-10-21 13:46:22 PDT
After talking to Ojan, it sounded like it was best to simply do what Darin originally suggested.
Jeremy Orlow
Comment 6 2009-10-21 13:46:56 PDT
Created attachment 41607 [details] Patch v1
Jeremy Orlow
Comment 7 2009-10-21 13:48:39 PDT
Created attachment 41608 [details] Patch v1
Ojan Vafai
Comment 8 2009-10-21 15:34:15 PDT
One nit: In your template, <script src="../../fast/js/resources/js-test-post-function.js"></script> should go after the runTest call. Is there a problem with that?
Darin Adler
Comment 9 2009-10-21 15:35:39 PDT
Comment on attachment 41608 [details] Patch v1 Looks good. r=me
Jeremy Orlow
Comment 10 2009-10-21 15:43:01 PDT
Ojan: I (In reply to comment #8) > One nit: > > In your template, <script > src="../../fast/js/resources/js-test-post-function.js"></script> should go > after the runTest call. Is there a problem with that? This won't work since the runTest call ends up calling it before the parser will load that file.
Ojan Vafai
Comment 11 2009-10-21 15:56:08 PDT
(In reply to comment #10) > Ojan: I (In reply to comment #8) > > One nit: > > > > In your template, <script > > src="../../fast/js/resources/js-test-post-function.js"></script> should go > > after the runTest call. Is there a problem with that? > > This won't work since the runTest call ends up calling it before the parser > will load that file. That seems fine. Now that you don't need to pass in the storage type from the TEMPLATE.html file, you could reduce the boilerplate needed for new storage JS tests, and be more consistent with most other JS tests. There's no need to have a runTest function at all. Just run the actual JS at the end of the test and set window.successfullyParsed = true. Then instead of using js-test-post-function.js in the TEMPLATE.html file, use js-test-post.js. <html> <head> <link rel="stylesheet" href="../../fast/js/resources/js-test-style.css"> <script src="../../fast/js/resources/js-test-pre.js"></script> </head> <body> <p id="description"></p> <div id="console"></div> <script src="YOUR_JS_FILE_HERE"></script> <script src="../../fast/js/resources/js-test-post.js"></script> </body> </html>
Jeremy Orlow
Comment 12 2009-10-21 16:43:45 PDT
(In reply to comment #11) > There's no need to have a runTest function at all. Just run the actual JS at > the end of the test I've changed my code to do it this way. > and set window.successfullyParsed = true. Then instead of > using js-test-post-function.js in the TEMPLATE.html file, use js-test-post.js. This is not possible since half the local storage tests involve events. This is why I need to use js-test-post-function. But I need to include it first because the other half don't and make the call before the page is fully loaded.
Jeremy Orlow
Comment 13 2009-10-21 16:48:00 PDT
Submitted because this is blocking my other change, but Ojan let me know if you have any further suggestions and I'll address them in future patches.
Ojan Vafai
Comment 14 2009-10-21 18:11:35 PDT
(In reply to comment #12) > (In reply to comment #11) > > and set window.successfullyParsed = true. Then instead of > > using js-test-post-function.js in the TEMPLATE.html file, use js-test-post.js. > > This is not possible since half the local storage tests involve events. This > is why I need to use js-test-post-function. But I need to include it first > because the other half don't and make the call before the page is fully loaded. Ah. I looked at the patch for cases like this and didn't see any. That makes sense then. Thanks for humoring my nits.
Jeremy Orlow
Comment 15 2009-10-21 18:21:20 PDT
(In reply to comment #14) > (In reply to comment #12) > > (In reply to comment #11) > > > and set window.successfullyParsed = true. Then instead of > > > using js-test-post-function.js in the TEMPLATE.html file, use js-test-post.js. > > > > This is not possible since half the local storage tests involve events. This > > is why I need to use js-test-post-function. But I need to include it first > > because the other half don't and make the call before the page is fully loaded. > > Ah. I looked at the patch for cases like this and didn't see any. That makes > sense then. Thanks for humoring my nits. The change that needs it is the one that depends on this. :-)
Jeremy Orlow
Comment 16 2009-10-21 18:21:46 PDT
(In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #12) > > > (In reply to comment #11) > > > > and set window.successfullyParsed = true. Then instead of > > > > using js-test-post-function.js in the TEMPLATE.html file, use js-test-post.js. > > > > > > This is not possible since half the local storage tests involve events. This > > > is why I need to use js-test-post-function. But I need to include it first > > > because the other half don't and make the call before the page is fully loaded. > > > > Ah. I looked at the patch for cases like this and didn't see any. That makes > > sense then. Thanks for humoring my nits. > > The change that needs it is the one that depends on this. :-) Er...the _test_ that needs this is in the _bug_ that depends on this.
Note You need to log in before you can comment on or make changes to this bug.