Convert MOCHITEST_METRO_FILES to test manifests

RESOLVED FIXED in Firefox 27

Status

Firefox for Metro
Build Config
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: mbrubeck, Assigned: mbrubeck)

Tracking

Trunk
Firefox 27
All
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Created attachment 813295 [details] [diff] [review]
patch

Bug 901990 added manifest files for mochitests, but it missed the metro-chrome mochitests.  This patch adds a METRO_CHROME_MANIFESTS moz.build variable and replaces Makefiles with manifests for all metro-chrome tests.

Because the manifest can't use relative paths starting with ".." I have changed the way that "../mochitest/head.js" is loaded in the "mochiperf" tests, and renamed mochiperf/perfhelpers.js to mochiperf/head.js.  (Note that the Splinter review tool does not always display renames correctly; look at the raw diff if Splinter is confusing.)
Attachment #813295 - Flags: review?(jmathies)
Attachment #813295 - Flags: review?(gps)

Comment 1

4 years ago
Comment on attachment 813295 [details] [diff] [review]
patch

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

I didn't perform an exhaustive audit that the list of files matched up. But the build bits all look good.

::: browser/metro/base/tests/mochiperf/metro.ini
@@ +20,5 @@
> +skip-if = debug
> +[browser_layers_01.js]
> +skip-if = debug
> +[browser_firstx.js]
> +skip-if = debug

Instead of redefining "skip-if = debug" for every entry, you can put this assignment under the [DEFAULT] section and it will get inherited by every section.
Attachment #813295 - Flags: review?(gps) → review+
(Assignee)

Comment 2

4 years ago
https://tbpl.mozilla.org/?tree=Try&rev=d2614bacd88b
(Assignee)

Comment 3

4 years ago
Created attachment 814258 [details] [diff] [review]
patch v2

Rebased, addressed review comment, and got rid of empty moz.build files.  Carrying r=gps.  Switching Metro reviewer, for load balancing.
Attachment #813295 - Attachment is obsolete: true
Attachment #813295 - Flags: review?(jmathies)
Attachment #814258 - Flags: review?(rsilveira)
(Assignee)

Updated

4 years ago
Blocks: 924257
Comment on attachment 814258 [details] [diff] [review]
patch v2

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

Looks good.

::: browser/metro/base/tests/mochitest/metro.ini
@@ +20,5 @@
> +  head.js
> +  helpers/BookmarksHelper.js
> +  helpers/HistoryHelper.js
> +  helpers/ViewStateHelper.js
> +  res/image01.png

I couldn't find the corresponding removal of browser\metro\base\tests\mochitest\res\Makefile.in

::: python/mozbuild/mozbuild/test/frontend/data/test-manifest-keys-extracted/metro.ini
@@ +1,3 @@
> +[DEFAULT]
> +
> +[test_metro.js]

Do we need to create an empty file in like the other js files in this dir?
Attachment #814258 - Flags: review?(rsilveira) → review+
(Assignee)

Comment 5

4 years ago
https://hg.mozilla.org/integration/fx-team/rev/ce7d9ece6243
Whiteboard: [fixed-in-fx-team]
(Assignee)

Comment 6

4 years ago
(I fixed the review comments before landing, and added a missing test change that was preventing the test from failing.)
https://hg.mozilla.org/mozilla-central/rev/ce7d9ece6243
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 27
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.