Closed Bug 1049137 Opened 10 years ago Closed 8 years ago

Intermittent failing test, TEST-UNEXPECTED-FAIL | /builds/slave/test/gaia/apps/system/test/marionette/app_usage_metrics_test.js | App Usage Metrics > Uninstalled apps are counted

Categories

(Firefox OS Graveyard :: Gaia, defect)

Product:
Firefox OS Graveyard
Component:
Gaia
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
2.6 S6 - 1/29

People

(Reporter: kgrandon, Assigned: sfoster)

References

Details

(Keywords: intermittent-failure, Whiteboard: [systemsfe])

Attachments

(1 file)

[gaia] sfoster:app-usage-gij-1049137 > mozilla-b2g:master
46 bytes, text/x-github-pull-request
thills
: review+
thills
: feedback+
Details | Review 
App Usage Metrics > Uninstalled apps are counted:
13:26:51     INFO -    JavaScriptError: (17) TypeError: manifest is null
13:26:51     INFO -    Remote Stack:
13:26:51     INFO -    @app://settings.gaiamobile.org/js/panels/app_permissions_list/panel.js, line 235
13:26:51     INFO -        at Error.MarionetteError (/builds/slave/test/gaia/node_modules/marionette-client/lib/marionette/error.js:67:13)
13:26:51     INFO -        at Object.Client._handleCallback (/builds/slave/test/gaia/node_modules/marionette-client/lib/marionette/client.js:476:19)
13:26:51     INFO -        at /builds/slave/test/gaia/node_modules/marionette-client/lib/marionette/client.js:510:21
13:26:51     INFO -        at TcpSync.send (/builds/slave/test/gaia/node_modules/marionette-client/lib/marionette/drivers/tcp-sync.js:153:10)
13:26:51     INFO -        at Object.send (/builds/slave/test/gaia/node_modules/marionette-client/lib/marionette/client.js:457:36)
13:26:51     INFO -        at Object.Client._sendCommand (/builds/slave/test/gaia/node_modules/marionette-client/lib/marionette/client.js:503:19)
13:26:51     INFO -        at Object._executeScript (/builds/slave/test/gaia/node_modules/marionette-client/lib/marionette/client.js:1468:19)
13:26:51     INFO -        at Object.executeAsyncScript (/builds/slave/test/gaia/node_modules/marionette-client/lib/marionette/client.js:1249:19)
13:26:51     INFO -        at Object.Client.waitForSync (/builds/slave/test/gaia/node_modules/marionette-client/lib/marionette/client.js:676:14)
13:26:51     INFO -        at Object.Client.waitFor (/builds/slave/test/gaia/node_modules/marionette-client/lib/marionette/client.js:650:60)
13:26:51     INFO -        at Object.MarionetteHelper.waitForElementToDisappear (/builds/slave/test/gaia/node_modules/marionette-helper/index.js:181:17)
13:26:51     INFO -        at Context.<anonymous> (/builds/slave/test/gaia/apps/system/test/marionette/app_usage_metrics_test.js:143:19)
13:26:51     INFO -        at callFn (/builds/slave/test/gaia/node_modules/mocha/lib/runnable.js:223:21)
13:26:51     INFO -        at Test.Runnable.run (/builds/slave/test/gaia/node_modules/mocha/lib/runnable.js:216:7)
13:26:51     INFO -        at Runner.runTest (/builds/slave/test/gaia/node_modules/mocha/lib/runner.js:373:10)
13:26:51     INFO -        at /builds/slave/test/gaia/node_modules/mocha/lib/runner.js:451:12
13:26:51     INFO -        at next (/builds/slave/test/gaia/node_modules/mocha/lib/runner.js:298:14)
13:26:51     INFO -        at /builds/slave/test/gaia/node_modules/mocha/lib/runner.js:308:7
13:26:51     INFO -        at next (/builds/slave/test/gaia/node_modules/mocha/lib/runner.js:246:23)
13:26:51     INFO -        at /builds/slave/test/gaia/node_modules/mocha/lib/runner.js:270:7
13:26:51     INFO -        at done (/builds/slave/test/gaia/node_modules/mocha/lib/runnable.js:185:5)
13:26:51     INFO -        at callFn (/builds/slave/test/gaia/node_modules/mocha/lib/runnable.js:228:7)
13:26:51     INFO -        at Hook.Runnable.run (/builds/slave/test/gaia/node_modules/mocha/lib/runnable.js:216:7)
13:26:51     INFO -        at next (/builds/slave/test/gaia/node_modules/mocha/lib/runner.js:258:10)
13:26:51     INFO -        at /builds/slave/test/gaia/node_modules/mocha/lib/runner.js:270:7
13:26:51     INFO -        at done (/builds/slave/test/gaia/node_modules/mocha/lib/runnable.js:185:5)
13:26:51     INFO -        at /builds/slave/test/gaia/node_modules/mocha/lib/runnable.js:199:9
13:26:51     INFO -        at Object.executeHook (/builds/slave/test/gaia/node_modules/marionette-client/lib/marionette/client.js:369:18)
13:26:51     INFO -        at process._tickCallback (node.js:415:13)
13:26:51     INFO -  make: *** [test-integration-test] Error 1
13:26:51    ERROR - Return code: 2
13:26:51     INFO - TinderboxPrint: gaia-integration-tests: 254/<em class="testfail">1</em>/48
13:26:51    ERROR - Tests exited with return code 2: harness failures

