838945 - activation tests

Status: RESOLVED DUPLICATE of bug 786133

(Firefox Graveyard :: SocialAPI, defect)

Description

[Shane Caraveo (:mixedpuppy)]

Attachment: activation tests

This patch is tests from an earlier refactoring patch that we have split out into several pieces.  It includes one small bug fix that the tests uncovered.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 711137 [details] [diff] [review]
activation tests

Having coverage of this is a good thing

Comment 2

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

Comment on attachment 711137 [details] [diff] [review]
activation tests

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

It would be helpful for me and for QA if the bug had some steps-to-reproduce and a description of what this is fixing.

::: browser/base/content/test/social/browser_social_activation.js
@@ +116,5 @@
> +      is(Social.provider.origin, gProviders[0].origin, "new provider is active");
> +      checkSocialUI();
> +      // hit "undo"
> +      document.getElementById("social-undoactivation-button").click();
> +      executeSoon(function() {

Can an observer be used here? executeSoon is a code smell.

@@ +166,5 @@
> +          ok(!SocialUI.notificationPanel.hidden, "activation panel should be showing");
> +          is(Social.provider.origin, gProviders[2].origin, "new provider is active");
> +          checkSocialUI();
> +          // A bit contrived, but set a new provider current while the
> +          // activation ui is up.

It would probably be a better test to run the command that is executed if the user uses the social menu or add-ons integration as compared to the undoactivation button.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Attachment: updated

This patch does use an observer - but even then still needs to executeSoon() as the other observers need to run before we can test the other observers did the right thing :)

I also added a test for provider removal via Social.deactivateFromOrigin (which is how they are removed via the normal UI), and Shane can add new tests specific to the addons integration in that patch.

Also added a check that the number of providers on the toolbar/menu is as expected.

Comment 4

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 711281 [details] [diff] [review]
updated

these test probably need to be updated with the changes from the install patch in bug 786133, so removing review for now, will freshen the patch.

Comment 5

[Shane Caraveo (:mixedpuppy)]

These tests are important for the install/activation changes in bug 786133, so this patch is being moved into that bug.