Summary: | Combine local and session storage tests into one and use script-tests properly | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jeremy Orlow <jorlow> | ||||||||
Component: | New Bugs | Assignee: | Jeremy Orlow <jorlow> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | darin, dglazkov, ojan | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 30535 | ||||||||||
Attachments: |
|
Description
Jeremy Orlow
2009-10-21 11:27:28 PDT
Created attachment 41590 [details]
Patch v1
Why doesn't using a TEMPLATE.html file in domstorage/script-tests work for this? (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? (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. After talking to Ojan, it sounded like it was best to simply do what Darin originally suggested. Created attachment 41607 [details]
Patch v1
Created attachment 41608 [details]
Patch v1
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? Comment on attachment 41608 [details]
Patch v1
Looks good. r=me
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. (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> (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. 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. (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. (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. :-) (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. |