354401 - Update Places unit-tests to work with xpcshell test-harness

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Dietrich Ayala (:dietrich)]

The existing places unit-tests need to be updated to work with the new xpcshell test-harness framework.

Comment 1

[Dietrich Ayala (:dietrich)]

Attachment: bookmark service tests

This patch converts the existing tests to use the xpcshell-simple harness. The set of tests exercises most of the bookmarks service. It should be expanded to completely cover of nsINavBookmarksService, but this is a good enough start.

Asking davel for review to make sure I'm using the harness correctly.

Comment 2

[Dietrich Ayala (:dietrich)]

Attachment: tests w/o extraneous debug output

removed some unnecessary print()s i'd added.

Comment 3

[Dave Liebreich [:davel]]

Comment on attachment 240363 [details] [diff] [review]
tests w/o extraneous debug output

I think you need to add

ifdef ENABLE_TESTS
TOOL_DIRS  += example
endif

to browser/components/places/Makefile.in to get "make check" in mozilla to run this test.

I'm rebuilding with places enabled so I can run this myself, but the code looks reasonable after a brief skim.

Comment 4

[Dave Liebreich [:davel]]

> ifdef ENABLE_TESTS
> TOOL_DIRS  += example
> endif

duh.  TOOL_DIRS += tests

Comment 5

[Dave Liebreich [:davel]]

Comment on attachment 240363 [details] [diff] [review]
tests w/o extraneous debug output

looks ok, except for the fact that none of this is run by a top-level make check.  Asking benjamin for advice

Comment 6

[Dave Liebreich [:davel]]

Comment on attachment 240363 [details] [diff] [review]
tests w/o extraneous debug output

in IRC, bsmedberg helped me figure out what was going on.  You'll need to change browser/components/places/Makefile.in as per a patch I'm about to attach to this bug

Comment 7

[Dave Liebreich [:davel]]

Attachment: additional required change to parent Makefile.in

with this additional change, I think the patch is good to go.

Comment 8

[Benjamin Smedberg]

Comment on attachment 240363 [details] [diff] [review]
tests w/o extraneous debug output

Do we have a bug on not putting these tests in dist/bin? We need to solve that sooner rather than later (and probably add standard rules for unit testing in general to rules.mk)

Comment 9

[Dave Liebreich [:davel]]

(In reply to comment #8)
> (From update of attachment 240363 [details] [diff] [review] [edit])
> Do we have a bug on not putting these tests in dist/bin? We need to solve that
> sooner rather than later (and probably add standard rules for unit testing in
> general to rules.mk)
> 

bug 351968

Comment 10

[Dietrich Ayala (:dietrich)]

(In reply to comment #7)
> Created an attachment (id=240471) [edit]
> additional required change to parent Makefile.in
> 
> with this additional change, I think the patch is good to go.
> 

Hi Dave, i don't see any differences between the two patches:

https://bugzilla.mozilla.org/attachment.cgi?oldid=240363&action=interdiff&newid=240471&headers=1

What are the changes that need to be made? Can you attach the changes, and explain them (or add them to http://developer.mozilla.org/en/docs/Writing_xpcshell-based_unit_tests) so that we know what's required when adding new tests elsewhere?

Comment 11

[Dave Liebreich [:davel]]

Attachment: the real add'l req'd changes to parent Makefile.in

there would be differences if I attached the right file the first time :-/

Comment 12

[Dietrich Ayala (:dietrich)]

checked in on trunk

Comment 13

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv