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

RESOLVED FIXED

Status

()

Firefox
Bookmarks & History
RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: dietrich, Assigned: dietrich)

Tracking

Trunk
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

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

Updated

12 years ago
Blocks: 353434
(Assignee)

Comment 1

12 years ago
Created attachment 240361 [details] [diff] [review]
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.
Attachment #240361 - Flags: review?(davel)
(Assignee)

Comment 2

12 years ago
Created attachment 240363 [details] [diff] [review]
tests w/o extraneous debug output

removed some unnecessary print()s i'd added.
Attachment #240361 - Attachment is obsolete: true
Attachment #240363 - Flags: review?(davel)
Attachment #240361 - Flags: review?(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.
> ifdef ENABLE_TESTS
> TOOL_DIRS  += example
> endif

duh.  TOOL_DIRS += tests
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
Attachment #240363 - Flags: review?(davel)
Attachment #240363 - Flags: review?(benjamin)
Attachment #240363 - Flags: review+
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
Attachment #240363 - Flags: review?(benjamin)
Created attachment 240471 [details] [diff] [review]
additional required change to parent Makefile.in

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

Comment 8

12 years ago
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)
Attachment #240363 - Flags: review+
(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

(Assignee)

Comment 10

12 years ago
(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?
Created attachment 240678 [details] [diff] [review]
the real add'l req'd changes to parent Makefile.in

there would be differences if I attached the right file the first time :-/
Attachment #240471 - Attachment is obsolete: true
(Assignee)

Comment 12

12 years ago
checked in on trunk
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.