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

Status: NEW

(Core :: Networking, defect, P5)

Description

[Jason Duell]

Attachment: wrappers for necko tests to run in child process under e10s

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?

Comment 1

[Jae-Seong Lee-Russo]

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.

Comment 2

[Jae-Seong Lee-Russo]

Attachment: Copy data/necessary files to unit_ipc

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.

Comment 3

[Jason Duell]

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.

Comment 4

[Jae-Seong Lee-Russo]

Attachment: Handling /data files

Depends on Bug 528145.

Comment 5

[Jason Duell]

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.

Jason


> 
> 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 
> <https://bugzilla.mozilla.org/show_bug.cgi?id=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 https://developer.mozilla.org/en/Writing_xpcshell-based_unit_tests for an explanation of head_ and tail_ scripts).

Comment 6

[Jae-Seong Lee-Russo]

(In reply to comment #5)
> So are you finding that all the tests succeed with a debug build once you copy
> the /data directory?
Yes.

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 http://hg.mozilla.org/projects/electrolysis, 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?

Comment 7

[Jae-Seong Lee-Russo]

Attachment: test_aboutblank_wrap.js on debug vs non-debug

Wish I could see printouts from 'child' on non-debug build.

Comment 8

[Jason Duell]

CC-ing Ted in case he has better ideas on how to dupe/link to a Data directory than I do.

Comment 9

[Jason Duell]

Comment on attachment 420593 [details] [diff] [review]
Handling /data files

Jae-Seong,

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.

Comment 10

[(not currently active) Ted Mielczarek]

You can write a libs:: rule and copy whatever data you want into the xpcshell tests directory. Here's an example:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/test/Makefile.in#72

Comment 11

[Jae-Seong Lee-Russo]

Attachment: Use libs:: rule to copy data

Comment 12

[Jae-Seong Lee-Russo]

Attachment: libs:: should be inside MOZ_IPC

Comment 13

[Chris Peterson [:cpeterson]]

Mass tracking-e10s flag change. Filter bugmail on "2be0fcce-e36a-4e2c-aa80-0e3d33eb5406".

Comment 14

[Firefox Bug Husbandry Bot]

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258