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

Status: RESOLVED FIXED

Description

[Jared Wein [:jaws] (please needinfo? me)]

Created attachment 648517 [details] [diff] [review]
WIP

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 1

[:Gavin Sharp [email: [e-mail]]]

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.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to :Gavin Sharp (use [e-mail] 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.

Comment 3

[Mark Hammond [:markh]]

Created attachment 654525 [details] [diff] [review]
Reinstate the share button test

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

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

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

Comment 5

[Mark Hammond [:markh]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c58f5c9a5e0b

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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