1273023 - Convert xpcshell-tests in toolkit/components/places/tests/bookmarks/test_385829.js to Bookmarks.jsm API

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[[e-mail]]

Convert test cases to use new bookmark API based on bug 1094903 comment 8.

The required API change could reference to bug 1271201

Comment 1

[[e-mail]]

Attachment: MozReview Request: Bug 1273023 - Convert xpcshell-tests in toolkit/components/places/tests/bookmarks/test_385829.js to Bookmarks.jsm API; r?mak

Review commit: https://reviewboard.mozilla.org/r/52688/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52688/

Comment 2

[Marco Bonardo [:mak]]

Comment on attachment 8752653 [details]
MozReview Request: Bug 1273023 - Convert xpcshell-tests in toolkit/components/places/tests/bookmarks/test_385829.js to Bookmarks.jsm API; r?mak

https://reviewboard.mozilla.org/r/52688/#review50900

::: toolkit/components/places/tests/bookmarks/test_385829.js:18
(Diff revision 1)
> -  var b4 = bmsvc.insertBookmark(folder, uri("http://a4.com/"),
> -                                bmsvc.DEFAULT_INDEX, "4 title");
>  
>    // ensure some unique values for date added and last modified
>    // for date added:    b1 < b2 < b3 < b4
>    // for last modified: b1 > b2 > b3 > b4

You missed setting the dateAdded and lastModified, while those may look nonsense, they are important to guarantee the test won't randomly fail, since timers resolution is low on some systems (Windows and virtual machines) and our timing methods are not monotone, 2 bookmarks may end up with the same value, and then sometimes ordering could fail.

::: toolkit/components/places/tests/bookmarks/test_385829.js:46
(Diff revision 1)
> +  Assert.equal(b2.title, "2 title");
> +  Assert.equal(b3.title, "3 title");
> +
> +  yield PlacesUtils.bookmarks.update({
> +    guid: b3.guid,
> +    title: "title 3"

why are you doing this? the original test doesn't seem to...

Comment 3

[[e-mail]]

https://reviewboard.mozilla.org/r/52688/#review50900

> why are you doing this? the original test doesn't seem to...

I haven't find a way to update dateAdded/lastModified directly after create bookmark with bm.insert, 
therefore I use bm.update to update modifyDate for last modified: b1 > b2 > b3 > b4 order.

Comment 4

[[e-mail]]

Comment on attachment 8752653 [details]
MozReview Request: Bug 1273023 - Convert xpcshell-tests in toolkit/components/places/tests/bookmarks/test_385829.js to Bookmarks.jsm API; r?mak

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52688/diff/1-2/

Comment 5

[[e-mail]]

Thanks for review. After more investigation, I found insert/update API validate dateAdded/modifiedDate time format and  modifiedDate should always larger then dateAdded time. 

Change dateAdded/modifiedDate with valid time format in insert/update method can do the update/modify as previous test does \o/

Comment 6

[Marco Bonardo [:mak]]

Comment on attachment 8752653 [details]
MozReview Request: Bug 1273023 - Convert xpcshell-tests in toolkit/components/places/tests/bookmarks/test_385829.js to Bookmarks.jsm API; r?mak

https://reviewboard.mozilla.org/r/52688/#review51198

LGTM! thank you!

Comment 7

[[e-mail]]

thanks!

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c470151f0f84

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c470151f0f84