664019 - Mozmill test for installing an add-on from "Firefox Collection"

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[u279076]

Tracking bug to develop a Mozmill test for the "Firefox Collection" section of the Promo module of the Discovery Pane of the Add-ons Manager.

Comment 1

[u415141]

A Pivotal Tracker story has been created for this Bug: https://www.pivotaltracker.com/story/show/14522899

Comment 2

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v1.0

This patch contains a fixed nit in addons.js, required for the test to work. 
Without this, we will fail. 

This test runs on mozilla-beta branch, with a clean profile for starters. 
run command : mozmill --show-all -b /../path to firefox -t /tests/remote/testDiscoveryPane/testInstallCollectionAddon.js

Comment 3

[u279076]

Comment on attachment 539153 [details] [diff] [review]
patch v1.0

Code-wise this is fine. Holding off final review until I can get these remote tests running and tested locally.

Comment 4

[Alex Lakatos[:AlexLakatos]]

Here is the screencast
http://dl.dropbox.com/u/22633148/Mozmill/screencast664019.ogv

Comment 5

[u279076]

>  var discovery = am.discoveryPane;
>  discovery.waitForPageLoad();

There is a failure point in here due to an insufficient timeout. Please refactor to use a waitFor() instead, as per email instructions.

Comment 6

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v1.1

Comment 7

[Maniac Vlad Florin (:vladmaniac)]

Anthony, there are no failures if using Mozmill 1.5.4b2, at least not for me and Alex

Comment 8

[u279076]

Comment on attachment 540734 [details] [diff] [review]
patch v1.1

Code-wise this is fine. Aaron, can you please test this patch on mozmill-release and see if it passes ok?

Comment 9

[Aaron Train [:aaronmt]]

Comment on attachment 540734 [details] [diff] [review]
patch v1.1

Tested on Mozmill 1.5.4b4 --

Firefox Beta: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:5.0) Gecko/20100101 Firefox/5.0
Result: Hangs on the Install Now prompt after timer countdown.

Firefox Release: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:5.0) Gecko/20100101 Firefox/5.0
Result: Hangs on the Install Now prompt after timer countdown.

Without looking at the patch, it would seem that this dialog needs to be handled correctly.

Comment 10

[u279076]

Looks like this failure is a regression from bug 666245. While waiting for that to be resolved can you make one minor change...

Please move this test to mozmill-tests/tests/remote/restartTests/testDiscoveryPane/test6.js and add a check to the end of your test which verifies the add-on is listed in the list of installed add-ons.

Comment 11

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch for addons.js

Please add this patch first for the test to work

Comment 12

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v1.2

This also tests if the addons is installed and present in the addon list from extensions pane

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 542747 [details] [diff] [review]
patch for addons.js

Why do we need this patch? We can't work with the original fx4-collection element, because it's getting cloned. That's necessary for cycling through the sub sections.

Comment 14

[u279076]

Comment on attachment 542747 [details] [diff] [review]
patch for addons.js

Can you please add this all into one patch?

Comment 15

[u279076]

Comment on attachment 542748 [details] [diff] [review]
patch v1.2

Canceling review...please merge your two patches and resubmit for review.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #15)
> Canceling review...please merge your two patches and resubmit for review.

We still have to figure out why this is necessary. When I talked with Matt Claypotch, he mentioned that we have to use the cloned panel. Why doesn't it work anymore? Has there anything changed on the website?

Comment 17

[Maniac Vlad Florin (:vladmaniac)]

Nothing I know of. From what I know it never worked using the cloned panel

Comment 18

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v2.0

I am uploading this patch as a request from anthony. We can leave it unreviewed until we figure out whether we should use the cloned panel or not.

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

When I have implemented that API I have tested it and worked fine. Which issues do you face when using the cloned panel?

Comment 20

[Maniac Vlad Florin (:vladmaniac)]

"message": "firstAddon is undefined", "fileName": "resource://mozmill/modules/frame.js -> file:///home/vladmaniac/Desktop/mozmill2/mozmill-tests/tests/remote/testDiscoveryPane/testInstallCollectionAddon.js", "name": "TypeError", "lineNumber": 78}}

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Hm, the behavior is strange. Now the mobile section is the cloned one. That's really something which doesn't make it easy for us to test this website.

Comment 22

[u279076]

Comment on attachment 543092 [details] [diff] [review]
patch v2.0

Canceling review for now. Please request review again once this issue is resolved.

Comment 23

[Maniac Vlad Florin (:vladmaniac)]

This test will fail, because:     

    Last Pass Addon has different versions, depending on the platform
    This is how the HTML for the Add To Firefox Button looks for Last Pass
    <p class="install-button">
    <a class="button platform windows add installer" href="/firefox/downloads/latest/8542/platform:5/addon-8542-latest.xpi?src=discovery-promo" data-hash="sha256:a5a888521fd9cefcb99128dcd9f0fd75e08ba3e186c1e63ef216c2657444a7e5" target="_self">
    <a class="button platform linux add installer" href="/firefox/downloads/latest/8542/platform:2/addon-8542-latest.xpi?src=discovery-promo" data-hash="sha256:debed5b17a52f62ca98f1168bce29fe0b616aff1e76d58c8075207587b063a0f" target="_self">
    <a class="button platform mac add installer" href="/firefox/downloads/latest/8542/platform:3/addon-8542-latest.xpi?src=discovery-promo" data-hash="sha256:9aa73c5762637a09c977a6b21fa7ceb8420e91d3cacb306fbfdf224c9d3358e6" target="_self">
    <a class="button platform android add installer" href="/firefox/downloads/latest/8542/platform:7/addon-8542-latest.xpi?src=discovery-promo" data-hash="sha256:ecf4a8652426b27189febe7b48445185cbd9351bb03a8958158757854f36ac61" target="_self">
    </p>
    Only one has a CSS attribute: display: inline-block and the rest have display:none.
     
    For a regular addon this is how the HTML looks like:
    <p class="install-button">
    <a class="button add installer" href="/firefox/downloads/latest/3456/addon-3456-latest.xpi?src=discovery-promo" data-hash="sha256:c01d80d90c51b016f7540d409e29e45e1a7eb4e2b31ea02482c430754d9e3ab1" target="_self">
    </p>
     
    So when nodeCollector.queryNodes(".install-button a"); collects it, it gets all messed up. 
    This can affect all tests, and we need to fix this in the API

