Open
Bug 1025740
Opened 10 years ago
Updated 2 years ago
UMimTypProvider should be registered in some tests
Categories
(MailNews Core :: Testing Infrastructure, defect)
MailNews Core
Testing Infrastructure
Tracking
(Not tracked)
NEW
People
(Reporter: hiro, Unassigned)
Details
(Whiteboard: [patchlove])
Attachments
(3 files)
2.03 KB,
patch
|
standard8
:
review-
|
Details | Diff | Splinter Review |
5.36 KB,
patch
|
Details | Diff | Splinter Review | |
3.23 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
Otherwise other tests might be affected the UMimTypProvider.
Reporter | ||
Comment 1•10 years ago
|
||
Attachment #8440515 -
Flags: review?(standard8)
Reporter | ||
Comment 2•10 years ago
|
||
Attachment #8440516 -
Flags: review?(standard8)
Comment 3•10 years ago
|
||
Comment on attachment 8440515 [details] [diff] [review] Fix I believe every xpcshell test is run with its own profile and code, so unless this is making global changes (which would be dangerous), this shouldn't be necessary. However, if we did want to do this, I would suggest simply registering the cleanup function in mailTestUtils.registerUMimTypProvider as then it never gets forgotten.
Attachment #8440515 -
Flags: review?(standard8) → review-
Comment 4•10 years ago
|
||
Comment on attachment 8440516 [details] [diff] [review] Fix other tests which need UMimTypeProvider individually See previous comments. Plus also, I don't understand why this is suddenly needed for these tests?
Attachment #8440516 -
Flags: review?(standard8)
Reporter | ||
Comment 5•10 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #4) > Comment on attachment 8440516 [details] [diff] [review] > Fix other tests which need UMimTypeProvider individually > > See previous comments. Plus also, I don't understand why this is suddenly > needed for these tests? Because I am still seeing the following errors on those tests running on my local machine: TEST-UNEXPECTED-FAIL | ../../../resources/logHelper.js | Error console says [stackFrame NS_ERROR_FAILURE: Failure'Failure' when calling method: [nsIDirectoryServiceProvider::getFile]] - See following stack: So I am suspecting that UMimTypProvider still exists when those tests run on build servers.
Comment 6•10 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #5) > (In reply to Mark Banner (:standard8) from comment #4) > > Comment on attachment 8440516 [details] [diff] [review] > > Fix other tests which need UMimTypeProvider individually > > > > See previous comments. Plus also, I don't understand why this is suddenly > > needed for these tests? ... > So I am suspecting that UMimTypProvider still exists when those tests run on > build servers. The alternative is that there's some mime type that is being looked up for those tests that the test machines have defined outside of mimeTypes.rdf and that you don't. This is unlikely to be affected by the unregistering (you should be able to prove that by pushing that unregister patch to try). So I would be happy adding the register functions to fix the tests, but I don't think we need the unregister.
Reporter | ||
Updated•10 years ago
|
Summary: UMimTypProvider should be unregistered when test is finished → UMimTypProvider should be registered in some tests
Reporter | ||
Comment 7•10 years ago
|
||
Attachment #8440946 -
Flags: review?(standard8)
Reporter | ||
Comment 8•10 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #6) > (In reply to Hiroyuki Ikezoe (:hiro) from comment #5) > > (In reply to Mark Banner (:standard8) from comment #4) > > > Comment on attachment 8440516 [details] [diff] [review] > > > Fix other tests which need UMimTypeProvider individually > > > > > > See previous comments. Plus also, I don't understand why this is suddenly > > > needed for these tests? > ... > > So I am suspecting that UMimTypProvider still exists when those tests run on > > build servers. > > The alternative is that there's some mime type that is being looked up for > those tests that the test machines have defined outside of mimeTypes.rdf and > that you don't. This is unlikely to be affected by the unregistering (you > should be able to prove that by pushing that unregister patch to try). You are absolutely right!
Updated•10 years ago
|
Attachment #8440946 -
Flags: review?(standard8) → review+
Updated•9 years ago
|
Whiteboard: [patchlove]
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•