Open Bug 526335 Opened 13 years ago Updated 3 months ago

Run full set of necko child-side xpcshell tests under electrolysis


(Core :: Networking, defect, P5)

Other Branch



Tracking Status
e10s + ---


(Reporter: jduell.mcbugs, Unassigned)


(Blocks 1 open bug)


(Whiteboard: [necko-would-take])


(3 files, 3 obsolete files)

I've written a set of wrappers that run the full set of existing necko xpcshell tests in the child process rather than chrome.  This patch is only for the e10s branch, and relies on the patch for bug 521922 to work.

Alas, at least one of the tests seems to be hanging at some point (i.e they run fine in chrome, but hang when run in the child process).  I haven't had time to investigate.  Jae-Seong, are you interested?
Yes, I am.  It will take hours to download and build the branch, so I will start working on the problem tomorrow, if it is okay.
I built the e10s with --enable-debug.  I created this patch to copy necessary files to unit_ipc, but I couldn't create a patch for an empty file (data/test_readline1.txt).

All the IPC tests run/pass with warnings, and no hang.  So I ran the tests after building e10s with --disable-debug.

Some tests seemed to run fine, but then the log shows that the tests (the child process) did not actually run.  Some tests hang randomly.
Attachment #410477 - Flags: review?
We should look into this some more.  I'd like to make sure the necko xpcshell tests all run in "vanilla" mode on the child before we start breaking them with e10s changes.  We may also need to think about how best to handle the /data files some tests need; copying them leaves litter.  Perhaps we can point httpd.js at the original data directory.
Blocks: fennecko
Attached patch Handling /data files (obsolete) — Splinter Review
Depends on Bug 528145.
Attachment #410477 - Attachment is obsolete: true
Attachment #410477 - Flags: review?
n 01/05/2010 08:24 AM, Jae-Seong Lee-Russo wrote:
>> I'd like to make sure the necko xpcshell tests all run in "vanilla" mode on the child before we start breaking them with e10s changes.
> Does "vanilla" mode mean using the non-debug version?  If not, please 
> explain and ignore the rest.

Sorry, by "vanilla" I just mean "not using e10s", i.e. using the existing necko implementation (once in each process), rather than the shared version we're developing for e10s.   Having the full version means everything works as it always does, but since each process has a totally separate necko layer, things that ought to be shared (cookies, etc.) aren't, which is wrong, and also it takes up lots of memory (the NSS library for HTTPS is big, and so is the HTTP cache).

The reason I want to test this "vanilla" version first is because that version of necko is really stable (i.e. it's the version we use everyday in the browser).   That way if things are busted, we can guess it's probably something wrong with how we're connecting xpcshell between the processes, or things like the "/data" directory being missing (thanks for finding this).

Sorry to take a few days to get back to you--things have been busy.


> test_reopen_wrap.js was hanging because a /data file was missing, and 
> the test was the only one hanging.  I can see all the necko tests run 
> fine on the debug build, but I am not sure on the non-debug one.  Is 
> there anyway to see the message printouts from the child process on the 
> non-debug build?

I actually haven't ever run the tests in non-debug mode.

So are you finding that all the tests succeed with a debug build once you copy the /data directory?  If so that's good news.

>> Perhaps we can point httpd.js at the original data directory.
> I don't see how you use httpd.js here.  Could you explain more detail?  

Yeah that was probably a bad idea.

> I was thinking of changing the do_get_file in head.js to use the 
> _XPCSHELL_PROCESS like you did on the _dump function (Bug 528145 
> <>).

Whatever works is probably OK. Copying the whole /data directory into test_ipc at test time is fine, I'm guessing, so long as we delete it once we're done.   Perhaps we could do the copy in a 'head_copy_data.js' file and then delete it in a "tail_delete_data.js" file. (see for an explanation of head_ and tail_ scripts).
(In reply to comment #5)
> So are you finding that all the tests succeed with a debug build once you copy
> the /data directory?

Just to make sure that I did right: I applied patches for bug 521922, bug 528145, and the 2 patches for this bug 526335 to the code pulled from, built from there, and ran the tests.

I did not copy the /data directory because of the patch in comment #4.  Can I ask you to review it or can I ask Jeff?
Wish I could see printouts from 'child' on non-debug build.
Attachment #420593 - Flags: review?(jduell.mcbugs)
CC-ing Ted in case he has better ideas on how to dupe/link to a Data directory than I do.
Comment on attachment 420593 [details] [diff] [review]
Handling /data files


I don't like hardcoding in the "../unit" convention into head.js.  If we could create a 'test/unit_ipc/head_copy_data.js' file that simply copies the entire unit/data directory into unit_ipc, and a "tail_purge_data.js" that deletes it when the tests are done, things ought to simply work, right?  It's a bit of a hack, but it keeps the change more local.

It's too bad we can't simply use a symbolic link, but I don't think we can use them on Windows.

It looks like we can simply create an nsIFile object for the "unit/Data" directory, and then use file.CopyTo() to copy it recursively into unit_ipc.  And then we can use Remove(PR_TRUE) to delete it when we're done.
Attachment #420593 - Flags: review?(jduell.mcbugs) → review-
You can write a libs:: rule and copy whatever data you want into the xpcshell tests directory. Here's an example:
Attached patch Use libs:: rule to copy data (obsolete) — Splinter Review
Attachment #420593 - Attachment is obsolete: true
Attachment #424949 - Flags: review?(jduell.mcbugs)
Blocks: 547809
Assignee: nobody → lusian
Attachment #424949 - Attachment is obsolete: true
Attachment #428408 - Flags: review?(jduell.mcbugs)
Attachment #424949 - Flags: review?(jduell.mcbugs)
Mass tracking-e10s flag change. Filter bugmail on "2be0fcce-e36a-4e2c-aa80-0e3d33eb5406".
tracking-e10s: --- → +
Attachment #428408 - Flags: review?(jduell.mcbugs)
Whiteboard: [necko-would-take]
Bulk change to priority:
Priority: -- → P5
Assignee: lusian → nobody
You need to log in before you can comment on or make changes to this bug.