Closed Bug 780010 Opened 8 years ago Closed 8 years ago

Update the browser_shareButton.js test to use common SocialAPI test functions

Categories

(Firefox Graveyard :: SocialAPI, defect)

17 Branch
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 17

People

(Reporter: jaws, Assigned: markh)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch WIP (obsolete) — Splinter Review
To match the other SocialAPI tests, we should rename browser_shareButton.js to browser_social_shareButton.js. The test should also be refactored to use the generic socialAPI functions defined in head.js.

I've attached a start of a patch that also sets a profile when testing the share button.
Comment on attachment 648517 [details] [diff] [review]
WIP

>diff --git a/browser/base/content/test/browser_shareButton.js b/browser/base/content/test/browser_shareButton.js

> function test() {

>   registerCleanupFunction(function() {
>     Services.prefs.clearUserPref(prefName);
>-    gBrowser.removeTab(tab);
>   });

> function testDisable() {

>+  while (gBrowser.tabs.length > 1) {
>+    gBrowser.removeCurrentTab();
>+  }

Why this change? It's generally better for tests to explicitly remove the tabs they add, rather than introducing "catch all" cleanup code that might wallpaper over other issues.
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #1)
> Why this change? It's generally better for tests to explicitly remove the
> tabs they add, rather than introducing "catch all" cleanup code that might
> wallpaper over other issues.

I meant to undo this change. It was one of the things I was doing to try to narrow down the leaks. The next version of this patch won't have it in there.
Blocks: 780987
fairly trivial update to the test to make things work.  FWIW, this patch is in the try run at https://tbpl.mozilla.org/?tree=Try&rev=1e0b679f0a6b
Assignee: jaws → mhammond
Attachment #648517 - Attachment is obsolete: true
Attachment #654525 - Flags: review?(jaws)
Comment on attachment 654525 [details] [diff] [review]
Reinstate the share button test

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

Please remove the comment in Makefile.in here: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/Makefile.in#67
Attachment #654525 - Flags: review?(jaws) → review+
https://hg.mozilla.org/mozilla-central/rev/c58f5c9a5e0b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.