So we can write Test-Double of nsIFile in xpcshell test. An example of those Test-Double is attached in bug 674742 comment #45.

I am sorry for the previous wrong patch.

Comment on attachment 652638 [details] [diff] [review] Fix Given what I know so far about this bug, this is definitely not a good idea. We cast nsIFile* to nsLocalFile* internally in a bunch of places, and I can't understand why you'd need to mock nsIFile in a test when you could just use it directly.

(In reply to Benjamin Smedberg [:bsmedberg] [away 27-July until 7-Aug] from comment #2) > Comment on attachment 652638 [details] [diff] [review] > Fix > > Given what I know so far about this bug, this is definitely not a good idea. > We cast nsIFile* to nsLocalFile* internally in a bunch of places, and I > can't understand why you'd need to mock nsIFile in a test when you could > just use it directly. Because I could not. How can we write test cases that nsIFile.moveTo (or other methods) fails without such mock?

You make a read-only directory and try to move a file into it?

(In reply to Benjamin Smedberg [:bsmedberg] from comment #4) > You make a read-only directory and try to move a file into it? That's a good idea in general but can be used in the case of bug 674742 due to NS_ERROR_FILE_ACCESS_DENIED before moveTo. I will reopen this bug when I implement another example.

Reopening. I've post another example in bug 784872, and a new Test-Double implementation of nsIFile in bug 784870. The new implementation can be used in most cases. In bug 784872 case, the Test-Double returns a fake size if nsIFile.fileSize is invoked.

Benjamin, what do you think of non-builtinclass only for test build (i.e. --enable-tests option for configure)? Though I do not have any idea how to do it yet.

We should test what actually shipped. Moreover, some feature may depend on the builtinclass-ness. For example, see bug 784436.

(In reply to Masatoshi Kimura [:emk] from comment #8) > We should test what actually shipped. > Moreover, some feature may depend on the builtinclass-ness. For example, see > bug 784436. Thanks for the info. Do you have any other idea to implement test for failure cases of nsIFile method such as bug 784870?

(In reply to Hiroyuki Ikezoe (:hiro) from comment #7) > Benjamin, what do you think of non-builtinclass only for test build (i.e. > --enable-tests option for configure)? Though I do not have any idea how to > do it yet. --enable-tests is the default, and we ship our release builds that way, FYI.

In bug 784870, it appears that you're re-implementing nsIFile in C++, so why do you need builtinclass at all? Although note that still isn't safe because of internal nsIFile* -> nsLocalFile* casts, so depending on which localfile methods you call you might just crash. In any case, I do not agree with the basic premise of this bug. You'll have to find a different way to test your code.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #11) > In bug 784870, it appears that you're re-implementing nsIFile in C++, so why > do you need builtinclass at all? Woohoo!!!! Thank you, Benjamin! You reminds me to add 'builtinclass' flag to a child of nsILocalFile. Now we can write stubs of nsIFile. Thanks a lot!