Closed Bug 1426530 Opened 2 years ago Closed 2 years ago

Make the normandy test suite pass with --verify

Categories

(Firefox :: Normandy Client, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
firefox59 --- fixed

People

(Reporter: mythmon, Assigned: mythmon)

References

Details

Attachments

(1 file)

./mach test --verify runs tests many times in situations that are likely to cause intermittent to fail. Currently, normandy's test suite does not pass in --verify mode because there are tests that fail when ran twice in a row. We should fix this, which will hopefully also help with intermittents such bug 1424400.
A lot of the changes in that patch are switching from a pattern like:

  add_task(withFoo(async function theTest() { ... }))

to a more flexible

  decorate_task(
    withFoo,
    withBar("canTakeParameters"),
    async function theTest() { ... }
  )

This helped with some tests, because it let me change to using test helpers that used parameters that are incompatible with the first method (or at least complicated). In other cases, it was changes that helped me track down why the tests were failing. Although a lot of these changes aren't needed, I think they are good changes to have, because it makes the normandy test suite a little more consistent.
Comment on attachment 8938190 [details]
Bug 1426530 - Make the normandy test suite pass with --verify

https://reviewboard.mozilla.org/r/208926/#review214824

Thanks for tackling these, this looks good. Just one nit below.

::: browser/extensions/shield-recipe-client/test/browser/browser_PreferenceExperiments.js:437
(Diff revision 1)
>      ok(
>        !Services.prefs.prefHasUserValue(`${startupPrefs}.fake.preference`),
>        "stop cleared the startup preference for fake.preference.",
>      );
>  
> +    Preferences.reset("fake.preference");

Can you explain why this is necessary in a comment, and why we're not using mockPreferences?
Attachment #8938190 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8938190 [details]
Bug 1426530 - Make the normandy test suite pass with --verify

https://reviewboard.mozilla.org/r/208926/#review214824

> Can you explain why this is necessary in a comment, and why we're not using mockPreferences?

I dug into this problem, and I found that the issue was actually in another test. I moved the fix and added a comment as to why it is needed.
Pushed by mcooper@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bf8f7fe0bf99
Make the normandy test suite pass with --verify r=Gijs
https://hg.mozilla.org/mozilla-central/rev/bf8f7fe0bf99
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Commit pushed to master at https://github.com/mozilla/normandy

https://github.com/mozilla/normandy/commit/adf9db60ae23ab0e0045899c88d5be0de31fd0a1
Bug 1426530 - Make the normandy test suite pass with --verify r=Gijs
Product: Shield → Firefox
You need to log in before you can comment on or make changes to this bug.