Closed Bug 1579315 Opened 3 months ago Closed 3 months ago

Test import-export.js

Categories

(Calendar :: Import and Export, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: darktrojan, Assigned: darktrojan)

References

Details

Attachments

(1 file)

There should be tests for importing and exporting items, and there aren't.

Here's a simple test for importing items. Most of the change to import-export.js is adding indentation, because I've made it return a Promise.

Assignee: nobody → geoff
Status: NEW → ASSIGNED
Attachment #9093470 - Flags: review?(paul)
Comment on attachment 9093470 [details] [diff] [review]
1579315-import-test-1.diff

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

Hooray for more test coverage!  And more async!  LGTM.  One optional "while we're here" question/suggestion.

I ran the test using:
$ ../mach mochitest calendar/test/browser/browser_import.js
and it passed.  When I changed the assertion about the expected results from 4 to 3, the test failed as expected.

::: calendar/base/content/import-export.js
@@ +53,4 @@
>      }
>  
> +    picker.open(async rv => {
> +      if (rv != Ci.nsIFilePicker.returnOK || !picker.file || !picker.file.path) {

What is rv?  If you know, can we give it a less obscure name while we're here?
Attachment #9093470 - Flags: review?(paul) → review+

rv is shorthand for return value. It's used all over the C++ code, although in JS it is rare.

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/315a09d57f1b
Add a test of importing events; r=pmorris

Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 71
You need to log in before you can comment on or make changes to this bug.