Closed Bug 1410818 Opened 5 years ago Closed 4 years ago

Add automated test for "Hover over Top Sites tiles"

Categories

(Firefox :: New Tab Page, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 60
Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox60 --- fixed

People

(Reporter: icrisan, Assigned: icrisan)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 5 obsolete files)

testrail link: https://testrail.stage.mozaws.net/index.php?/cases/view/76175

Preconditions: More information about default Top Sites can be found here: https://docs.google.com/spreadsheets/d/15yLsFBkic5DFUSdbvSFlR3Rzjxqwmizq9EmksK28l6I/edit#gid=170333837

Steps:
1. Hover over the 6 default Top Sites tiles and observe the behavior
2. Click on "(...)" More button
3. Observe that default Top Sites do not have a "Delete from History" option

Expected results:
Step 1:
A border is displayed around the tile when hovering
A "(...)" button appears over the top right corner after ~1 second.
Step 2:	
a. More menu has the following options:
- Pin
- Open in a New Window
- Open in a Private Window
- Dismiss
b. Each option has its own icon.
Step 3:
Default Top Sites cannot be deleted from history.
Component: General → Activity Streams: Newtab
Product: Core → Firefox
Summary: Hover over Top Sites tiles → Add automated test for "Hover over Top Sites tiles"
Priority: -- → P3
Attached patch defaultTopSites_menuOptions (obsolete) — Splinter Review
Assignee: nobody → icrisan
Attachment #8949403 - Flags: review?(edilee)
Comment on attachment 8949403 [details] [diff] [review]
defaultTopSites_menuOptions

