Closed Bug 732657 Opened 8 years ago Closed 8 years ago doesn't fail e10s tests if child process aborts/crashes


(Testing :: XPCShell Harness, defect)

Not set


(Not tracked)



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



(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/
@@ +662,5 @@
> +                                            re.MULTILINE)) or
> +                      (stdout and"^child: CHILD-TEST-STARTED", 
> +                                            stdout, re.MULTILINE) 
> +                              and not"^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 :)
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.