1094853 - Convert chrome tests in browser/components/places to Bookmarks.jsm API

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Marco Bonardo [:mak]]

convert tests to the new async API.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1094853-Convert-chrome-tests-in-browser-componen.patch

Comment 2

[Marco Bonardo [:mak]]

Comment on attachment 8556437 [details] [diff] [review]
0001-Bug-1094853-Convert-chrome-tests-in-browser-componen.patch

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

::: browser/components/places/tests/chrome/test_0_multiple_left_pane.xul
@@ +35,5 @@
>  
>      function runTest() {
> +      SimpleTest.waitForExplicitFinish();
> +
> +      Task.spawn(function* () {

sigh, hope soon or later we'll get bug 1078657.

@@ +45,5 @@
> +             "current version is: " + PlacesUIUtils.ORGANIZER_LEFTPANE_VERSION );
> +
> +        var fakeLeftPanes = [];
> +        var as = PlacesUtils.annotations;
> +        var bs = PlacesUtils.bookmarks;

while touching this code, please
s/var/let

and remove as and bs shortcuts.
The test is small enough to not need to shortcut those

::: browser/components/places/tests/chrome/test_bug427633_no_newfolder_if_noip.xul
@@ +45,4 @@
>        SimpleTest.waitForExplicitFinish();
> +
> +      Task.spawn(function* () {
> +        let bs = PlacesUtils.bookmarks;

ditto

::: browser/components/places/tests/chrome/test_bug485100-change-case-loses-tag.xul
@@ +40,5 @@
> +      SimpleTest.waitForExplicitFinish();
> +
> +      Task.spawn(function* () {
> +        let bs = PlacesUtils.bookmarks;
> +        let ts = PlacesUtils.tagging;

ditto

@@ +51,5 @@
> +        let bm = yield bs.insert({
> +          parentGuid: bs.toolbarGuid,
> +          index: bs.DEFAULT_INDEX,
> +          type: bs.TYPE_BOOKMARK,
> +          url: testURI.spec,

the API also accepts nsIURI

::: browser/components/places/tests/chrome/test_bug549192.xul
@@ +57,5 @@
> +             visitDate: ++vtime, transition: ttype },
> +           { uri: Services.io.newURI("http://example3.tld/", null, null),
> +             visitDate: ++vtime, transition: ttype }];
> +
> +        yield new Promise(resolve => addVisits(places, resolve));

we need a bug to replace the addVisits in head.js with PlacesTestUtils.addVisits. that would also make this code simpler...

::: browser/components/places/tests/chrome/test_selectItems_on_nested_tree.xul
@@ +72,5 @@
> +          title: "bookmark"
> +        });
> +
> +        // Setup the places tree contents.
> +        var tree = document.getElementById("tree");

s/var/let/

::: browser/components/places/tests/chrome/test_treeview_date.xul
@@ +59,1 @@
>                 getService(Ci.nsINavHistoryService);

please remove these

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Marco Bonardo [::mak] (needinfo? me) from comment #2)
> > +        yield new Promise(resolve => addVisits(places, resolve));
> 
> we need a bug to replace the addVisits in head.js with
> PlacesTestUtils.addVisits. that would also make this code simpler...

Filed bug 1129978.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/3fb22d2c7fd7

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3fb22d2c7fd7