>+++ b/browser/extensions/activity-stream/test/functional/mochitest/browser_topsites_contextMenu_options.js	Thu Feb 08 17:56:36 2018 +0200
>+add_task(async function defaultTopSites_menuOptions() {
>+  // The pref for TopSites is empty by default.
>+  await SpecialPowers.pushPrefEnv({
>+    set: [["browser.newtabpage.activity-stream.default.sites",
>+      "https://www.youtube.com/,https://www.facebook.com/,https://www.amazon.com/,https://www.reddit.com/,https://www.wikipedia.org/,https://twitter.com/"]]
>+  });
>+  // Toggle the feed off and on as a workaround to read the new prefs.
>+  await SpecialPowers.pushPrefEnv({ set: [["browser.newtabpage.activity-stream.feeds.topsites", false]] });
>+  await SpecialPowers.pushPrefEnv({ set: [["browser.newtabpage.activity-stream.feeds.topsites", true]] });>+  let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);>+  await SpecialPowers.popPrefEnv();
These prefs and tab opening are better handled with the `test_newtab` with the `before` method and wrappers.

>+++ b/browser/extensions/activity-stream/test/functional/mochitest/head.js	Thu Feb 08 17:56:36 2018 +0200
>+var waitForCondition = async function(conditionFn, errorMsg) {
This should already exist in the content process as `ContentTaskUtils.waitForCondition`

>+function topSitesContextMenuOptions() {
>+  const firstTopSite = content.document.querySelector(".top-sites-list:first-child");
Directly accessing content from chrome is not desired and on the way to being an eslint error, but these helper methods are not being detected right now. Below is a patch that provides a way to add helpers to content:

diff --git a/browser/extensions/activity-stream/test/functional/mochitest/head.js b/browser/extensions/activity-stream/test/functional/mochitest/head.js
index b2aed3d2b3da..dff2536ef941 100644
--- a/browser/extensions/activity-stream/test/functional/mochitest/head.js
+++ b/browser/extensions/activity-stream/test/functional/mochitest/head.js
@@ -58,16 +58,41 @@ async function addHighlightsBookmarks(count) { // eslint-disable-line no-unused-
     // Bookmarks need at least one visit to show up as highlights.
     await PlacesTestUtils.addVisits(placeInfo.url);
   }
 
   // Force HighlightsFeed to make a request for the new items.
   refreshHighlightsFeed();
 }
 
+/**
+ * Helper to add various helpers to the content process by injecting variables
+ * and functions to the `content` global.
+ */
+function addContentHelpers() {
+  const {document} = content;
+  Object.assign(content, {
+    /**
+     * Click the context menu button for an item and get its options list.
+     *
+     * @param selector {String} Selector to get an item (e.g., top site, card)
+     * @return {Array} The nodes for the options.
+     */
+    openContextMenuAndGetOptions(selector) {
+      const item = document.querySelector(selector);
+      const contextButton = item.querySelector(".context-menu-button");
+      contextButton.click();
+
+      const contextMenu = item.querySelector(".context-menu");
+      const contextMenuList = contextMenu.querySelector(".context-menu-list");
+      return [...contextMenuList.getElementsByClassName("context-menu-item")];
+    }
+  });
+}
+
 /**
  * Helper to run Activity Stream about:newtab test tasks in content.
  *
  * @param testInfo {Function|Object}
  *   {Function} This parameter will be used as if the function were called with
  *              an Object with this parameter as "test" key's value.
  *   {Object} The following keys are expected:
  *     before {Function} Optional. Runs before and returns an arg for "test"
@@ -105,16 +130,19 @@ function test_newtab(testInfo) { // eslint-disable-line no-unused-vars
   let testTask = async () => {
     // Open about:newtab without using the default load listener
     let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:newtab", false);
 
     // Specially wait for potentially preloaded browsers
     let browser = tab.linkedBrowser;
     await waitForPreloaded(browser);
 
+    // Add shared helpers to the content process
+    ContentTask.spawn(browser, {}, addContentHelpers);
+
     // Wait for React to render something
     await BrowserTestUtils.waitForCondition(() => ContentTask.spawn(browser, {},
       () => content.document.getElementById("root").children.length),
       "Should render activity stream content");
 
     // Chain together before -> contentTask -> after data passing
     try {
       let contentArg = await before({pushPrefs: scopedPushPrefs, tab});


And here's an example usage that prints:

options: Pin,Edit,Open in a New Window,Open in a New Private Window,Dismiss

diff --git a/browser/extensions/activity-stream/test/functional/mochitest/browser_topsites_section.js b/browser/extensions/activity-stream/test/functional/mochitest/browser_topsites_section.js
index 9325f4cd8432..ec3cb7d9a981 100644
--- a/browser/extensions/activity-stream/test/functional/mochitest/browser_topsites_section.js
+++ b/browser/extensions/activity-stream/test/functional/mochitest/browser_topsites_section.js
@@ -21,16 +21,17 @@ test_newtab({
     // The pref for TopSites is empty by default.
     await pushPrefs(["browser.newtabpage.activity-stream.default.sites", "https://www.youtube.com/,https://www.facebook.com/,https://www.amazon.com/,https://www.reddit.com/,https://www.wikipedia.org/,https://twitter.com/"]);
     // Toggle the feed off and on as a workaround to read the new prefs.
     await pushPrefs(["browser.newtabpage.activity-stream.feeds.topsites", false]);
     await pushPrefs(["browser.newtabpage.activity-stream.feeds.topsites", true]);
   },
   // it should pin the website when we click the first option of the topsite context menu.
   test: async function topsites_pin_unpin() {
+    dump("options: " + content.openContextMenuAndGetOptions(".top-sites-list:first-child").map(v => v.textContent) + "\n");
     await ContentTaskUtils.waitForCondition(() => content.document.querySelector(".top-site-icon"),
       "Topsite tippytop icon not found");
     // There are only topsites on the page, the selector with find the first topsite menu button.
     const topsiteContextBtn = content.document.querySelector(".context-menu-button");
     topsiteContextBtn.click();
 
     const contextMenu = content.document.querySelector(".context-menu");
     ok(contextMenu && !contextMenu.hidden, "Should find a visible topsite context menu");
Attachment #8949403 - Flags: review?(edilee) → review-
andreio, were there content helpers that you wanted to share if something like comment 2's `addContentHelpers` existed?
Flags: needinfo?(andrei.br92)
I looked through the opened issues. Having access to the menu options and clicking them covers a lot of the tests. Another useful helper would be toggling the sections but with the sidebar going away soon it's probably fine to just toggle prefs and assert the result.
Flags: needinfo?(andrei.br92)
Attached patch defaultTopSites_menuItems (obsolete) — Splinter Review
Attachment #8949403 - Attachment is obsolete: true
Attachment #8952114 - Flags: review?(edilee)
Comment on attachment 8952114 [details] [diff] [review]
defaultTopSites_menuItems

>+++ b/browser/extensions/activity-stream/test/functional/mochitest/browser_topsites_contextMenu_options.js	Mon Feb 19 10:58:20 2018 +0200
>+   async before({pushPrefs}) {
>+     // The pref for TopSites is empty by default.
>+     await pushPrefs(["browser.newtabpage.activity-stream.default.sites", "https://www.youtube.com/,https://www.facebook.com/,https://www.amazon.com/,https://www.reddit.com/,https://www.wikipedia.org/,https://twitter.com/"]);
>+     // Toggle the feed off and on as a workaround to read the new prefs.
>+     await pushPrefs(["browser.newtabpage.activity-stream.feeds.topsites", false]);
>+     await pushPrefs(["browser.newtabpage.activity-stream.feeds.topsites", true]);
If we'll end up needing to do this frequently, we'll probably want some helper, e.g.,

function setDefaultTopSites(sites = "https://www.youtube.com/") {
  return async({pushPrefs}) => {
    await pushPrefs…
    …
  };
}

test_newtab({
  before: setDefaultTopSites(),
  test: …
})
Attachment #8952114 - Flags: review?(edilee) → review+
Attached patch defaultTopSites_menuItems_1 (obsolete) — Splinter Review
Attachment #8952114 - Attachment is obsolete: true
Attachment #8952355 - Flags: review?(edilee)
Comment on attachment 8952355 [details] [diff] [review]
defaultTopSites_menuItems_1

>+     // The pref for TopSites is empty by default.
>+     await setDefaultTopSites();
>+     // Toggle the feed off and on as a workaround to read the new prefs.
>+     await pushPrefs(["browser.newtabpage.activity-stream.feeds.topsites", false]);
>+     await pushPrefs(["browser.newtabpage.activity-stream.feeds.topsites", true]);
If we're going to move the setDefault to a helper, it should include the workaround to toggle the feeds pref as both parts are needed to actually get things working, and ideally it would eventually get fixed in the one helper instead of all the callers.

Let's update the other caller to use it too: https://searchfox.org/mozilla-central/source/browser/extensions/activity-stream/test/functional/mochitest/browser_topsites_section.js#21-25
Attachment #8952355 - Flags: review?(edilee) → review-
Attached patch defaultTopSites_menuItems_2 (obsolete) — Splinter Review
Attachment #8952355 - Attachment is obsolete: true
Attachment #8952712 - Flags: review?(edilee)
Comment on attachment 8952712 [details] [diff] [review]
defaultTopSites_menuItems_2

>+++ b/browser/extensions/activity-stream/test/functional/mochitest/browser_topsites_section.js	Mon Feb 19 10:58:20 2018 +0200
>-  // it should pin the website when we click the first option of the topsite context menu.
Let's avoid removing this comment
Attachment #8952712 - Flags: review?(edilee) → review+
Attached patch defaultTopSites_menuItems_3 (obsolete) — Splinter Review
Attachment #8952712 - Attachment is obsolete: true
Keywords: checkin-needed
When trying to apply your patch it has conflicts, please rebase the patch with the lastest mozilla-central changes.
Flags: needinfo?(icrisan)
Attachment #8952977 - Attachment is obsolete: true
Flags: needinfo?(icrisan)
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bebe83ac3b64
Add a test to verify the menu options for a default top site. r=Mardak
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/bebe83ac3b64
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Commit pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/64b8b6759becdf58a3bf395961106baa84902233
chore(mc): Port Bug 1410818 - Add a test to verify the menu options for a default top site. r=Mardak (#3992)
Blocks: 1441984
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.