Closed
Bug 1410818
Opened 7 years ago
Closed 7 years ago
Add automated test for "Hover over Top Sites tiles"
Categories
(Firefox :: New Tab Page, enhancement, P3)
Firefox
New Tab Page
Tracking
()
RESOLVED
FIXED
Firefox 60
People
(Reporter: icrisan, Assigned: icrisan)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 5 obsolete files)
5.94 KB,
patch
|
Details | Diff | Splinter Review | |
52 bytes,
text/x-github-pull-request
|
Details | Review |
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.
Assignee | ||
Updated•7 years ago
|
Component: General → Activity Streams: Newtab
Product: Core → Firefox
Summary: Hover over Top Sites tiles → Add automated test for "Hover over Top Sites tiles"
Updated•7 years ago
|
status-firefox57:
--- → wontfix
Updated•7 years ago
|
status-firefox58:
--- → wontfix
Updated•7 years ago
|
Priority: -- → P3
Assignee | ||
Comment 1•7 years ago
|
||
Assignee: nobody → icrisan
Attachment #8949403 -
Flags: review?(edilee)
Comment 2•7 years ago
|
||
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-
Comment 3•7 years ago
|
||
andreio, were there content helpers that you wanted to share if something like comment 2's `addContentHelpers` existed?
Flags: needinfo?(andrei.br92)
Comment 4•7 years ago
|
||
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)
Assignee | ||
Comment 5•7 years ago
|
||
Attachment #8949403 -
Attachment is obsolete: true
Attachment #8952114 -
Flags: review?(edilee)
Assignee | ||
Comment 6•7 years ago
|
||
Comment 7•7 years ago
|
||
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+
Assignee | ||
Comment 8•7 years ago
|
||
Attachment #8952114 -
Attachment is obsolete: true
Attachment #8952355 -
Flags: review?(edilee)
Assignee | ||
Comment 9•7 years ago
|
||
Comment 10•7 years ago
|
||
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-
Assignee | ||
Comment 11•7 years ago
|
||
Attachment #8952355 -
Attachment is obsolete: true
Attachment #8952712 -
Flags: review?(edilee)
Assignee | ||
Comment 12•7 years ago
|
||
Comment 13•7 years ago
|
||
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+
Assignee | ||
Comment 14•7 years ago
|
||
Attachment #8952712 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 15•7 years ago
|
||
When trying to apply your patch it has conflicts, please rebase the patch with the lastest mozilla-central changes.
Flags: needinfo?(icrisan)
Assignee | ||
Comment 16•7 years ago
|
||
Attachment #8952977 -
Attachment is obsolete: true
Flags: needinfo?(icrisan)
Comment 17•7 years ago
|
||
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
Comment 18•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Comment 19•7 years ago
|
||
Comment 20•7 years ago
|
||
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)
Updated•6 years ago
|
Component: Activity Streams: Newtab → New Tab Page
You need to log in
before you can comment on or make changes to this bug.
Description
•