Merge mock app-info code used in xpcshell tests.

RESOLVED FIXED in Firefox 48

Status

()

Firefox
General
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: arai, Assigned: arai)

Tracking

44 Branch
Firefox 48
Points:
---

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Derived from bug 1153978.

There are so much copies of mock app-info (@mozilla.org/xre/app-info;1) code around the tree, with several variants, used in xpcshell test.
Most of them could simply be merged into single implementation, maybe with some parameter.

non-exhaustive list here
https://dxr.mozilla.org/mozilla-central/search?q=%22AppInfo+%3D+%7B%22+-nsIXULAppInfo+-nsXULAppInfo&redirect=false&case=true
(Assignee)

Comment 1

2 years ago
Looks like testing/modules/AppInfo.jsm can be used, with some modification to support custom ID, name, etc.
I hope it's also usable from comm-central tree, and it will solve the issue related to bug 1153978, that is, it changes the interface that is also used in comm-central, and to avoid breaking comm-central, we should change the mock app-info implementation at once.
(Assignee)

Comment 2

2 years ago
Created attachment 8730631 [details] [diff] [review]
Merge mock app-info implementation into AppInfo.jsm.

Changed AppInfo.jsm APIs

[updateAppInfo]
The `obj` argument is not used, and I need to pass customization instead.  So changed the argument to `options`, that's an object contains customization data:

> * options is a object with following keys:
>  *   ID:              nsIXULAppInfo.ID
>  *   name:            nsIXULAppInfo.name
>  *   version:         nsIXULAppInfo.version
>  *   platformVersion: nsIXULAppInfo.platformVersion
>  *   OS:              nsIXULRuntime.OS
>  *
>  *   crashReporter:   nsICrashReporter interface is implemented if true
>  *   extraProps:      extra properties added to XULAppInfo

Then, each updateAppInfo call creates new AppInfo instance, with given options, with `newAppInfo` function added by this patch.

[newAppInfo]

Receives `options` from `updateAppInfo`, and returns newly created AppInfo instance.  if options is not passed, it returns almost same thing as APP_INFO in un-patched code.

This function is also exported, as some code needs to create AppInfo instance without registering it automatically.

[getAppInfo]

Returns most recent AppInfo instance.

AppInfo.jsm initially creates AppInfo instance and holds it as `currentAppInfo`.  `getAppInfo` now returns it.  `currentAppInfo` is overwritten by each `updateAppInfo` call.


Then, changes all mock app-info codes to use AppInfo.jsm.

Several code assumes that appBuildID and/or platformBuildID is the specified constant, but after bug 1153978, at least platformBuildID shouldn't be modified (it should return same value as original one, to keep XDR working correctly), so fixed following tests to use getAppInfo().appBuildID and getAppInfo().platformBuildID instead.

  * browser/experiments/test/xpcshell/test_conditions.js
  * toolkit/components/search/tests/xpcshell/test_json_cache.js
  * toolkit/components/telemetry/tests/unit/test_TelemetryController.js
  * toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js
  * toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
  * toolkit/components/urlformatter/tests/unit/test_urlformatter.js
  * toolkit/mozapps/extensions/test/xpcshell/test_bug430120.js


Also, in browser/experiments/test/xpcshell/head.js, |optionsIn| parameter of |createAppInfo| function was not used correctly,

> -function createAppInfo(optionsIn) {

as it's called like following:

> createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
> createAppInfo();

So I changed it to receive each property separately.

> function createAppInfo(ID="xpcshell@tests.mozilla.org", name="XPCShell",
>                        version="1.0", platformVersion="1.0") {


Green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=613ae58f54c0
Assignee: nobody → arai.unmht
Attachment #8730631 - Flags: review?(bzbarsky)
Comment on attachment 8730631 [details] [diff] [review]
Merge mock app-info implementation into AppInfo.jsm.

Please ask someone more familiar with the relevant code bits (e.g. see blame on them) for review on this.  It'd take me some time to sort through this, and I'm quite behind on reviews at the moment, unfortunately....
Attachment #8730631 - Flags: review?(bzbarsky)
(Assignee)

Updated

2 years ago
Attachment #8730631 - Flags: review?(gps)

Comment 4

2 years ago
Comment on attachment 8730631 [details] [diff] [review]
Merge mock app-info implementation into AppInfo.jsm.

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

Sure.

Next time, please split commits into small parts to make review easier.
Attachment #8730631 - Flags: review?(gps) → review+
(Assignee)

Comment 5

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/c36ae47d019b8015595b0adad2a4568b9821b299
Bug 1256088 - Merge mock app-info implementation into AppInfo.jsm. r=gps

Comment 6

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c36ae47d019b
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
You need to log in before you can comment on or make changes to this bug.