Closed Bug 732657 Opened 8 years ago Closed 8 years ago

runxpcshell.py doesn't fail e10s tests if child process aborts/crashes

Categories

(Testing :: XPCShell Harness, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla13

People

(Reporter: jduell.mcbugs, Assigned: jduell.mcbugs)

Details

Attachments

(2 files, 1 obsolete file)

Attached patch test caseSplinter Review
Trying running this with "make check-one SOLO_FILE=test_traceable_channel_wrap.js" and you'll see the log has an ABORT message but runxpcshell reports that everything is happy happy.
Attached patch Fix v1 (obsolete) — Splinter Review
I don't know if there's a better way to deal with this, but this fixes things by searching for START/COMPLETE log msgs from child processes--if we START but don't COMPLETE, considers it a failure.
Attachment #602568 - Flags: review?(ted.mielczarek)
Attachment #602568 - Flags: review?(ted.mielczarek) → review?(jmaher)
Comment on attachment 602568 [details] [diff] [review]
Fix v1

Review of attachment 602568 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for doing this.

::: testing/xpcshell/runxpcshelltests.py
@@ +662,5 @@
> +                                            re.MULTILINE)) or
> +                      (stdout and re.search("^child: CHILD-TEST-STARTED", 
> +                                            stdout, re.MULTILINE) 
> +                              and not re.search("^child: CHILD-TEST-COMPLETED",
> +                                                stdout, re.MULTILINE)))

oh man, this is a nasty looking condition.  Is there anyway to split this up?  Just reading this I am not sure if the logic is correcct.  It seems that this is a list of all failure conditions, maybe a comment to help decipher it?
Attachment #602568 - Flags: review?(jmaher) → review+
how about this?  I just added comments.  We could split it up, but honestly, I'm not sure how much easier it'd be to follow (I don't find it that bad :)
Attachment #602568 - Attachment is obsolete: true
Attachment #603984 - Flags: review?(jmaher)
Comment on attachment 603984 [details] [diff] [review]
v2: now with comments

Review of attachment 603984 [details] [diff] [review]:
-----------------------------------------------------------------

thanks.  The comments help break it up.
Attachment #603984 - Flags: review?(jmaher) → review+
Try was green, so it doesn't appear this bug was actually masking any test failures.  How nice!  Still best to remove luck from the process :)

https://hg.mozilla.org/integration/mozilla-inbound/rev/0681b89e5be9
https://hg.mozilla.org/mozilla-central/rev/0681b89e5be9
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.