Closed Bug 1369875 Opened 7 years ago Closed 7 years ago

Turn off activity-stream.enabled for new tab related tests

Categories

(Firefox Graveyard :: Activity Streams: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed

People

(Reporter: k88hudson, Assigned: k88hudson)

Details

Attachments

(1 file)

This will prepare the New Tab (and remote new tab) -related tests for when activity-stream is enabled by default in Nightly. For these tests, the activity-stream pref needs to be turned off.
Attachment #8874003 - Flags: review?(edilee)
Assignee: nobody → khudson
Looks reasonable to me; you might want to add a few comments into the code itself about why the new code is present.
Comment on attachment 8874003 [details]
Bug 1369875 - Turn off activity-stream.enabled for new tab related tests

https://reviewboard.mozilla.org/r/145456/#review150266

::: browser/base/content/test/newtab/head.js:12
(Diff revision 1)
>  
>  Services.prefs.setBoolPref(PREF_NEWTAB_ENABLED, true);
> +Services.prefs.setBoolPref(PREF_NEWTAB_ACTIVITY_STREAM, false);
> +
> +// Open a new tab to clear any existing preloaded
> +BrowserOpenTab();

Any particular reason to use `BrowerOpenTab` instead of `BrowserTestUtils`? Potentially there's concerns of the `async`-ish nature as well as the overhead of doing this for every single test?

But as dmose suggested, a comment would be useful as this should be short-term-ish, so perhaps the above concerns can be just be noted for now.
Comment on attachment 8874003 [details]
Bug 1369875 - Turn off activity-stream.enabled for new tab related tests

https://reviewboard.mozilla.org/r/145456/#review150266

> Any particular reason to use `BrowerOpenTab` instead of `BrowserTestUtils`? Potentially there's concerns of the `async`-ish nature as well as the overhead of doing this for every single test?
> 
> But as dmose suggested, a comment would be useful as this should be short-term-ish, so perhaps the above concerns can be just be noted for now.

To be honest, I just did it this way because it was what the rest of the new tab tests I was looking at were doing – I could change it if using BrowserTestUtils the preffered way
Comment on attachment 8874003 [details]
Bug 1369875 - Turn off activity-stream.enabled for new tab related tests

https://reviewboard.mozilla.org/r/145456/#review150350
Attachment #8874003 - Flags: review?(edilee) → review+
Comment on attachment 8874003 [details]
Bug 1369875 - Turn off activity-stream.enabled for new tab related tests

https://reviewboard.mozilla.org/r/145456/#review150266

> To be honest, I just did it this way because it was what the rest of the new tab tests I was looking at were doing – I could change it if using BrowserTestUtils the preffered way

Ok I updated the comments. Does this look ok?
Pushed by khudson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/76d4bd75e8e2
Turn off activity-stream.enabled for new tab related tests r=Mardak
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
https://hg.mozilla.org/mozilla-central/rev/76d4bd75e8e2
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Fixing bug is verified on Latest Beta --
Build ID   :20170727114534
User Agent :Mozilla/5.0 (Windows NT 6.1; rv:55.0) Gecko/20100101 Firefox/55.0

The activity-stream.enabled is turned off for new tab related tests

Tested OS--Windows7 32bit
QA Whiteboard: [bugday-20170726]
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: