1113087 - Add automated test to crash openH264 plugin

Status: RESOLVED WONTFIX

(Mozilla QA Graveyard :: Mozmill Tests, defect)

Description

[Cosmin Malutan, [:cosmin-malutan]]

https://moztrap.mozilla.org/manage/case/14441/

Steps:
1 Launch Firefox with a clean profile.
2 Go to Add-ons Manager -> Plugins and wait for openH264 plugin to install.
3 Visit http://mozilla.github.io/webrtc-landing/pc_test.html and start a h.264 video call (Note: Make sure that Require H.264 video checkbox is marked).
4 Navigate to about:config and click on "I'll be careful, I promise!" button.
 > The crash reporting notification is shown.
5 Create "media.gmp.plugin.crash" (right click -> New -> Boolean) pref and set it to true.
 > The crash reporting notification is shown on the page opened at step 3.
6 Click on "Submit a crash report" button present in the Crash reporter notification on the page from step 3.
 > "Submit a crash report" button is available in the crash reporting notification.
7 Navigate to about:crashes and click on the new generated crash report.
 > A new crash report is properly generated.

This test needs and plugin that installs from remote openH264 and needs to access remote resources so it's a mozmill candidate.

Comment 1

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

Why do you think that this test is doable as a mozmill test? There is no way for us to submit a crash report.

Comment 2

[Cosmin Malutan, [:cosmin-malutan]]

We don't have to actually submit it, we have only to have it under about:crashes, we can have it there even without internet connection. Also keep in mind that Firefox doesn't crash, the plugin crashes and it won't affect the Firefox process itself. I checked all the steps manually 3 times on different platforms this is perfectly doable also is marked with need-automation.

Comment 3

[Cosmin Malutan, [:cosmin-malutan]]

To have full coverage for moztrap 14537 - https://moztrap.mozilla.org/manage/case/14537/
after steps from comment 0 above we have to:
 > Navigate to about:healthreport -> Raw Data and search for gmplugin-crash entry (at the bottom of the page).

Comment 4

[Cosmin Malutan, [:cosmin-malutan]]

In order to test this plugin, we need to have a test-page that uses it. The link from the user story requires to have a real web-cam, and also it starts a two-ways communication link, which results in 4 RTP streams and we need only one, to send/receive video that is encoded with openH264 plugin. I will file a bug to add this test-page to testcase-data, and use the screen-share so we can test this on stations that don't have a real-webcam.

Comment 5

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: MVP patch

This is a work in progress test which covers all the steps, it will need some polishing before it will be a review candidate. 
I had to reset the environment variable that disables the crash reporting(MOZ_CRASHREPORTER_NO_REPORT) and setting it back in teardown. It is disabled by the test harness but we need it enabled.
The reason why mozcrash doesn't see this crash and marks the test as aborted is because we submit the crash so it's moved from 'profile/minidumps' and from 'Crash Reports/pending'.

Something that i's unclear for me is what should I check at step 7:
>7 Navigate to about:crashes and click on the new generated crash report.
> > A new crash report is properly generated.
Should a simply waitForPageLoad be sufficient?

Comment 6

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: 1113087.patch

I updated the patch by splitting  the code on specific test functions and checking for crash signature on crash report.

http://mozmill-crowd.blargon7.com/#/remote/report/5c7e7a22f9984b46554bc3b9fc654877

Comment 7

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch v1.0

I didn't exported the patch :D

Comment 8

[Mihaela Velimiroviciu (:mihaelav)]

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

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

Please add a valid commit message for the patch.

