Last Comment Bug 770046 - test_ipc.html does not notice failures
: test_ipc.html does not notice failures
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: IndexedDB (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla18
Assigned To: Ben Turner (not reading bugmail, use the needinfo flag!)
:
Mentors:
: 769017 (view as bug list)
Depends on:
Blocks: 677964
  Show dependency treegraph
 
Reported: 2012-07-01 12:14 PDT by :Ms2ger (⌚ UTC+1/+2)
Modified: 2012-08-27 19:17 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch, v1 (11.72 KB, patch)
2012-08-27 11:00 PDT, Ben Turner (not reading bugmail, use the needinfo flag!)
khuey: review+
Details | Diff | Splinter Review

Description :Ms2ger (⌚ UTC+1/+2) 2012-07-01 12:14:51 PDT
test_ipc.html overrides TestRunner.log, but fails to override TestRunner.error. This leads to failures not being counted and test runs going green on TBPL even when tests fail. See <https://tbpl.mozilla.org/php/getParsedLog.php?id=13147301&full=1&branch=mozilla-inbound> for an example.

Kyle, please fix ASAP.
Comment 1 Justin Wood (:Callek) 2012-07-01 12:18:57 PDT

*** This bug has been marked as a duplicate of bug 769017 ***
Comment 2 :Ms2ger (⌚ UTC+1/+2) 2012-07-01 12:20:39 PDT
*** Bug 769017 has been marked as a duplicate of this bug. ***
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2012-07-15 03:46:14 PDT
I'm going to disable test_ipc.html if this isn't fixed by tomorrow.
Comment 5 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-07-16 06:57:07 PDT
(In reply to :Ms2ger from comment #4)
> I'm going to disable test_ipc.html if this isn't fixed by tomorrow.
By attaching a patch and requesting a review from an IndexedDB peer? Great. Otherwise, please don't.

I'm on vacation this week and won't have any time to do this, but the benefit of running these tests and exercising this code on all platforms far outweighs missing some random orange logging for broken child process behavior. We have found plenty of broken stuff form logging just the parent side already.
Comment 6 Ed Morley [:emorley] 2012-07-18 05:57:09 PDT
https://hg.mozilla.org/mozilla-central/rev/353b4785836a
Comment 7 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-07-18 06:01:00 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9c45b6a8dfb
Comment 8 :Ms2ger (⌚ UTC+1/+2) 2012-07-18 06:06:17 PDT
Then fix it yourself, as bent is too lazy to do his job.
Comment 9 :Ms2ger (⌚ UTC+1/+2) 2012-07-18 06:09:51 PDT
And get your facts in order, bent is a peer.
Comment 10 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2012-07-18 06:17:22 PDT
(In reply to :Ms2ger from comment #0)
> test_ipc.html overrides TestRunner.log, but fails to override
> TestRunner.error. This leads to failures not being counted and test runs
> going green on TBPL even when tests fail. See
> <https://tbpl.mozilla.org/php/getParsedLog.
> php?id=13147301&full=1&branch=mozilla-inbound> for an example.
> 
> Kyle, please fix ASAP.

onTestMessage tries to handle this with a regex, but it probably isn't the right regex.  It probably would make sense to transmit both message and error and map them to things more like exact parallels.
Comment 11 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-07-23 14:58:04 PDT
(In reply to :Ms2ger from comment #8)
As I tried to explain in comment 5 this is not a very serious issue. It is simply one of many bugs that I need to fix. We're currently catching some, but not all, test failures. You propose that we should disable this test. Doing so would of course mean that we would not catch *any* failures. I don't see how that helps.

I'm currently working on other bugs which are higher priority but I will fix this as soon as I have the time to do so (unless someone else wants to fix it first, of course).
Comment 12 :Ms2ger (⌚ UTC+1/+2) 2012-07-24 02:41:10 PDT
It's a five-line patch. I don't understand why you want to waste time arguing about the fix rather that doing the work.
Comment 13 :Ms2ger (⌚ UTC+1/+2) 2012-08-06 01:51:47 PDT
Disabled again, since clearly nothing is going to happen otherwise

https://hg.mozilla.org/integration/mozilla-inbound/rev/77e5cdc7af9a
Comment 14 Ed Morley [:emorley] 2012-08-06 07:41:55 PDT
https://hg.mozilla.org/mozilla-central/rev/77e5cdc7af9a
Comment 15 Ed Morley [:emorley] 2012-08-06 08:11:51 PDT
([leave open] needs to be in the whiteboard or the merging tool will close the bug)
Comment 16 Johnny Stenback (:jst, jst@mozilla.com) 2012-08-07 09:19:11 PDT
(In reply to :Ms2ger from comment #13)
> Disabled again, since clearly nothing is going to happen otherwise
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/77e5cdc7af9a

Ms2ger, you disabled these tests *again* w/o following due process. As many people have already told you, this is on bent's list of things to do very soon. It's just that there are other things ahead if this in his immediate priority queue and disabling these tests isn't going to change that priority queue or help these tests get fixed any faster.

https://hg.mozilla.org/mozilla-central/rev/99fb6977d46c
Comment 17 Ed Morley [:emorley] 2012-08-13 06:48:00 PDT
The politics around this test disabling aside; the weird thing is that when the test was re-enabled in comment 16 - native Android mochitest-3 went permaorange with bug 781837. Do we have any ideas why?
Comment 18 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-08-13 06:51:13 PDT
Aren't the imported tests before indexedDB?  Are you sure that there's correlation here and not just coincidence?
Comment 19 Ed Morley [:emorley] 2012-08-13 07:01:11 PDT
Press down arrow once on:
https://tbpl.mozilla.org/?noignore=1&tree=Mozilla-Inbound&jobname=Android%20Tegra%20250%20mozilla-inbound%20opt%20test%20mochitest-3&rev=2dfbe4b9403b

Same for:
https://tbpl.mozilla.org/?jobname=Android Tegra 250 mozilla-central opt test mochitest-3&noignore=1&rev=2dfbe4b9403b

The second is not so clear visually (you have to ignore the test_event_propagation.html failures), but both seem to confirm that re-enabling test_ipc.html somehow turned bug 781837 permaorange - unless I'm missing something?
Comment 20 Ed Morley [:emorley] 2012-08-17 03:32:37 PDT
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #18)
> Aren't the imported tests before indexedDB?  Are you sure that there's
> correlation here and not just coincidence?

Yes I am now.

Filed bug 783513.
Comment 24 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-08-27 11:00:40 PDT
Created attachment 655653 [details] [diff] [review]
Patch, v1

This fixes the failure case and makes sure that the harness fails if it isn't run in a child process.
Comment 25 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-08-27 11:02:07 PDT
Comment on attachment 655653 [details] [diff] [review]
Patch, v1

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

Please separate the test_ipc fixes and the test_blob stuff.
Comment 26 Ben Turner (not reading bugmail, use the needinfo flag!) 2012-08-27 12:50:58 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/28cbe2f75f5d
Comment 27 Ryan VanderMeulen [:RyanVM] 2012-08-27 19:17:56 PDT
https://hg.mozilla.org/mozilla-central/rev/28cbe2f75f5d

Note You need to log in before you can comment on or make changes to this bug.