Open Bug 1025740 Opened 10 years ago Updated 2 years ago

UMimTypProvider should be registered in some tests

Categories

(MailNews Core :: Testing Infrastructure, defect)

defect

Tracking

(Not tracked)

People

(Reporter: hiro, Unassigned)

Details

(Whiteboard: [patchlove])

Attachments

(3 files)

Otherwise other tests might be affected the UMimTypProvider.
Attached patch FixSplinter Review
Attachment #8440515 - Flags: review?(standard8)
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 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)
(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.
(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.
Summary: UMimTypProvider should be unregistered when test is finished → UMimTypProvider should be registered in some tests
(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!
Attachment #8440946 - Flags: review?(standard8) → review+
Whiteboard: [patchlove]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: