Closed
Bug 1244723
Opened 9 years ago
Closed 9 years ago
xpcshell should shutdown profile after all the registered cleanup functions, not before them
Categories
(Testing :: XPCShell Harness, defect)
Testing
XPCShell Harness
Tracking
(firefox47 fixed)
RESOLVED
FIXED
mozilla47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: mak, Assigned: mak)
Details
(Keywords: dev-doc-complete)
Attachments
(2 files)
do_get_profile registers profile shutdown topics through do_register_cleanup, but by doing so the function will be invoked before the cleanup functions registered by the test... Instead it should be the last one, after the test cleanup functions. We should probably not use do_register_cleanup, instead store it separately and invoke it before we walk the clenaup functions array.
Assignee | ||
Comment 1•9 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #0) > We should probably not use do_register_cleanup, instead store it separately > and invoke it before we walk the clenaup functions array. ehr, that should have been "after".
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Assignee | ||
Updated•9 years ago
|
Summary: xpcshell should run its cleanup after all the other functions registered through do_register_cleanup → xpcshell should shutdown profile after all the registered cleanup functions, not before them
Assignee | ||
Comment 2•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/33035/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33035/
Attachment #8714343 -
Flags: review?(ted)
Assignee | ||
Comment 3•9 years ago
|
||
looks like Try is showing some errors to be fixed. Many (or all of them?) are "Unable to find a rejection expected by expectUncaughtRejection" that sounds like a positive thing! Will check later and add another changeset on top.
Assignee | ||
Comment 4•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/33247/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/33247/
Attachment #8714843 -
Flags: review?(ted)
Comment 5•9 years ago
|
||
Comment on attachment 8714343 [details] MozReview Request: Bug 1244723 - xpcshell should shutdown profile after all the registered cleanup functions, not before them. r=ted https://reviewboard.mozilla.org/r/33035/#review30025 Well that was silly. Good catch!
Attachment #8714343 -
Flags: review?(ted) → review+
Comment 6•9 years ago
|
||
Comment on attachment 8714843 [details] MozReview Request: Bug 1244723 - Remove some of the expected uncaught exceptions not happening anymore. r=ted https://reviewboard.mozilla.org/r/33247/#review30027
Attachment #8714843 -
Flags: review?(ted) → review+
Assignee | ||
Comment 8•9 years ago
|
||
The second part is not needed anymore since bug 989960 has temporarily been backed out.
Comment 9•9 years ago
|
||
Good to see less chances for error. There's no way to register an asynchronous cleanup function to be executed after profile shutdown notifications anymore (only synchronous through the observers), but I think this won't likely be needed in practice. Remember to update the documentation of do_get_profile at: https://developer.mozilla.org/en-US/docs/Mozilla/QA/Writing_xpcshell-based_unit_tests
Keywords: dev-doc-needed
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to :Paolo Amadini from comment #9) > Good to see less chances for error. There's no way to register an > asynchronous cleanup function to be executed after profile shutdown > notifications anymore (only synchronous through the observers), but I think > this won't likely be needed in practice. you can add an observer to do that, if needed, and you can use async shutdown tooling to wait for async functions. the scope of cleanup functions is cleanup, not really do testing of shutdown or such. > Remember to update the documentation of do_get_profile at: > > https://developer.mozilla.org/en-US/docs/Mozilla/QA/Writing_xpcshell- > based_unit_tests what should I update there? it still looks correct to me.
Assignee | ||
Updated•9 years ago
|
Keywords: dev-doc-needed → dev-doc-complete
Comment 12•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b49ff0115100
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in
before you can comment on or make changes to this bug.
Description
•