Closed
Bug 350525
Opened 18 years ago
Closed 18 years ago
New session restore API needs accompanying unit tests
Categories
(Firefox :: Session Restore, defect)
Firefox
Session Restore
Tracking
()
RESOLVED
FIXED
People
(Reporter: ispiked, Assigned: dietrich)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
13.63 KB,
patch
|
sayrer
:
review+
|
Details | Diff | Splinter Review |
Bug 344640 added a lot of new API functionality to session restore. I think it would be cool if we could get unit tests for these APIs. Something similar to what Dietrich did here would be nice: http://lxr.mozilla.org/seamonkey/source/browser/components/sessionstore/test/nsSessionStoreTest.xul
Comment 1•18 years ago
|
||
a quick question - does anyone know how nsSessionStoreTest.xul was designed to be invoked? I'd very much appreciate a recipe using a standard firefox build, and listing all preferences that need to be set and how exactly to open this doc. Thanks _Dave
Assignee | ||
Comment 2•18 years ago
|
||
(In reply to comment #1) > a quick question - does anyone know how nsSessionStoreTest.xul was designed to > be invoked? I'd very much appreciate a recipe using a standard firefox build, > and listing all preferences that need to be set and how exactly to open this > doc. > > Thanks > > _Dave > Hey Dave! I was opening it via chrome://, after putting it in an extension's content dir. This was only because the infrastructure for the system-wide unit tests wasn't in place. Is the infrastructure for this ready? If so, I'll convert the SS tests to use it.
Assignee | ||
Comment 3•18 years ago
|
||
converted old SS tests to new mochikit-based tests. also added a couple of new tests.
Comment 4•18 years ago
|
||
Comment on attachment 245596 [details] [diff] [review] wip patch r=sayrer for the tests.
Attachment #245596 -
Flags: review?(sayrer) → review+
Updated•18 years ago
|
Component: Tabbed Browser → Session Restore
Updated•18 years ago
|
QA Contact: tabbed.browser → session.restore
Reporter | ||
Comment 5•18 years ago
|
||
Here's an updated patch to tie this into the existing Mochitest framework. I removed nsSessionStoreTest.xul since test_bug350525.xul is basically a Mochitest-ized version of it. I've tested this and it works.
Attachment #245596 -
Attachment is obsolete: true
Attachment #266788 -
Flags: review?(sayrer)
Reporter | ||
Comment 6•18 years ago
|
||
Oh yeah, this test doesn't used the waitForExplicitFinish function like some of the other chrome tests do, but it works. I guess this is because the script finishes before onload fires...
Comment 7•18 years ago
|
||
Comment on attachment 266788 [details] [diff] [review] updated patch these look ok to me, but dietrich is probably the person to ask about session restore stuff.
Attachment #266788 -
Flags: review?(sayrer)
Attachment #266788 -
Flags: review?(dietrich)
Attachment #266788 -
Flags: review+
Comment 8•18 years ago
|
||
Comment on attachment 266788 [details] [diff] [review] updated patch Adam tells me nothing really changed in the tests. works for me.
Attachment #266788 -
Flags: review?(dietrich)
Comment 9•18 years ago
|
||
undoCloseTab tests seem to have been added. There's one caveat with those: you can't easily compare the number of reopenable tabs before and after closing a tab - if that list already has the length browser.sessionstore.max_tabs_undo, it won't get any longer. Might be better to check that the previously closed tab (i.e. the first in the list) indeed contains the URL opened in it... of course, ensuring that getClosedTabCount <= browser.sessionstore.max_tabs_undo wouldn't hurt, either.
Reporter | ||
Comment 10•18 years ago
|
||
Simon, should that really block this patch's landing, or could it be filed as a follow-up bug? I'd really like to get these tests running, even if they're not perfect.
Comment 11•18 years ago
|
||
You'll get useful results from the test under the condition that they will run in a browser with default settings (.max_tabs_undo = 10) and that todo(...) doesn't throw when newcount == count (i.e. when the number of closed tabs already maxed out). Given that, please go ahead and land these tests!
Reporter | ||
Updated•18 years ago
|
Whiteboard: [checkin needed]
Comment 12•18 years ago
|
||
browser/components/sessionstore/Makefile.in 1.3 browser/components/sessionstore/test/Makefile.in 1.1 browser/components/sessionstore/test/nsSessionStoreTest.xul delete browser/components/sessionstore/test/chrome/Makefile.in 1.1 browser/components/sessionstore/test/chrome/test_bug350525.xul 1.1
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
Updated•18 years ago
|
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•