Closed Bug 1094906 Opened 6 years ago Closed 5 years ago

Convert xpcshell-tests in toolkit/components/places/tests/queries to Bookmarks.jsm API

Categories

(Toolkit :: Places, defect)

defect
Not set
Points:
3

Tracking

()

RESOLVED FIXED
mozilla39
Iteration:
39.2 - 23 Mar
Tracking Status
firefox39 --- fixed

People

(Reporter: mak, Assigned: ttaubert)

References

Details

Attachments

(1 file)

not a lot of tests, but we must make them async
Flags: qe-verify-
Flags: firefox-backlog+
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Iteration: --- → 38.3 - 23 Feb
Iteration: 38.3 - 23 Feb → 39.1 - 9 Mar
Iteration: 39.1 - 9 Mar → 39.2 - 23 Mar
Comment on attachment 8576697 [details] [diff] [review]
0001-Bug-1094906-Convert-xpcshell-tests-in-toolkit-compon.patch

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

::: toolkit/components/places/tests/queries/test_tags.js
@@ +376,5 @@
>  
>      do_print("Add bookmarks and tag the URIs");
>      for (let [pURI, tags] in Iterator(urisAndTags)) {
>        let nsiuri = uri(pURI);
> +      yield addBookmark(nsiuri);

please change function to function*

if you want to explicit add_task for each function here, feel free to... I don't really like the .forEach(add_task) habit.

@@ +428,5 @@
>               "same name) should not throw off query results");
>      var tagName = "foo";
>  
>      do_print("Add bookmark and tag it normally");
> +    yield addBookmark(TEST_URI);

ditto

@@ +438,5 @@
> +      parentGuid: PlacesUtils.bookmarks.tagsGuid,
> +      type: PlacesUtils.bookmarks.TYPE_FOLDER,
> +      title: tagName
> +    });
> +    do_check_true(dupTag);

this check is pointless, the previous one was not useful too.

@@ +445,5 @@
> +      parentGuid: dupTag.guid,
> +      title: "title",
> +      url: TEST_URI
> +    });
> +    do_check_true(bm);

ditto

@@ +472,5 @@
> +      parentGuid: PlacesUtils.bookmarks.unfiledGuid,
> +      type: PlacesUtils.bookmarks.TYPE_FOLDER,
> +      title: tagName
> +    });
> +    do_check_true(folder);

you can remove this check.

we had the habit to check the returned id from methods, but it's not really needed, methods would throw in case of errors.

@@ +577,5 @@
> +    title: aURI.spec,
> +    url: aURI
> +  });
> +
> +  do_check_true(bm, "Sanity check: insert should not fail");

ditto
Attachment #8576697 - Flags: review?(mak77) → review+
(In reply to Marco Bonardo [::mak] from comment #2)
> if you want to explicit add_task for each function here, feel free to... I
> don't really like the .forEach(add_task) habit.

Will do.
https://hg.mozilla.org/mozilla-central/rev/73e19da9241b
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.