Open Bug 1852375 Opened 1 year ago Updated 1 year ago

MerinoTestUtils.initWeather() doesn't play well with pushPrefEnv()

Categories

(Firefox :: Address Bar, defect, P3)

Product:
Firefox
Component:
Address Bar
Type:
defect

Tracking

()

People

(Reporter: adw, Unassigned)

Details

In Slack today Marco and I were discussing a problem with MerinoTestUtils.initWeather(). A TV test was hanging on await fetchPromise here because the weather feature was already initialized, so setting the weather prefs to true again didn't retrigger a fetch.

The problem turned out to be a couple later tasks in the test used pushPrefEnv() to set weather.featureGate to false. That created this scenario (copy-pasted from Slack):

  1. When those pushPrefEnv calls run, weather.featureGate is true because of the weather task [the one task in the file that enabled the weather feature]
  2. When the test ends, first MerinoTestUtil’s cleanup function runs and clears that pref, resetting it to false
  3. But then the pref env is popped by the test harness, setting the pref back to true. So the weather feature ends up being enabled when it shouldn’t be

I'm not sure of the best fix but it ought to be harmless for a test to pushPrefEnv the weather prefs at any time. Ideas off the top of my head:

  • MerinoTestUtils.initWeather() should check to see if the feature is already enabled. But this wouldn't fix the underlying problem where the pref gets into an unexpected state in the first place.
  • Maybe add a MerinoTestUtils.uninitWeather() that consumers would be expected to call when they’re totally done with it. Then consumers would not be relying on MerinoTestUtils’s cleanup function to disable weather.
  • MerinoTestUtils itself maybe should use pushPrefEnv? Not clear if that would fix anything

This bug causes permanent failure of TV on browser_glean_telemetry_engagement_selected_result.js

You need to log in before you can comment on or make changes to this bug.