test_ipc.html does not notice failures

RESOLVED FIXED in mozilla18

Status

()

Core
DOM: IndexedDB
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ms2ger, Assigned: Ben Turner (not reading bugmail, use the needinfo flag!))

Tracking

Trunk
mozilla18
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

5 years ago
Component: DOM → DOM: IndexedDB
QA Contact: general → indexeddb

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 769017
(Reporter)

Updated

5 years ago
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
(Reporter)

Updated

5 years ago
Duplicate of this bug: 769017
Assignee: khuey → bent.mozilla
(Reporter)

Comment 3

5 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=13312476&tree=Mozilla-Inbound
(Reporter)

Comment 4

5 years ago
I'm going to disable test_ipc.html if this isn't fixed by tomorrow.
(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.
https://hg.mozilla.org/mozilla-central/rev/353b4785836a
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9c45b6a8dfb
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 8

5 years ago
Then fix it yourself, as bent is too lazy to do his job.
Assignee: bent.mozilla → khuey
(Reporter)

Comment 9

5 years ago
And get your facts in order, bent is a peer.
(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.
(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).
Assignee: khuey → bent.mozilla
Severity: critical → normal
Target Milestone: mozilla17 → ---
(Reporter)

Comment 12

5 years ago
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.
(Reporter)

Comment 13

5 years ago
Disabled again, since clearly nothing is going to happen otherwise

https://hg.mozilla.org/integration/mozilla-inbound/rev/77e5cdc7af9a
https://hg.mozilla.org/mozilla-central/rev/77e5cdc7af9a
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [test disabled][leave open]
([leave open] needs to be in the whiteboard or the merging tool will close the bug)
(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
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?
Aren't the imported tests before indexedDB?  Are you sure that there's correlation here and not just coincidence?
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?
Whiteboard: [test disabled][leave open]
(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.
(Reporter)

Comment 21

5 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=14744784&tree=Mozilla-Inbound
(Reporter)

Comment 22

5 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=14747063&tree=Mozilla-Inbound
(Reporter)

Comment 23

5 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=14747236&tree=Firefox
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.
Attachment #655653 - Flags: review?(khuey)
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.
Attachment #655653 - Flags: review?(khuey) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/28cbe2f75f5d
https://hg.mozilla.org/mozilla-central/rev/28cbe2f75f5d
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: mozilla17 → mozilla18
You need to log in before you can comment on or make changes to this bug.