Last Comment Bug 557496 - Port Firefox bookmarks tests to SeaMonkey
: Port Firefox bookmarks tests to SeaMonkey
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.1a3
Assigned To: Robert Kaiser
:
Mentors:
Depends on:
Blocks: SMPlacesBMarks 632860 725529
  Show dependency treegraph
 
Reported: 2010-04-06 06:33 PDT by Robert Kaiser
Modified: 2012-02-08 18:48 PST (History)
2 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
add those tests that succeed (347.28 KB, patch)
2010-07-18 18:36 PDT, Robert Kaiser
bugspam.Callek: review+
Details | Diff | Splinter Review

Description Robert Kaiser 2010-04-06 06:33:52 PDT
Once bug 498596 switches us to places bookmarks, we also should get tests for those, starting porting the Firefox tests over to SeaMonkey.
Comment 1 Robert Kaiser 2010-07-18 18:36:01 PDT
Created attachment 458244 [details] [diff] [review]
add those tests that succeed

This patch adds all the tests that succeed, I spent too many hours trying to make others work, esp. in the browser-chrome area, and I gave up on them for now.
Comment 2 Justin Wood (:Callek) 2010-07-18 19:04:32 PDT
Comment on attachment 458244 [details] [diff] [review]
add those tests that succeed

rs+ based on a skim... but

>+++ b/suite/common/places/tree.xml
>-            for (let j = min.value; j <= max.value; ++j)
>-              nodes.push(resultview.nodeForTreeIndex(j));
>+            if (max.value > -1)
>+              for (let j = min.value; j <= max.value; ++j)
>+                nodes.push(resultview.nodeForTreeIndex(j));

Was this another patches remnant, or needed for a test here, if so can you please explain it for posterity.

(If I find time to actually review the tests, I may steal review from neil; but I would think my rs+ is ok for the test-only changes, especially if they all pass.)
Comment 3 Robert Kaiser 2010-07-19 04:26:16 PDT
(In reply to comment #2)
> Was this another patches remnant, or needed for a test here, if so can you
> please explain it for posterity.

Thanks for catching that, it's indeed a remnant, though from testing why some of the not added tests failed. It's not wrong, but it probably doesn't belong there.
Comment 4 Robert Kaiser 2010-08-08 13:12:46 PDT
Pushed as http://hg.mozilla.org/comm-central/rev/b9e980d6f85d
Comment 5 Serge Gautherie (:sgautherie) 2011-02-09 06:42:13 PST
(In reply to comment #3)
> It's not wrong, but it probably doesn't belong there.

This |if (max.value > -1)| got pushed anyway.
Should we revert it (ftb)?
Or try to add it to Firefox too?

Note You need to log in before you can comment on or make changes to this bug.