528145 - under e10s have head.js indicate which process is printing xpcshell status messages

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Jason Duell]

Attachment: prints either "Child:" or "Parent:" before head.js status messages, when run under e10s.

This patch prints either "Child:" or "Parent:" before head.js status messages, when run under e10s.

Comment 1

[Jeff Walden [:Waldo]]

Comment on attachment 411907 [details] [diff] [review]
prints either "Child:" or "Parent:" before head.js status messages, when run under e10s. 

>diff --git a/testing/xpcshell/head.js b/testing/xpcshell/head.js

>+function _dump(str) {
>+  if (typeof _XPCSHELL_PROCESS == "undefined") {
>+    dump(str);
>+  } else {
>+    dump(_XPCSHELL_PROCESS + ": " + str);
>+  }
>+}

Single-line-if bracing isn't kosher; don't brace either part of this statement.


>-  while (!_quit)
>+  while (!_quit) {
>     thr.processNextEvent(true);
>-
>+  }

Don't brace this either.

I kind of feel like we should have a better name than _dump, but nothing immediately springs to mind.

Comment 2

[Jason Duell]

Attachment: Fixes error detection in runxpcshell

This version teaches runxpcshell to search for optional "parent|child :" before TEST-FAIL-UNEXPECTED.  Without that, error detection is broken.

> Single-line-if bracing isn't kosher

I refer you to the following statement in the Mozilla Coding Style page...

"Always brace controlled statements, even a single-line consequent of an if else else. This is redundant typically, but it avoids dangling else bugs, so it's safer at scale than fine-tuning."

I don't actually agree with this for single-line "if"s (so I took that one out), but I have personally lost time debugging unbraced if-else, so I left that one in.  I can take them out if you still object!

Comment 3

[Jason Duell]

Attachment: Fix minor bitrot

The latest patch to bug 521922 broke the context for last patch.  Fixed.

Comment 4

[Jeff Walden [:Waldo]]

Comment on attachment 415470 [details] [diff] [review]
Fix minor bitrot

>   if (truePassedChecks > 0)
>-    dump("TEST-PASS | (xpcshell/head.js) | " + truePassedChecks + " (+ " +
>+    _dump("TEST-PASS | (xpcshell/head.js) | " + truePassedChecks + " (+ " +
>             _falsePassedChecks + ") check(s) passed\n");
>   else
>     // ToDo: switch to TEST-UNEXPECTED-FAIL when all tests have been updated. (Bug 496443)
>-    dump("TEST-INFO | (xpcshell/head.js) | No (+ " + _falsePassedChecks + ") checks actually run\n");
>+    _dump("TEST-INFO | (xpcshell/head.js) | No (+ " + _falsePassedChecks + ") checks actually run\n");

I don't remember what this file usually has done (which is why this is only a suggestion), but my (gradually evolving) style sense says both of these are actually multi-line bodies (even the second with a comment for one line and a statement for the other), and therefore both dumps would be braced.  If you don't want to change it that's fine, leaving it as is should be good enough for now.

Comment 5

[Jason Duell]

Attachment: updated with braces around some more if statements

Comment 6

[Jason Duell]

This is ready to be checked into the /projects/electrolysis as soon as (or in tandem with) the patch for bug 521922.

Comment 7

[Jason Duell]

Attachment: Fix bitrot on approved patch: ready for commit

Comment 8

[Jason Duell]

http://hg.mozilla.org/projects/electrolysis/rev/2efb17a8af65

Comment 9

[Jason Duell]

Attachment: Version checked in (had runxpcshell.js fix that got lost during various bitrot updates)