Closed Bug 770046 Opened 8 years ago Closed 8 years ago

test_ipc.html does not notice failures

Categories

(Core :: Storage: IndexedDB, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: Ms2ger, Assigned: bent.mozilla)

References

Details

Attachments

(1 file)

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.
Component: DOM → DOM: IndexedDB
QA Contact: general → indexeddb
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 769017
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Duplicate of this bug: 769017
Assignee: khuey → bent.mozilla
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
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Then fix it yourself, as bent is too lazy to do his job.
Assignee: bent.mozilla → khuey
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 → ---
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.
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
Closed: 8 years ago8 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.
Attached patch Patch, v1Splinter Review
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/mozilla-central/rev/28cbe2f75f5d
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: mozilla17 → mozilla18
You need to log in before you can comment on or make changes to this bug.