1150484 - Replace custom function LOG() with do_print

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Marco Bonardo [:mak]]

we have a custom logger in toolkit/components/places/tests/head_common.js#61 called LOG() that is used in some tests:
mxr.mozilla.org/mozilla-central/search?string=LOG(&case=on&find=places

we should replace the LOG() calls with do_print, and remove the custom logger.

Comment 1

[Ryan Hamley]

Hi Marco, I'm interested in fixing this bug. I have Firefox built and found the references in the code from the link to mxr. How should I proceed to fix the problem?

Comment 2

[Marco Bonardo [:mak]]

these are the calls to replace, 
http://mxr.mozilla.org/mozilla-central/search?string=LOG(&case=on&find=places

they should be changed from LOG( to do_print(

Comment 3

[Ryan Hamley]

Attachment: I replaced the LOG() calls with do_print() and removed the LOG() function

I'm new to Mercurial and patches in general. I believe I've done this correctly. I'm unsure of how exactly to proceed if this patch is deemed acceptable however.

Comment 4

[Marco Bonardo [:mak]]

Comment on attachment 8588067 [details] [diff] [review]
I replaced the LOG() calls with do_print() and removed the LOG() function

Review of attachment 8588067 [details] [diff] [review]:
-----------------------------------------------------------------

There is a small typo in the commit message "removed" should be "remove"

also, you can use nicknames for reviewers, so in this case r=mak would suffice.

::: toolkit/components/places/tests/bookmarks/test_417228-other-roots.js
@@ +28,5 @@
>      do_check_eq(rootNode.childCount, 4);
>  
>      // create a test root
>      this._folderTitle = "test folder";
> +    this._folderId =

your editor is setup to remove all trailing spaces in the files you touch.
While it's not a big deal here since you are only touching tests, it usually is.
The best setting is for it to remove trailing spaces from the LINES you touch.

::: toolkit/components/places/tests/queries/test_results-as-tag-contents-query.js
@@ +96,5 @@
>                    title: "mozadded",
>                    isBookmark: true,
>                    isTag: true,
>                    tagArray: ["moz", "bugzilla"] };
> +  do_print("Adding item to query")

missing semicolon here. it was already broken but please fix it since you're touching this code.

::: toolkit/components/places/tests/queries/test_searchterms-domain.js
@@ +87,5 @@
>    compareArrayToResult(testData, root);
>  
>    // If that passes, check liveupdate
>    // Add to the query set
> +  do_print("Adding item to query")

missing semicolon (again, not your fault but please fix it)

::: toolkit/components/places/tests/queries/test_searchterms-uri.js
@@ +83,5 @@
>     compareArrayToResult(testData, root);
>  
>     // If that passes, check liveupdate
>     // Add to the query set
> +   do_print("Adding item to query")

another one

Comment 5

[Marco Bonardo [:mak]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0df8673f19fa

Comment 6

[Marco Bonardo [:mak]]

we should be fine to checkin once we get the updated patch.

Comment 7

[Marco Bonardo [:mak]]

I'll take care of landing this.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/5b8b725af86c

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/acb44c37e171 (backout)
https://hg.mozilla.org/integration/fx-team/rev/fe67ac8df399

Comment 10

[Carsten Book [:Tomcat]]

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