Convert MOCHITEST_METRO_FILES to test manifests

RESOLVED FIXED in Firefox 27


Firefox for Metro
Build Config
4 years ago
3 years ago


(Reporter: mbrubeck, Assigned: mbrubeck)


Firefox 27
Windows 8.1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)



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

Bug 901990 added manifest files for mochitests, but it missed the metro-chrome mochitests.  This patch adds a METRO_CHROME_MANIFESTS 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]

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+

Comment 2

4 years ago

Comment 3

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

Rebased, addressed review comment, and got rid of empty 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)


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\

::: python/mozbuild/mozbuild/test/frontend/data/test-manifest-keys-extracted/metro.ini
@@ +1,3 @@
> +
> +[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+

Comment 5

4 years ago
Whiteboard: [fixed-in-fx-team]

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.)
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.