The test fails on my machine (Ubuntu 14.04):
ERROR | Test Failure | {
  "exception": {
    "message": ": could not find element ID: startButton", 
    "lineNumber": 348, 
    "name": "Error", 
    "fileName": "resource://mozmill/driver/mozelement.js"
  }
}
TEST-UNEXPECTED-FAIL | /home/mihaelavelimiroviciu/Desktop/reviews/openh264/firefox/tests/remote/testAddons/testOpenH264Plugin.js | testScreenSharing
TEST-START | /home/mihaelavelimiroviciu/Desktop/reviews/openh264/firefox/tests/remote/testAddons/testOpenH264Plugin.js | testPluginCrashReport
ERROR | Test Failure | {
  "exception": {
    "message": "Notification transition finished", 
    "lineNumber": 27, 
    "name": "TimeoutError", 
    "fileName": "resource://mozmill/modules/errors.js"
  }
}
TEST-UNEXPECTED-FAIL | /home/mihaelavelimiroviciu/Desktop/reviews/openh264/firefox/tests/remote/testAddons/testOpenH264Plugin.js | testPluginCrashReport
TEST-START | /home/mihaelavelimiroviciu/Desktop/reviews/openh264/firefox/tests/remote/testAddons/testOpenH264Plugin.js | testHealthReportData
ERROR | Test Failure | {
  "exception": {
    "message": "'gmplugin-crash' entry has been found.", 
    "lineNumber": 27, 
    "name": "TimeoutError", 
    "fileName": "resource://mozmill/modules/errors.js"
  }
}
TEST-UNEXPECTED-FAIL | /home/mihaelavelimiroviciu/Desktop/reviews/openh264/firefox/tests/remote/testAddons/testOpenH264Plugin.js | testHealthReportData

::: firefox/tests/remote/testAddons/testOpenH264Plugin.js
@@ +7,5 @@
> +var addons = require("../../../../lib/addons");
> +var prefs = require("../../../../lib/prefs");
> +var tabs = require("../../../lib/tabs");
> +var utils = require("../../../../lib/utils");
> +var domUtils = require("../../../../lib/dom-utils");

Please order these alphabetically

@@ +47,5 @@
> +
> +function teardownModule(aModule) {
> +  addons.resetDiscoveryPaneURL();
> +  prefs.clearUserPref(PREF_SCREENSHARING_DOMAINS);
> +  prefs.clearUserPref(PREF_GMP_PLUGIN_CRASH);

You should also reset the extensions.ui.lastCategory pref.

@@ +53,5 @@
> +
> +  aModule.tabBrowser.closeAllTabs();
> +}
> +
> +function testOpenH264PluginInstalled() {

There should be a small description for what each of the tests does.

@@ +94,5 @@
> +                                         parent: menupopup});
> +  var shareSbutton = locationBar.getNotificationElement(
> +    "webRTC-shareDevices-notification", {type: "anonid", value: "button"}
> +  );
> +  shareMenu.click();

Separate with a blank lone from the variables declarations block.

@@ +116,5 @@
> +  // Submit the crash report
> +  var submitCrashLabel = browserWindow.getProperty("crashedpluginsMessage.submitButton.label");
> +  var submitCrashButton = browserWindow.tabs
> +                          .getTabPanelElement(browserWindow.tabs.selectedIndex,
> +                                              '/{"value":"plugin-crashed"}/{"label":"' + submitCrashLabel + '"}');

This line is too long. Maybe it would be better to assign that string to a variable and use it here instead.

@@ +131,5 @@
> +
> +  var details = findElement.ID(controller.tabs.activeTab,
> +                               "report-header-details");
> +  assert.ok(details.getNode().textContent.contains(TEST_DATA.crashSignature),
> +            "Crash signature is contains in reports header");

nit: "Crash signature is contained in the report's header"

@@ +146,5 @@
> +
> +  // Wait for iFrame content document to load
> +  controller.waitForPageLoad(remoteReport.getNode().contentWindow.document);
> +
> +

You have an extra blank line, please remove

@@ +147,5 @@
> +  // Wait for iFrame content document to load
> +  controller.waitForPageLoad(remoteReport.getNode().contentWindow.document);
> +
> +
> +  var rawDataLink = findElement.Selector(remoteReport.getNode().contentWindow.document, ".nav li:last-child a");

This line is too long, please split it

@@ +150,5 @@
> +
> +  var rawDataLink = findElement.Selector(remoteReport.getNode().contentWindow.document, ".nav li:last-child a");
> +  rawDataLink.click();
> +
> +  var rawData = findElement.Selector(remoteReport.getNode().contentWindow.document, "#data code pre");

Split this line, as well.

