Closed Bug 1019450 Opened 6 years ago Closed 3 years ago

Cleanup test_promiseBookmarksTree

Categories

(Toolkit :: Places, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: mano, Assigned: mak)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch patch.diff (obsolete) — Splinter Review
Minor cleanup.
Attachment #8433174 - Flags: review?(mak77)
Comment on attachment 8433174 [details] [diff] [review]
patch.diff

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

::: toolkit/components/places/tests/head_common.js
@@ +985,5 @@
> +    return "bookmarksMenuFolder";
> +  case PlacesUtils.toolbarFolderId:
> +    return "toolbarFolder";
> +  case PlacesUtils.unfiledBookmarksFolderId:
> +    return "unfiledBookmarksFolder";

please indent case(s) deeper than the switch

::: toolkit/components/places/tests/unit/test_promiseBookmarksTree.js
@@ +4,5 @@
>  
>  function* check_has_child(aParentGUID, aChildGUID) {
>    let parentTree = yield PlacesUtils.promiseBookmarksTree(aParentGUID);
> +  ok("children" in parentTree);
> +  notEqual(parentTree.children.find( e => e.guid == aChildGUID ), null);

my opinion is that we should retain the "Assert." part in Places as our style, to make test calls better distinguishable overall in the tests. what do you think?
Attachment #8433174 - Flags: review?(mak77) → review+
Assignee: asaf → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(mak77)
I will just address my comments and land Mano's patch.
Assignee: nobody → mak77
Flags: needinfo?(mak77)
Comment on attachment 8867157 [details]
Bug 1019450 - Cleanup test_promiseBookmarksTree.

https://reviewboard.mozilla.org/r/138756/#review142044
Attachment #8867157 - Flags: review?(mak77) → review+
Attachment #8433174 - Attachment is obsolete: true
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/318c40d7d0d8
Cleanup test_promiseBookmarksTree. r=mak
https://hg.mozilla.org/mozilla-central/rev/318c40d7d0d8
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.