Closed
Bug 1100695
Opened 9 years ago
Closed 9 years ago
testChild() in selftest.py doesn't wait for child process to start up before shutting down the parent process
Categories
(Testing :: XPCShell Harness, defect)
Testing
XPCShell Harness
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla36
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
Attachments
(2 files)
887 bytes,
patch
|
Details | Diff | Splinter Review | |
1.53 KB,
patch
|
chmanchester
:
review+
|
Details | Diff | Splinter Review |
In bug 1091766, I'm changing the behavior of some IPC code to keep running when we hit an error, rather than just calling exit(0) immediately. One issue it has exposed is that the child process in testChild() crashes in startup, because it can't send a message to the parent. It looks like it doesn't start running until after ContentParent has done ActorDestroy, so when the child process tries to send a message to the ContentParent, it fails, which causes us to abort in SendPNeckoConstructor. Changing the body of add_test in CHILD_HARNESS_SIMPLE to do_load_child_test_harness(); do_test_pending("some child"); sendCommand("Assert.ok(true);", do_test_finished); run_next_test(); ...seems to fix the crash, but I don't understand what this test is supposed to test, so maybe this is making it so we don't test anything.
Comment 1•9 years ago
|
||
This is testing the reference error I introduced and fixed in bug 1093834 -- the version above still tests that. Thanks for tracking that down, I had no idea that introduced a crash.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → continuation
Assignee | ||
Comment 2•9 years ago
|
||
When I change the test to be as in comment 0, I get a new failure: 0:20.03 TEST-UNEXPECTED-FAIL | No test output (missing mozunit.main() call?): ./testing/xpcshell/selftest.py This is generated by mach itself: http://mxr.mozilla.org/mozilla-central/source/python/mach_commands.py#109 This is weird because the log is running some code in the child process that does a TEST- thing, but somehow mach isn't seeing it? If you uncomment the line that calls _testLogger.error in this patch, then you get a different error: 0:19.86 TEST-UNEXPECTED-FAIL | /var/folders/j7/00m_17816s5_1b4pl8crf6p00000gn/T/tmpZUBzbi/test_child_assertions.js | xpcshell return code: 0 It is expected that it fails, because I added a failure, but the weird thing is that now we don't get the original error about no test output. I added some logging, and in this case, I think mach does see the trivial passing test that the sendCommand is supposed to do: TEST-PASS | /var/folders/j7/00m_17816s5_1b4pl8crf6p00000gn/T/tmpZUBzbi/test_child_assertions.js | None - true == true So it seems like maybe the child process messages are being eating somewhere in the logging infrastructure in a way that mach doesn't see?
Assignee | ||
Comment 3•9 years ago
|
||
Would you mind looking into this, Chris? It seems to involve a bunch of layers of our test harness framework that I'm entirely unfamiliar with. Failing that, maybe we could disable this for now? I don't know how much danger there is in this regressing.
Flags: needinfo?(cmanchester)
Comment 4•9 years ago
|
||
I'll look into this later this afternoon. I think that's all explained by the fact that process output is suppressed in runxpcshelltests.py if there's no failure, and maybe mach is doing further analysis of output that's slightly spurious in this case. I usually run these with make check, which is what our test infrastructure does. What command are you using to invoke these tests? Just |./mach python testing/xpcshell/selftest.py| ?
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Chris Manchester [:chmanchester] from comment #4) > I'll look into this later this afternoon. Thanks! > I think that's all explained by > the fact that process output is suppressed in runxpcshelltests.py if there's > no failure, and maybe mach is doing further analysis of output that's > slightly spurious in this case. That sounds very plausible. I was trying to figure out how to just dump something in the output to make mach happy, but I couldn't come up with anything that worked. > I usually run these with make check, which is what our test infrastructure > does. What command are you using to invoke these tests? Just |./mach python > testing/xpcshell/selftest.py| ? Ah, so maybe it could land like this without turning the tree orange. I can do a try push. Though this extra mach check seems weird. I'm running it with ./mach python-test testing/xpcshell/selftest.py
Comment 6•9 years ago
|
||
Looks like mach's python-test is doing some extra log parsing (http://hg.mozilla.org/mozilla-central/file/4e8f29e386bd/python/mach_commands.py#l95) that causes the failure output. 'print "TEST-"' in selftest.py placates that. Output from the child process otherwise seems to work ok.
Flags: needinfo?(cmanchester)
Assignee | ||
Comment 7•9 years ago
|
||
Thanks! That ends up printing something extra to the console, but I think that isn't a big deal, given how few people probably even run this test locally.
Assignee | ||
Comment 8•9 years ago
|
||
the unit test part of this try run looks fine: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=82f7ce409c07
Attachment #8526961 -
Flags: review?(cmanchester)
Comment 9•9 years ago
|
||
Comment on attachment 8526961 [details] [diff] [review] Wait for child to finish in test_child_assert XPCShell unit test. Review of attachment 8526961 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/xpcshell/selftest.py @@ +483,5 @@ > self.assertEquals(0, self.x.todoCount) > self.assertInLog(TEST_PASS_STRING) > self.assertNotInLog(TEST_FAIL_STRING) > > + print TEST_PASS_STRING + " This is a fake pass message to stop a bogus mach failure" I didn't mean to imply this should actually be added, although I'm sure it doesn't hurt anything. My conclusion was that these tests shouldn't be run with the mach python-test target (mach python or make check work fine).
Attachment #8526961 -
Flags: review?(cmanchester) → review+
Assignee | ||
Comment 10•9 years ago
|
||
Oh, oops. I'll play around with that.
Assignee | ||
Comment 11•9 years ago
|
||
(In reply to Chris Manchester [:chmanchester] from comment #9) > I didn't mean to imply this should actually be added, although I'm sure it > doesn't hurt anything. My conclusion was that these tests shouldn't be run > with the mach python-test target (mach python or make check work fine). Oh, I think I re-misunderstood you. Basically, you are saying that I should land the patch I have here, except without the print statement, and then just live with the fact that running this test via mach python-test will produce a failure? I can do that, and then just add a comment to the effect that this test will fail when run that way. And I guess file a bug about it against mach.
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(cmanchester)
Assignee | ||
Comment 12•9 years ago
|
||
I filed bug 1103226 on the mach issue.
Comment 13•9 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #11) > (In reply to Chris Manchester [:chmanchester] from comment #9) > > I didn't mean to imply this should actually be added, although I'm sure it > > doesn't hurt anything. My conclusion was that these tests shouldn't be run > > with the mach python-test target (mach python or make check work fine). > > Oh, I think I re-misunderstood you. Basically, you are saying that I should > land the patch I have here, except without the print statement, and then > just live with the fact that running this test via mach python-test will > produce a failure? I can do that, and then just add a comment to the effect > that this test will fail when run that way. And I guess file a bug about it > against mach. Exactly. Sorry for the confusion. I've never run across the python-test target before, but it looks like it expects output from a test runner that selftest.py doesn't use. I see the error with or without your patch when running from python-test, it seems unrelated to this bug.
Updated•9 years ago
|
Flags: needinfo?(cmanchester)
Assignee | ||
Comment 14•9 years ago
|
||
Thanks for your help. I removed the print and added a reference to the bug I filed. https://hg.mozilla.org/integration/mozilla-inbound/rev/f86a92a28ec9
Comment 15•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f86a92a28ec9
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•