671479 - Mozmill Endurance test for loading a SWF video via iframe

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[u279076]

Create a Mozmill Endurance test for loading an SWF video embedded into a webpage using the <iframe> tag. An example of how YouTube does this:

<iframe width="425" height="349" src="http://url.to.youtube/video" frameborder="0" allowfullscreen></iframe>

Comment 1

[u279076]

For consistency, please use a width of 640 and a height of 480.

Comment 2

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

That will work with flash content? I would be surprised.

Comment 3

[u279076]

(In reply to comment #2)
> That will work with flash content? I would be surprised.

Yes, I've tested it locally and it works. Simply replace the youtube URL in the code above with the URL to our SWF video on Mozqa.com -- it works.

Comment 4

[u279076]

FYI, in case I wasn't clear before, this is how YouTube does their embedding. Go to any video click the Share button then the Embed button.

Comment 5

[Alex Lakatos[:AlexLakatos]]

Attachment: patch v1.0 [backed-out]

The test page still uses the old flash files. Bug 705238 has to land before this one.

Comment 6

[u279076]

Comment on attachment 576891 [details] [diff] [review]
patch v1.0 [backed-out]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/e1f5d8b8b580 (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/87af5196f9a5 (mozilla-aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/abe51c2208e3 (mozilla-beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/c4ae62f59d88 (mozilla-release)

Comment 7

[u279076]

Please verify with tomorrow's testrun.

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

This appears to be causing a crash in the testruns.

See:
http://mozmill-release.brasstacks.mozilla.com/#/endurance/report/45c0ad8f20b88e32d5d7aab6295c48bf
http://mozmill-release.brasstacks.mozilla.com/#/endurance/report/45c0ad8f20b88e32d5d7aab6295bdf90

It would appear that memory is not being cleared between iterations as expected. Can we replicate this locally? We might need to back this out as it's preventing us from seeing results form other endurance tests.

Comment 9

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

Yes, we should backout the patch for now. Since it has been landed on all branches, we have to take care of any of those. For the future I would really like to propose that we only land it on default and once it has been proven to work we backport it to other branches.

Comment 10

[u279076]

(In reply to Henrik Skupin (:whimboo) [away 11/25 - 12/03] from comment #9)
> Yes, we should backout the patch for now. Since it has been landed on all
> branches, we have to take care of any of those. For the future I would
> really like to propose that we only land it on default and once it has been
> proven to work we backport it to other branches.

I agree Henrik, and is something I was going to propose to the team. I was just waiting until you returned from vacation to send out.

Comment 11

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

Anthony, why this hasn't been backed out yesterday? Any disconnect should cause an immediate backout. 

Dave, can you please take care of? We need a clean testrun today. Thanks.

Comment 12

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Backed out:
http://hg.mozilla.org/qa/mozmill-tests/rev/026bab19d77a (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/e97be7416ca7 (mozilla-aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/bc6e1c4e2bc8 (mozilla-beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/b8a8a35f3e9b (mozilla-release)

Can we try to replicate this locally with different versions of Firefox as this may be a critical regression.

Comment 13

[u279076]

I think this is likely caused by the same issue as bug 671476 comment 71. I'm noticing a degradation in performance over time (possibly GC related). Let's debug it on bug 671476 and see if the fix applies to all of these tests.

Comment 14

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

I would propose to file a new bug for the investigation, just to limit the conversation over on bug 671476. Also we could move such a new bug into another component if it's really not our fault.

Comment 15

[u279076]

(In reply to Henrik Skupin (:whimboo) from comment #14)
> I would propose to file a new bug for the investigation, just to limit the
> conversation over on bug 671476. Also we could move such a new bug into
> another component if it's really not our fault.

We will make that call by discussing on bug 671476 to limit the discussion happening across several bugs.

Comment 16

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

Dave, is that a test which would still be useful for us?

Comment 17

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Yes, I believe so.

Comment 18

[Daniela Petrovici]

Patch still applies correctly. Testrun reports for Linux, Windows and Mac are below for this issue and the one logged in bug 671476:
http://mozmill-crowd.blargon7.com/#/endurance/report/8ec48e7ab0431a61b624e36d31b829ec
http://mozmill-crowd.blargon7.com/#/endurance/report/8ec48e7ab0431a61b624e36d31bd62ae
http://mozmill-crowd.blargon7.com/#/endurance/report/8ec48e7ab0431a61b624e36d31c20b38

Comment 19

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Please at least update the license block in the patch and request review.

Comment 20

[Daniela Petrovici]

Attachment: patch v1.1

I have modified the patch to have the new license and also made changes:
- added aModule parameter to setupModule
- added tearDown because we might be failing at waitForPageLoad and not close the tabs before the end of the test
- added manifest.ini inside the new folder
- added the new manifest.ini to the endurance/manifest.ini

Reports were done together with the new changes from bug 671476 since there were issues running both of them in a single testrun.
Linux: http://mozmill-crowd.blargon7.com/#/endurance/report/8ec48e7ab0431a61b624e36d31ff4d2c
Windows: http://mozmill-crowd.blargon7.com/#/endurance/report/c8094a822ef568b588c5eddaca00cf92
MAC: http://mozmill-crowd.blargon7.com/#/endurance/report/c8094a822ef568b588c5eddaca0fe07c

NOTE: This patch was created above the one from bug 671476 since this bug is dependent on it. Also, both patches are modifying the manifest.ini from endurance.
It takes about 2 hours for the endurance runs after adding these two tests

Comment 21

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Apart from both modifying the same manifest file, I don't see in interdependency here. Both bugs should have patches based on default, and whichever lands last will likely need an update to apply.

Comment 22

[Dave Hunt [:davehunt] [he/him] ⌚BST]

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

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

Please don't based patches on top of other unlanded patches.

Comment 23

[Dave Hunt [:davehunt] [he/him] ⌚BST]

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

>+const TEST_DOMAIN = "http://www.mozqa.com/";
>+const TEST_PAGE = TEST_DOMAIN + "data/firefox/plugins/flash/test_swf_iframes.html";

Please use test_swf_iframes_nosound.html

Comment 24

[Daniela Petrovici]

Attachment: patch v1.2

This patch is created without the changes from bug 671476. Reports:
http://mozmill-crowd.blargon7.com/#/endurance/report/c8094a822ef568b588c5eddaca2c3365
http://mozmill-crowd.blargon7.com/#/endurance/report/c8094a822ef568b588c5eddaca2be2f4

Comment 25

[Andreea Matei [:AndreeaMatei]]

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

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

::: tests/endurance/testFlash_SWFVideoIframe/test1.js
@@ +7,5 @@
> +var endurance = require("../../../lib/endurance");
> +var tabs = require("../../../lib/tabs");
> +
> +const BASE_URL = "http://www.mozqa.com/";
> +const TEST_URL = BASE_URL + "data/firefox/plugins/flash/test_swf_iframes.html";

I think what was established in bug 827752 for the remote pages was to use TEST_URL and not separate the link in 2 constants.
Also as Dave pointed, we should use test_swf_iframes_nosound.html

@@ +14,5 @@
> +
> +function setupModule(aModule) {
> +  aModule.controller = mozmill.getBrowserController();
> +
> +  aModule.enduranceManager = new endurance.EnduranceManager(controller);

aModule.controller as parameter

@@ +15,5 @@
> +function setupModule(aModule) {
> +  aModule.controller = mozmill.getBrowserController();
> +
> +  aModule.enduranceManager = new endurance.EnduranceManager(controller);
> +  aModule.tabBrowser = new tabs.tabBrowser(controller);

Same here

@@ +25,5 @@
> +    if (aAddon.isActive && aAddon.type === "plugin" && aAddon.name === "Shockwave Flash")
> +      return true;
> +  });
> +
> +  if (isFlashActive[0] !== true) {

We could use !isFlashActive[0] here.

@@ +26,5 @@
> +      return true;
> +  });
> +
> +  if (isFlashActive[0] !== true) {
> +    testFlashURL.__force_skip__ = "No enabled Flash plugin detected";

This should be testFlashIframe() and please skip teardownModule() as well.

@@ +51,5 @@
> +      controller.open(TEST_URL);
> +      controller.waitForPageLoad(TIMEOUT_PAGE);
> +      enduranceManager.addCheckpoint("Web page has been loaded");
> +    });
> +    // Close all tabs

We can remove this comment and leave a blank line, as it is cleared from the method's name what it will do.

Comment 26

[Daniela Petrovici]

Attachment: patch v1.3

Modified patch based on review. Reports are:
Linux: http://mozmill-crowd.blargon7.com/#/endurance/report/c8094a822ef568b588c5eddacaa49a9e
MAC: http://mozmill-crowd.blargon7.com/#/endurance/report/c8094a822ef568b588c5eddacaa0e599
Windows: http://mozmill-crowd.blargon7.com/#/endurance/report/c8094a822ef568b588c5eddacaa2bd50

Comment 27

[Daniela Petrovici]

Attachment: patch v1.4

Due to bug 671476 that landed on default, this patch does not apply cleanly anymore. So, I have modified it.
Reports:
Linux: http://mozmill-crowd.blargon7.com/#/endurance/report/49feb8c5f96a46e9037cffdde1c8a865
MAC: http://mozmill-crowd.blargon7.com/#/endurance/report/49feb8c5f96a46e9037cffdde1c89f2f\
Windows: http://mozmill-crowd.blargon7.com/#/endurance/report/49feb8c5f96a46e9037cffdde1c8cbc8

Comment 28

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 742991 [details] [diff] [review]
patch v1.4

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

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/23fbcafd2b77 (default)

Dave, can we close this now without backporting?

Comment 29

[Dave Hunt [:davehunt] [he/him] ⌚BST]

As mentioned in yesterday's meeting, we do backport new endurance tests.

Comment 30

[Daniela Petrovici]

Attachment: patch v1.0 for ESR17

The patch for Nightly applies cleanly on all branches except for ESR17. On ESR17 there is the manifest.ini file where we have an additional test, so I have created the patch for it.

Reports:
Aurora:
Linux: http://mozmill-crowd.blargon7.com/#/endurance/report/452ec32f8deec0960aea87aa06f19708
MAC: http://mozmill-crowd.blargon7.com/#/endurance/report/452ec32f8deec0960aea87aa06f1cb89
Windows: http://mozmill-crowd.blargon7.com/#/endurance/report/452ec32f8deec0960aea87aa06f2064f

Beta:
Windows: http://mozmill-crowd.blargon7.com/#/endurance/report/452ec32f8deec0960aea87aa06feee79
MAC: http://mozmill-crowd.blargon7.com/#/endurance/report/452ec32f8deec0960aea87aa06feb473
Linux: http://mozmill-crowd.blargon7.com/#/endurance/report/452ec32f8deec0960aea87aa06fea152

Release:
Linux: http://mozmill-crowd.blargon7.com/#/endurance/report/ea82256a8ae9808d91b7e8145e05ee5c
MAC: http://mozmill-crowd.blargon7.com/#/endurance/report/ea82256a8ae9808d91b7e8145e05faf3
Windows: http://mozmill-crowd.blargon7.com/#/endurance/report/ea82256a8ae9808d91b7e8145e060c2d

ESR17:
Linux: http://mozmill-crowd.blargon7.com/#/endurance/report/ea82256a8ae9808d91b7e8145e18090a
MAC: http://mozmill-crowd.blargon7.com/#/endurance/report/ea82256a8ae9808d91b7e8145e180694
Windows: http://mozmill-crowd.blargon7.com/#/endurance/report/ea82256a8ae9808d91b7e8145e1ba330

Comment 31

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 746868 [details] [diff] [review]
patch v1.0 for ESR17

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

Transplanted as:
http://hg.mozilla.org/qa/mozmill-tests/rev/d155bd6e6eb5 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/5c3126d75200 (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/73292d5b3863 (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/53d8cf674509 (esr17)

Thanks Daniela!