Closed Bug 838945 Opened 8 years ago Closed 8 years ago

activation tests

Categories

(Firefox Graveyard :: SocialAPI, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 786133

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch activation tests (obsolete) — Splinter Review
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.
Attachment #711137 - Flags: review?(felipc)
Attachment #711137 - Flags: feedback?(mhammond)
Comment on attachment 711137 [details] [diff] [review]
activation tests

Having coverage of this is a good thing
Attachment #711137 - Flags: feedback?(mhammond) → feedback+
OS: Mac OS X → All
Hardware: x86 → All
Version: 19 Branch → Trunk
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.
Attached patch updatedSplinter Review
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.
Attachment #711137 - Attachment is obsolete: true
Attachment #711137 - Flags: review?(felipc)
Attachment #711281 - Flags: review?(felipc)
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.
Attachment #711281 - Flags: review?(felipc)
These tests are important for the install/activation changes in bug 786133, so this patch is being moved into that bug.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 786133
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.