Closed Bug 712518 Opened 9 years ago Closed 9 years ago

Improve MockFilePicker.jsm

Categories

(Testing :: Mochitest, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla12

People

(Reporter: darktrojan, Assigned: darktrojan)

References

Details

Attachments

(1 file)

There's some things that could be improved about the MockFilePicker, namely:
* create separate methods for setup and cleanup
* unregister the class when finished
* use the result of showCallback, if there is one, instead of returnValue

Anything else?
* adding the GPL/LGPL, whoops.
Blocks: 712993
Attached patch patchSplinter Review
Attachment #584147 - Flags: review?(jmaher)
Comment on attachment 584147 [details] [diff] [review]
patch

Review of attachment 584147 [details] [diff] [review]:
-----------------------------------------------------------------

My only question is we are replacing reset() with init() at the beginning of most of the test cases.  With init() we assigned default values to variables.  Is that not needed now?
Attachment #584147 - Flags: review?(jmaher) → review+
(In reply to Joel Maher (:jmaher) from comment #3)
> My only question is we are replacing reset() with init() at the beginning of
> most of the test cases.  With init() we assigned default values to
> variables.  Is that not needed now?

ITYM "With reset() we ...".
They still get set, init() calls reset().
https://hg.mozilla.org/integration/mozilla-inbound/rev/e20ed47b03f2
Flags: in-testsuite-
Target Milestone: --- → mozilla12
oh, I overlooked that, that one line hidden in there:)
https://hg.mozilla.org/mozilla-central/rev/e20ed47b03f2
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.