Closed Bug 1396274 Opened 7 years ago Closed 7 years ago

Enable Activity Stream about:home but disable in browser_aboutHome.js

Categories

(Firefox :: New Tab Page, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: Mardak, Assigned: ursula)

References

Details

Attachments

(1 file)

Some stuff will probably need to be skipped and others fixed
Blocks: 1394533
Assignee: nobody → usarracini
Comment on attachment 8907732 [details]
Bug 1396274 - Disable Activity Stream about:home in browser_aboutHome.js

https://reviewboard.mozilla.org/r/179408/#review184622

Should be good enough hopefully without intermittent timeouts.

::: browser/base/content/test/about/browser_aboutHome.js
(Diff revision 2)
>  /* Any copyright is dedicated to the Public Domain.
>   * http://creativecommons.org/publicdomain/zero/1.0/
>   */
>  
> -// This test needs to be split up. See bug 1258717.
> -requestLongerTimeout(4);

Are we sure this or the other split out tests won't need the longer timeout? It requested 4, and now the test is split into 3. That would seem on average still greater than 1 timeouts worth.

::: browser/base/content/test/about/browser_aboutHome_search.js:57
(Diff revision 2)
> +
> +    // Perform a search to increase the SEARCH_COUNT histogram.
> +    await ContentTask.spawn(browser, { searchStr }, async function(args) {
> +      let doc = content.document;
> +      info("Perform a search.");
> +      let el = doc.querySelector(["#searchText", "#newtab-search-text"]);

This would have been easier to review if the splitting up was done as a separate patch

::: browser/base/content/test/about/browser_aboutHome_search.js:100
(Diff revision 2)
> +        // Ready to execute the tests!
> +        let needle = "Search for something awesome.";
> +
> +        let p = promiseContentSearchChange(browser, engine.name);
> +        Services.search.defaultEngine = engine;
> +        await p;

looks like some whitespace here got changed, but oh well.

::: browser/base/content/test/about/browser_aboutHome_snippets.js:143
(Diff revision 2)
> +    let doc = content.document;
> +    let snippetsElt = doc.getElementById("snippets");
> +    ok(snippetsElt, "Found snippets element");
> +    ok(snippetsElt.getElementsByTagName("a")[0].href != "about:rights",
> +      "Snippet link should not point to about:rights.");
> +  });

The original tests cleared the override pref after the previous 2 tests, but I suppose these back to back tests would happen to "restore" the `true` value. The cleanup/popping of prefs only happens at the end of the test file, but we happen to be okay here.
Attachment #8907732 - Flags: review?(edilee) → review+
Here's a try push that should help detect if we need to request longer or split up more:

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

try: -b do -p linux,linux64,macosx64,win32,win64 -u mochitest-browser-chrome-1,mochitest-browser-chrome-e10s-1,mochitest-e10s-browser-chrome-1 --try-test-paths browser-chrome:browser/base/content/test/about --artifact --rebuild 20
Comment on attachment 8907732 [details]
Bug 1396274 - Disable Activity Stream about:home in browser_aboutHome.js

https://reviewboard.mozilla.org/r/179408/#review184708

Looks like the splitting will happen in bug 1258717. We'll want to get the search tests running with activity stream.
Pushed by edilee@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3ce5a9419d5b
Disable Activity Stream about:home in browser_aboutHome.js r=Mardak
Summary: Get browser_aboutHome.js passing on both about:homes with activity stream or not → Disable Activity Stream about:home in browser_aboutHome.js
Blocks: 1399648
https://hg.mozilla.org/mozilla-central/rev/3ce5a9419d5b
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Summary: Disable Activity Stream about:home in browser_aboutHome.js → Enable Activity Stream about:home but disable in browser_aboutHome.js
Depends on: 1400065
Depends on: 1403414
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.