Improve MockFilePicker.jsm

RESOLVED FIXED in mozilla12

Status

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: darktrojan, Assigned: darktrojan)

Tracking

unspecified
mozilla12
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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
Posted 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: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.