@@ +154,5 @@
> +  var rawData = findElement.Selector(remoteReport.getNode().contentWindow.document, "#data code pre");
> +
> +  // Raw-data gets updated asynchronously after we click on 'Raw Data' link
> +  assert.waitFor(()=> {
> +    return rawData.getNode().innerHTML && rawData.getNode().innerHTML.contains("gmplugin-crash");

Split this line, as well

Comment 9

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch v2.0

(In reply to Mihaela Velimiroviciu [QA] (:mihaelav) from comment #8)
> ERROR | Test Failure | {
>   "exception": {
>     "message": ": could not find element ID: startButton", 
>     "lineNumber": 348, 
>     "name": "Error", 
>     "fileName": "resource://mozmill/driver/mozelement.js"
>   }
> }
I don't know why you've got this, I added an waitThenClick, this should fix it.

Report:
http://mozmill-crowd.blargon7.com/db/04e54d608fc589a28217304fe676de93

Comment 10

[Mihaela Velimiroviciu (:mihaelav)]

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

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

Please update the commit message.

(In reply to Cosmin Malutan from comment #9)
> I don't know why you've got this, I added an waitThenClick, this should fix
> it.

Indeed, it is better now.

::: firefox/tests/remote/testAddons/testOpenH264Plugin.js
@@ +4,5 @@
> +
> +"use strict";
> +
> +var addons = require("../../../../lib/addons");
> +var domUtils = require("../../../../lib/dom-utils");

You don't use this in this test, it can be removed.

@@ +50,5 @@
> +function teardownModule(aModule) {
> +  addons.resetDiscoveryPaneURL();
> +  prefs.clearUserPref(PREF_SCREENSHARING_DOMAINS);
> +  prefs.clearUserPref(PREF_GMP_PLUGIN_CRASH);
> +  env.set(ENV_CRASHREPORTER_STATE, defaultCrashReporter);

Please reset the extensions.ui.lastCategory pref

@@ +62,5 @@
> +function testOpenH264PluginInstalled() {
> +  am.open();
> +  am.setCategory({category: am.getCategoryById({id: "plugin"})});
> +
> +  // get the OpenH264Plugin and wait for it to install(wait for warning to go away)

nit: "Get" instead of "get"

@@ +79,5 @@
> +/**
> + * Start the screen sharing and check that it started
> + */
> +function testScreenSharing() {
> +  // After the plugin is installed open the test page

"// Open the test page" because the plugin installation is part of the previous test

@@ +91,5 @@
> +    panel = aPanel;
> +    startButton.waitThenClick();
> +  }, {type: "notification"});
> +
> +  // Select the first application in the dropdown then click on share button

nit: // Select the first application in the dropdown then click on the "Share" button

@@ +133,5 @@
> +    '/{"value":"plugin-crashed"}/{"label":"' + submitCrashLabel + '"}'
> +  );
> +  submitCrashButton.click();
> +
> +  // Open the first link under the about crashes

// Open the first link from the about:crashes page

@@ +161,5 @@
> +  // Open the healthreport and check for gmplugin-crash entry under the raw-data tab
> +  controller.open("about:healthreport");
> +  controller.waitForPageLoad();
> +
> +  // Wait for iFrame element to exists

nit: Wait for the iFrame element to exist

Comment 11

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

So there is bug 1046052 which is exactly the same but for mochitest. It might be good to evaluate that we do not duplicate the efforts and what might be better to have.

Comment 12

[Cosmin Malutan, [:cosmin-malutan]]

Has the same steps and it's in a more advanced steps plus that it won't use the production server for submitting the crashes so I'll drop the work on this one.

Comment 13

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

Please not that hasty! The question is still what could we do more than a Mochitest! Please think about that. Keep in mind that we e.g. run for all locales, so it could be additional coverage. Maybe Georg can give his feedback.

Comment 14

[Cosmin Malutan, [:cosmin-malutan]]

We also use the real plugin and we cover the https://moztrap.mozilla.org/manage/case/14537/ too.

Comment 15

[Georg Fritzsche [:gfritzsche]]

* for the purposes of testing crash functionality, it doesn't matter whether we use the real or fake plugin
* the moztrap case could be covered by bug 1046052 (or another browser test)
* the functionality tested by bug 1046052 is not dependent on locale, except a not-so-critical label/button test
* we already have test coverage of download and installation behavior

Are there any other interesting differences here?

Comment 16

[Cosmin Malutan, [:cosmin-malutan]]

It makes sense, as I said, I would drop this bug in the favor of browser test(bug 1046052).
Henrik do you agree?

Comment 17

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

Sounds like we can close it.