Comment 24

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Good explanation. Please file a new bug for the API then. Thanks.

Comment 25

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v3.0 for beta and release branches

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 543459 [details] [diff] [review]
patch v3.0 for beta and release branches

This test fails for me. The click on the install button doesn't work, so no installation gets triggered.

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 543459 [details] [diff] [review]
patch v3.0 for beta and release branches

>       case "mainFeature_collectionAddons":
>-        nodeCollector.queryNodes("li.cloned #fx4-collection .addons li>a");
>+        nodeCollector.queryNodes("#fx4-collection .addons li>a");

That's the only part now which fails to get applied. I will upload an updated patch, so we can check-in this test today.

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v4 (beta, release) [checked-in]

Updated patch for check-in. Please test again for safety once my API enhancement has been checked-in. For me it works fine in each case (single and multiple platform).

Comment 29

[Geo Mealer [:geo] -- This account is inactive after 2015-07-07]

Comment on attachment 543598 [details] [diff] [review]
Patch v4 (beta, release) [checked-in]

Looks fine to me, aside from my usual comment that the waitFor comment below could be simplified to got 'false', expected 'true' since that's the only way you're going to see it. But I don't actually expect us to change that; if you don't get anything back from Anthony, please go ahead and put this in. 

I'm not sufficiently worried about cross-checking with your other patch this weekend to do more work on that, personally--we can do a sample test run once this has all landed and fix any outstanding edge case bugs that may occur. 

That's my suggestion for protocol going forward as well--desk test yourself as best we can, we'll pick out the rest in the first test run after landing. Otherwise nobody will do reviews because they don't want to be on the hook to desk-test stuff before landing. It's nice to be cautious, but that's too cautious. :)

Comment 30

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 543598 [details] [diff] [review]
Patch v4 (beta, release) [checked-in]

I will land that patch on beta and release now.

Comment 31

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 543598 [details] [diff] [review]
Patch v4 (beta, release) [checked-in]

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/0b26519f20cb (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/1fa8a7b0d108 (release)

Keeping the bug open for the remaining patch on default and aurora.

Comment 32

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v4.0 for nightly and aurora branches

Comment 33

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 543730 [details] [diff] [review]
patch v4.0 for nightly and aurora branches

>+++ b/tests/remote/restartTests/testDiscoveryPane_installCollectionsAddon/test1.js

Don't change file names. Tests have to be named the same across the branches. Otherwise it looks ok.

Comment 34

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v4.1 for nightly and aurora branches

Comment 35

[Maniac Vlad Florin (:vladmaniac)]

yup, sorry about that, little typo

Comment 36

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v4.1 (aurora) [checked-in]

With changes to addons.js

Comment 37

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 544246 [details] [diff] [review]
patch v4.1 (aurora) [checked-in]

API part looks good but please ask review from Anthony first. If the patch works we can go ahead and land it after the merge has finished.

Comment 38

[u279076]

Comment on attachment 544246 [details] [diff] [review]
patch v4.1 (aurora) [checked-in]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/2aa80943fe1c (mozilla-aurora)

Patch does not apply cleanly to default -- please provide a patch for default.

Comment 39

[Maniac Vlad Florin (:vladmaniac)]

This concerns me a bit. The initial patch was meant for aurora and default branch. Do you get any errors there I might be able to look at? 

I will re-patch this but the test itself is passing with nightlies. I can only hope creating another patch will solve this

Comment 40

[Maniac Vlad Florin (:vladmaniac)]

Attachment: patch v4.1 for default [checked-in]

again, hope this will apply cleanly this time.

Comment 41

[u279076]

Comment on attachment 544767 [details] [diff] [review]
patch v4.1 for default [checked-in]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/3b6663d45a7f (default)

Comment 42

[Maniac Vlad Florin (:vladmaniac)]

Thanks for landing this Anthony. However, we have decided in the past to split coverage of this https://litmus.mozilla.org/show_test.cgi?id=15371 litmus test into three mozmill tests, since it was too general. 

Due to this decision, how will we update the litmus test now?

Comment 43

[u279076]

(In reply to comment #42)
> Thanks for landing this Anthony. However, we have decided in the past to
> split coverage of this https://litmus.mozilla.org/show_test.cgi?id=15371
> litmus test into three mozmill tests, since it was too general. 
> 
> Due to this decision, how will we update the litmus test now?

The same as all other Litmus tests except you will reference three test files. Remind me which bug numbers cover this litmus test and I will update the Litmus test.

Comment 44

[Maniac Vlad Florin (:vladmaniac)]

Bugs: 

Bug 657492, Bug 664019, Bug 664018

Comment 45

[u279076]

Vlad, can this test be marked verified? If so, please do.