1298023 - Redundant code in Android reftest harness

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Testing, defect, P2)

Description

[Geoff Brown [:gbrown]]

I noticed some "extra" code and prefs in remotereftests.py.

Comment 1

[Geoff Brown [:gbrown]]

Attachment: remove redundant prefs

Most of these prefs are already set to test "dummy" values in mozprofile.

Some are set to the default value. eg font.size.inflation.minTwips = 0 in mobile.js.

And pandaboards are history.

:snorp -- Way back in bug 907351, we set gfx.canvas.azure.accelerated = False in android reftests, to get reftests running on pandaboards. It seems like it is not needed on emulators. OK to remove this pref setting from Android reftests, changing the effective setting to True?

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb9ee4650a81

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8784814 [details] [diff] [review]
remove redundant prefs

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

I really like removing a bunch of prefs, this gets us closer to running the tests as the end user does.

::: layout/tools/reftest/remotereftest.py
@@ -254,5 @@
> -        prefs["extensions.getAddons.getWithPerformance.url"] = "http://127.0.0.1:8888/extensions-dummy/repositoryGetWithPerformanceURL"
> -        prefs["extensions.getAddons.search.browseURL"] = "http://127.0.0.1:8888/extensions-dummy/repositoryBrowseURL"
> -        prefs["extensions.getAddons.search.url"] = "http://127.0.0.1:8888/extensions-dummy/repositorySearchURL"
> -        # Make sure the GMPInstallManager won't hit the network
> -        prefs["media.gmp-manager.url.override"] = "http://127.0.0.1:8888/dummy-gmp-manager.xml";

I believe a lot of these prefs are defined for desktop and talos, is it ok to remove these from our desktop tests as well?

Comment 3

[Geoff Brown [:gbrown]]

(In reply to Joel Maher ( :jmaher) from comment #2)
> I believe a lot of these prefs are defined for desktop and talos, is it ok
> to remove these from our desktop tests as well?

I don't see them defined by desktop test harnesses - am I missing something?

Many of these same prefs are defined by Talos, and I think the same reasoning applies: If Talos is using mozprofile for its profile, setting them again in Talos is redundant. I'm not feeling very familiar with Talos these days. Want me to follow up or can I leave that to you?

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

talos uses mozprofile, I think I see where all the problems came from and there is nothing to specifically do here!

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8784814 [details] [diff] [review]
remove redundant prefs

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

Looks ok modulo the one

::: layout/tools/reftest/remotereftest.py
@@ -263,5 @@
>  
> -        # Disable skia-gl: see bug 907351
> -        prefs["gfx.canvas.azure.accelerated"] = False
> -
> -        prefs["media.autoplay.enabled"] = True

I think we still need this one, maybe.

Comment 6

[Geoff Brown [:gbrown]]

I clarified with snorp on irc; media.autoplay.enabled is already set to true in reftest-preferences.js and prefs-general.js -- we should be good to go here.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f73c97e6b56
Remove some redundant android reftest prefs; r=jmaher

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2f73c97e6b56