948258 - Move browser tests from dom/indexedDB/test/Makefile.in to browser.ini

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

dom/indexedDB/test/Makefile.in contains tests which are run conditionally based on MOZ_BUILD_APP="browser".  This patch adds a new mozinfo variable 'buildapp' and moves the tests to a browser.ini using the condition |+run-if = buildapp == "browser"|

Comment 1

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

Attachment: Bug-948258-Move-browser-tests-from-dom-indexedDB-tes.patch

Comment 2

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

Attachment: Bug-948258-Move-browser-tests-from-dom-indexedDB-tes.patch

doh - forgot to include the patch to mozinfo.py

Comment 3

[Mike Hommey [:glandium]]

If the tests are to be run for browser only, why not move them under browser instead? Or, in the opposite direction, why aren't those tests run on other builds than browser?

Comment 4

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

(In reply to Mike Hommey [:glandium] from comment #3)
> If the tests are to be run for browser only, why not move them under browser
> instead? Or, in the opposite direction, why aren't those tests run on other
> builds than browser?

That's a good question. I dug a little into blame, and the most recent change here was:

-ifneq (mobile,$(MOZ_BUILD_APP))
+ifeq (browser,$(MOZ_BUILD_APP))

ie, originally they were just to be excluded under "mobile".

Given this wasn't completely clear, and given I really just want these tests in a manifest, I figured it was simpler to maintain the status-quo in terms of when they are run and what directory they live in.

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 8345105 [details] [diff] [review]
Bug-948258-Move-browser-tests-from-dom-indexedDB-tes.patch

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

r=me, but I'm not clear why you need the buildapp addition, except for perfect fidelity with the existing Makefile.

::: python/mozbuild/mozbuild/mozinfo.py
@@ +52,5 @@
>          d["appname"] = substs["MOZ_APP_NAME"]
>  
> +    # Build app name
> +    if 'MOZ_BUILD_APP' in substs:
> +        d["buildapp"] = substs["MOZ_BUILD_APP"]

Does this actually add any value vs. using appname?

Comment 6

[Mike Hommey [:glandium]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #5)
> Does this actually add any value vs. using appname?

Actually, using the appname is probably wrong.

Comment 7

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

(In reply to Mike Hommey [:glandium] from comment #6)
> (In reply to Ted Mielczarek [:ted.mielczarek] from comment #5)
> > Does this actually add any value vs. using appname?
> 
> Actually, using the appname is probably wrong.

Cool - I pushed the original version using 'buildapp'.

https://hg.mozilla.org/integration/mozilla-inbound/rev/43a7c4ff96e1

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/43a7c4ff96e1