https://tbpl.mozilla.org/php/getParsedLog.php?id=45271258&tree=B2g-Inbound
Note sure if it's the cause but I've found that some of the affected code is timezone-sensitive in bug 1234207 so it might cause intermittent issues depending on the host timezone.
I'll take a look into this.
Assignee: nobody → sfoster
Whiteboard: [systemsfe]
Target Milestone: --- → 2.6 S5 - 1/15
I'm not sure why this is labelled as intermittent. Perhaps it was originally, but I guess since it was disabled it has rotted badly. The first thing I see here is that the AppUsageMetrics test helper expects window.wrappedJSObject.appUsageMetrics to exist. But since the big system startup changes in bug 1094759 it has been moved to window.wrappedJSObject.core.appUsageMetrics, so it should fail every time. I'll need to confirm if that changed before or after it was disabled, but I'm a bit worried we're getting misleading errors out of marionette.
Patch in progress for this.
Target Milestone: 2.6 S5 - 1/15 → 2.6 S6 - 1/29
Comment on attachment 8707121 [details] [review]
[gaia] sfoster:app-usage-gij-1049137 > mozilla-b2g:master

I think I've got everything taken care of here except this final issue: the test installs an app from a mock server with a localhost hostname. UsageData.prototype.shouldTrackApp will only accept a marketplace or gaiamobile.org hostname though so the install is not counted (and the test fails).

I guess something change in here while this test has been disabled? Unfortunately the mock server hardcodes a localhost hostname. I'm not sure what the ramifications of adding localhost into shouldTrackApp would be - possibly not good. Any suggestions?
Attachment #8707121 - Flags: feedback?(thills)
Comment on attachment 8707121 [details] [review]
[gaia] sfoster:app-usage-gij-1049137 > mozilla-b2g:master

Hi Sam,
I guess there is no way to do a MockApp like we did for the unit tests, right?  I think we were able to get the installs counted correctly by hardcoding that into the MOckApp.

The other thought I had rather than adding localhost into shouldTrackApp is perhaps checking a config or a setting for it? We can check whether the test runner is calling or it's live.

Although, I can't think of any scenario off-hand that localhost would be a problem to add to shouldTrackApp, we don't want to track private apps due to privacy concern.  In theory, someone could probably host a local website on their device and their app install would be counted.

Thanks,
-tamara
Attachment #8707121 - Flags: feedback?(thills) → feedback+
(In reply to Tamara Hills [:thills] from comment #9)
> Comment on attachment 8707121 [details] [review]
> [gaia] sfoster:app-usage-gij-1049137 > mozilla-b2g:master
> I guess there is no way to do a MockApp like we did for the unit tests,
> right?  

We could mock the app, but that defeats the point of the integration test. 

> Although, I can't think of any scenario off-hand that localhost would be a
> problem to add to shouldTrackApp, we don't want to track private apps due to
> privacy concern.  In theory, someone could probably host a local website on
> their device and their app install would be counted.

Yeah that was my worry - it would be unexpected to see anything you had installed from localhost show up in app install metrics. So I would only want to do that in conjunction with a flag that indicated we were under test. To that end, there is an 'CI' environment variable which I could carry through to the system app via the build script - similar to how we expose GAIA_DEVICE_TYPE in the feature detector. But that seems awfully round-about. At that point, maybe monkey-patching 'shouldTrackApp' isn't so terrible. Its a trade-off between  shipping CI/test-only code, or having an integration test know about and patch the implementation.
Update: I found what seems to me like a reasonable workaround - exposing the MARKETPLACE_ORIGINS const as a 'static' on the AppUsageMetrics constructor allows me to add the system app during that test to ensure the test app install is tracked. Fingers crossed on the test run.
Comment on attachment 8707121 [details] [review]
[gaia] sfoster:app-usage-gij-1049137 > mozilla-b2g:master

This patch is now looking pretty good. I re-triggered 30 times and the couple of failures there appear to be a framework issue (AssertionError: deleteSession called on client w/o driver!) We can also rope in a system peer for review/stamp if necessary but most of the changes are metrics so :thills seemed like a good first stop.
Attachment #8707121 - Flags: review?(thills)
Test failures on Gij18 look to be bug 1240854
Comment on attachment 8707121 [details] [review]
[gaia] sfoster:app-usage-gij-1049137 > mozilla-b2g:master

Hi Sam,

This looks good to me.  I think this is a good solution to expose the MARKETPLACE_ORIGINS.

Thanks,
-tamara
Attachment #8707121 - Flags: review?(thills) → review+
Thanks Tamara. Merged to master: https://github.com/mozilla-b2g/gaia/commit/0217c7a2b208e18b9cb7960a8c9766802669bb7a
Status: NEW → RESOLVED
Closed: 8 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: