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

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Marco Bonardo [:mak]]

not a lot of tests, but we must make them async

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1094906-Convert-xpcshell-tests-in-toolkit-compon.patch

Comment 2

[Marco Bonardo [:mak]]

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

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

(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.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/73e19da9241b

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/73e19da9241b