Closed Bug 1059940 Opened 5 years ago Closed 5 years ago

Plugin crash tests do not cleanup CrashManager

Categories

(Core :: Plug-ins, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: poiru, Assigned: poiru)

References

Details

Attachments

(4 files, 4 obsolete files)

Tests such as test_crash_submit.xul don't cleanup CrashManager and so leave crash records lingering around.
Assignee: nobody → birunthan
Status: NEW → ASSIGNED
Attachment #8480755 - Flags: review?(benjamin)
Attachment #8480759 - Attachment is obsolete: true
Attachment #8480759 - Flags: review?(benjamin)
Attachment #8480761 - Flags: review?(benjamin)
I'm checking for `ok(!!store.crashes.length)` rather than `is(store.crashes.length, 1)` as there are probably other tests as well that fail to cleanup. After they are all fixed, I'll change these to check for a specific number.
Attachment #8480764 - Flags: review?(benjamin)
Updated the patch to apply the fixes to crash_hang_submit.xul as well.
Attachment #8480758 - Attachment is obsolete: true
Attachment #8480758 - Flags: review?(benjamin)
Attachment #8480920 - Flags: review?(benjamin)
Updated the patch to apply the fixes to crash_hang_submit.xul as well.
Attachment #8480761 - Attachment is obsolete: true
Attachment #8480761 - Flags: review?(benjamin)
Attachment #8480921 - Flags: review?(benjamin)
Updated the patch to apply the fixes to browser_pluginCrashCommentAndURL.js as well.
Attachment #8480764 - Attachment is obsolete: true
Attachment #8480764 - Flags: review?(benjamin)
Attachment #8480922 - Flags: review?(benjamin)
Do we really care about this? Might it be better to let this data into the crash manager, and if there is a test that specifically cares about testing the crash manager, it should explicitly reset it?
Flags: needinfo?(birunthan)
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #9)
> Do we really care about this? Might it be better to let this data into the
> crash manager, and if there is a test that specifically cares about testing
> the crash manager, it should explicitly reset it?

Yeah, we can do that. Could you still review Part 1, though?
Flags: needinfo?(birunthan)
Attachment #8480920 - Flags: review?(benjamin)
Attachment #8480921 - Flags: review?(benjamin)
Attachment #8480922 - Flags: review?(benjamin)
Attachment #8480755 - Flags: review?(benjamin) → review+
https://hg.mozilla.org/mozilla-central/rev/b97cbea6f8b8
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.