Closed Bug 1094853 Opened 6 years ago Closed 5 years ago

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

Categories

(Firefox :: Bookmarks & History, defect)

defect
Not set
Points:
2

Tracking

()

RESOLVED FIXED
Firefox 38
Iteration:
38.2 - 9 Feb
Tracking Status
firefox38 --- fixed

People

(Reporter: mak, Assigned: ttaubert)

References

Details

Attachments

(1 file)

convert tests to the new async API.
Flags: qe-verify-
Flags: firefox-backlog+
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Iteration: --- → 38.2 - 9 Feb
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
Attachment #8556437 - Flags: review?(mak77) → review+
Blocks: 1129978
(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.
https://hg.mozilla.org/mozilla-central/rev/3fb22d2c7fd7
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
You need to log in before you can comment on or make changes to this bug.