Adam Guthrie Reporter
	Description • 18 years ago

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

	Dave Liebreich [:davel]
	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

	Dietrich Ayala (:dietrich) 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.

	Dietrich Ayala (:dietrich) Assignee
	Comment 3 • 18 years ago

converted old SS tests to new mochikit-based tests. also added a couple of new tests.

	Robert Sayre
	Comment 4 • 18 years ago

Comment on attachment 245596 [details] [diff] [review]
wip patch

r=sayrer for the tests.

	Adam Guthrie 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.

	Adam Guthrie 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...

	Robert Sayre
	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.

	Robert Sayre
	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.

	Simon Bünzli
	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.

	Adam Guthrie 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.

	Simon Bünzli
	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!

	Phil Ringnalda (:philor)
	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