Closed Bug 948258 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla29

People

(Reporter: markh, Assigned: markh)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

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"|
Assignee: nobody → mhammond
Status: NEW → ASSIGNED
Attachment #8345104 - Flags: review?(ted)
doh - forgot to include the patch to mozinfo.py
Attachment #8345104 - Attachment is obsolete: true
Attachment #8345104 - Flags: review?(ted)
Attachment #8345105 - Flags: review?(ted)
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?
(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 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?
Attachment #8345105 - Flags: review?(ted) → review+
(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.
(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
https://hg.mozilla.org/mozilla-central/rev/43a7c4ff96e1
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Whiteboard: [qa-]
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.