From c16c20a39446067bba970f592384f0e4a3b58752 Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Mon, 8 Apr 2019 14:11:33 -0700 Subject: [PATCH] QA: Port Notifications browser test to Node.js Uncover a bug and fix it in the process \o/ - it seems that the close icon is misplaced between clicking the notifications icon and loading the contents of the overlay - this confuses the webdriver as the button is not clickable. Bug: T219920 Change-Id: Ib4d076fd9b7ea1cd48b6b58940a50560eacd51a0 --- .../mainmenu.less | 14 ++++++++ .../mobile.notifications.overlay/minerva.less | 14 -------- tests/selenium/README.md | 2 +- .../features/notification.feature | 4 --- .../features/step_definitions/common_steps.js | 30 +++++++++++++++++ .../step_definitions/create_page_api_steps.js | 7 +--- .../features/step_definitions/index.js | 14 +++++++- .../step_definitions/notification_steps.js | 32 +++++++++++++++++++ .../step_definitions/overlay_steps.js | 0 .../features/support/pages/article_page.js | 1 + .../pages/article_page_with_overlay.js | 19 +++++++++++ tests/selenium/specs/notification.js | 31 ++++++++++++++++++ 12 files changed, 142 insertions(+), 26 deletions(-) rename tests/{browser => selenium}/features/notification.feature (88%) create mode 100644 tests/selenium/features/step_definitions/notification_steps.js create mode 100644 tests/selenium/features/step_definitions/overlay_steps.js create mode 100644 tests/selenium/features/support/pages/article_page_with_overlay.js create mode 100644 tests/selenium/specs/notification.js diff --git a/resources/skins.minerva.mainMenu.styles/mainmenu.less b/resources/skins.minerva.mainMenu.styles/mainmenu.less index 63c90ca..975d411 100644 --- a/resources/skins.minerva.mainMenu.styles/mainmenu.less +++ b/resources/skins.minerva.mainMenu.styles/mainmenu.less @@ -180,6 +180,20 @@ .notifications-overlay.navigation-drawer { left: @rightDrawerLeftOffset; + + .overlay-header { + padding-left: 0; + padding-right: 0; + margin: 0; // T210191 + width: 100%; // T218731 + // T170903 + max-width: none; + + .cancel { + // 0 because we want to have some tappable area to the left of the icon + left: 0; + } + } } } diff --git a/skinStyles/mobile.notifications.overlay/minerva.less b/skinStyles/mobile.notifications.overlay/minerva.less index 4a56bc9..ba5ea1c 100644 --- a/skinStyles/mobile.notifications.overlay/minerva.less +++ b/skinStyles/mobile.notifications.overlay/minerva.less @@ -14,19 +14,5 @@ .mw-echo-notification { padding: 1.75em @contentPaddingTablet; } - - .overlay-header .cancel { - // 0 because we want to have some tappable area to the left of the icon - left: 0; - } - - .overlay-header { - padding-left: 0; - padding-right: 0; - margin: 0; // T210191 - width: 100%; // T218731 - // T170903 - max-width: none; - } } } diff --git a/tests/selenium/README.md b/tests/selenium/README.md index 81e0e83..98218e5 100644 --- a/tests/selenium/README.md +++ b/tests/selenium/README.md @@ -58,7 +58,7 @@ Example: https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/501792 ## Step 2 - add boilerplate for missing steps Run the feature in cucumber ``` -./node_modules/.bin/wdio tests/selenium/wdio.conf.cucumber.js --spec tests/selenium/features/.feature +npm run selenium-test-cucumber -- --spec tests/selenium/features/.feature ``` You will get warnings such as: diff --git a/tests/browser/features/notification.feature b/tests/selenium/features/notification.feature similarity index 88% rename from tests/browser/features/notification.feature rename to tests/selenium/features/notification.feature index 0a93fcb..4a5c5d2 100644 --- a/tests/browser/features/notification.feature +++ b/tests/selenium/features/notification.feature @@ -5,10 +5,6 @@ Feature: Notification Given I am logged into the mobile website And I have no notifications When I click on the notification icon - And the notifications overlay appears - - @smoke - Scenario: Opening notifications Then I should see the notifications overlay Scenario: Closing notifications (overlay button) diff --git a/tests/selenium/features/step_definitions/common_steps.js b/tests/selenium/features/step_definitions/common_steps.js index 1421392..695cff1 100644 --- a/tests/selenium/features/step_definitions/common_steps.js +++ b/tests/selenium/features/step_definitions/common_steps.js @@ -1,7 +1,14 @@ const assert = require( 'assert' ), Api = require( 'wdio-mediawiki/Api' ), + ArticlePageWithOverlay = require( '../support/pages/article_page_with_overlay' ), { ArticlePage, UserLoginPage, api } = require( '../support/world.js' ); +const waitForPropagation = ( timeMs ) => { + // wait 2 seconds so the change can propogate. + const d = new Date(); + browser.waitUntil( () => new Date() - d > timeMs ); +}; + const login = () => { return api.loginGetEditToken( { username: browser.options.username, @@ -64,7 +71,30 @@ const iShouldSeeAToastNotification = () => { ArticlePage.notification_element.waitForVisible(); }; +const iClickTheBrowserBackButton = () => { + browser.back(); +}; + +const iClickTheOverlayCloseButton = () => { + waitForPropagation( 2000 ); + ArticlePageWithOverlay.overlay_close_element.click(); +}; + +const iSeeAnOverlay = () => { + ArticlePageWithOverlay.overlay_element.waitForVisible(); + assert.strictEqual( ArticlePageWithOverlay.overlay_element.isVisible(), true ); +}; + +const iDoNotSeeAnOverlay = () => { + browser.waitUntil( () => !ArticlePageWithOverlay.overlay_element.isVisible() ); + assert.strictEqual( ArticlePageWithOverlay.overlay_element.isVisible(), false ); +}; + module.exports = { + waitForPropagation, + iSeeAnOverlay, iDoNotSeeAnOverlay, + iClickTheOverlayCloseButton, + iClickTheBrowserBackButton, createPage, createPages, pageExists, iAmOnAPageThatDoesNotExist, iShouldSeeAToastNotification, iAmLoggedIntoTheMobileWebsite, diff --git a/tests/selenium/features/step_definitions/create_page_api_steps.js b/tests/selenium/features/step_definitions/create_page_api_steps.js index 24b8519..82d7dce 100644 --- a/tests/selenium/features/step_definitions/create_page_api_steps.js +++ b/tests/selenium/features/step_definitions/create_page_api_steps.js @@ -2,16 +2,11 @@ const { api, ArticlePage } = require( '../support/world' ); const Api = require( 'wdio-mediawiki/Api' ); const { iAmOnPage, + waitForPropagation, createPages, createPage } = require( './common_steps' ); -const waitForPropagation = ( timeMs ) => { - // wait 2 seconds so the change can propogate. - const d = new Date(); - browser.waitUntil( () => new Date() - d > timeMs ); -}; - const iAmInAWikiThatHasCategories = ( title ) => { const msg = 'This page is used by Selenium to test category related features.', wikitext = ` diff --git a/tests/selenium/features/step_definitions/index.js b/tests/selenium/features/step_definitions/index.js index 397adf0..54b7c3b 100644 --- a/tests/selenium/features/step_definitions/index.js +++ b/tests/selenium/features/step_definitions/index.js @@ -7,7 +7,7 @@ const { defineSupportCode } = require( 'cucumber' ), iGoToAPageThatHasLanguages } = require( './create_page_api_steps' ), { pageExists, iAmOnAPageThatDoesNotExist, iShouldSeeAToastNotification, - iAmUsingTheMobileSite, + iAmUsingTheMobileSite, iClickTheBrowserBackButton, iAmLoggedIntoTheMobileWebsite, iAmOnPage, iAmInBetaMode } = require( './common_steps' ), @@ -21,6 +21,10 @@ const { defineSupportCode } = require( 'cucumber' ), iTypeIntoTheEditor, iClickContinue, iClickSubmit, iSayOkayInTheConfirmDialog, theTextOfTheFirstHeadingShouldBe, thereShouldBeARedLinkWithText } = require( './editor_steps' ), + { iHaveNoNotifications, iClickOnTheNotificationIcon, + iShouldSeeTheNotificationsOverlay, iClickTheNotificationsOverlayCloseButton, + iShouldNotSeeTheNotificationsOverlay + } = require( './notification_steps' ), { iClickOnTheHistoryLinkInTheLastModifiedBar } = require( './history_steps' ); @@ -48,6 +52,7 @@ defineSupportCode( function ( { Then, When, Given } ) { Given( /^I am logged into the mobile website$/, iAmLoggedIntoTheMobileWebsite ); Then( /^I should see a toast notification$/, iShouldSeeAToastNotification ); + When( /I click the browser back button/, iClickTheBrowserBackButton ); // Page steps Given( /^I am in a wiki that has categories$/, () => { @@ -67,6 +72,13 @@ defineSupportCode( function ( { Then, When, Given } ) { Then( /^I should see "(.*?)" as added content$/, iShouldSeeAddedContent ); Then( /^I should see "(.*?)" as removed content$/, iShouldSeeRemovedContent ); + // notifications + Then( /I have no notifications/, iHaveNoNotifications ); + When( /I click on the notification icon/, iClickOnTheNotificationIcon ); + When( /I click the notifications overlay close button/, iClickTheNotificationsOverlayCloseButton ); + Then( /after 1 seconds I should not see the notifications overlay/, iShouldNotSeeTheNotificationsOverlay ); + Then( /I should see the notifications overlay/, iShouldSeeTheNotificationsOverlay ); + // Category steps When( /^I click on the category button$/, iClickOnTheCategoryButton ); diff --git a/tests/selenium/features/step_definitions/notification_steps.js b/tests/selenium/features/step_definitions/notification_steps.js new file mode 100644 index 0000000..5f05281 --- /dev/null +++ b/tests/selenium/features/step_definitions/notification_steps.js @@ -0,0 +1,32 @@ +const ArticlePage = require( '../support/pages/article_page' ); +const { iClickTheOverlayCloseButton, iSeeAnOverlay, iDoNotSeeAnOverlay } = require( './common_steps' ); + +const iHaveNoNotifications = () => { + ArticlePage.notifications_button_element.waitForVisible(); + // This is somewhat hacky, but we don't want this test making use of + // Echo's APIs which may change + browser.execute( '$( function () { $( ".notification-count span" ).hide(); } );' ); +}; + +const iClickOnTheNotificationIcon = () => { + ArticlePage.waitUntilResourceLoaderModuleReady( 'skins.minerva.notifications' ); + ArticlePage.notifications_button_element.click(); +}; + +const iShouldSeeTheNotificationsOverlay = () => { + iSeeAnOverlay(); +}; + +const iClickTheNotificationsOverlayCloseButton = () => { + iClickTheOverlayCloseButton(); +}; + +const iShouldNotSeeTheNotificationsOverlay = () => { + iDoNotSeeAnOverlay(); +}; + +module.exports = { + iHaveNoNotifications, iClickOnTheNotificationIcon, + iShouldSeeTheNotificationsOverlay, iClickTheNotificationsOverlayCloseButton, + iShouldNotSeeTheNotificationsOverlay +}; diff --git a/tests/selenium/features/step_definitions/overlay_steps.js b/tests/selenium/features/step_definitions/overlay_steps.js new file mode 100644 index 0000000..e69de29 diff --git a/tests/selenium/features/support/pages/article_page.js b/tests/selenium/features/support/pages/article_page.js index 8ee6c1b..2f3f76e 100644 --- a/tests/selenium/features/support/pages/article_page.js +++ b/tests/selenium/features/support/pages/article_page.js @@ -10,6 +10,7 @@ const MinervaPage = require( './minerva_page' ); class ArticlePage extends MinervaPage { + get notifications_button_element() { return $( '.user-button' ); } get category_element() { return $( '.category-button' ); } get edit_link_element() { return $( '#ca-edit' ); } get first_heading_element() { return $( '#section_0' ); } diff --git a/tests/selenium/features/support/pages/article_page_with_overlay.js b/tests/selenium/features/support/pages/article_page_with_overlay.js new file mode 100644 index 0000000..50e82fc --- /dev/null +++ b/tests/selenium/features/support/pages/article_page_with_overlay.js @@ -0,0 +1,19 @@ +/** + * Represents a generic article page with the editor overlay open + * + * @class ArticlePageWithOverlay + * @extends MinervaPage + * @example + * https://en.m.wikipedia.org/wiki/Barack_Obama#/editor/0 + */ + +const MinervaPage = require( './minerva_page' ); + +class ArticlePageWithOverlay extends MinervaPage { + get overlay_element() { return $( '.overlay' ); } + + // overlay components + get overlay_close_element() { return $( '.overlay .cancel' ); } +} + +module.exports = new ArticlePageWithOverlay(); diff --git a/tests/selenium/specs/notification.js b/tests/selenium/specs/notification.js new file mode 100644 index 0000000..58c99af --- /dev/null +++ b/tests/selenium/specs/notification.js @@ -0,0 +1,31 @@ +const + { + iClickTheBrowserBackButton, + iAmLoggedIntoTheMobileWebsite + } = require( './../features/step_definitions/common_steps' ), + { iHaveNoNotifications, iClickOnTheNotificationIcon, + iShouldSeeTheNotificationsOverlay, iClickTheNotificationsOverlayCloseButton, + iShouldNotSeeTheNotificationsOverlay + } = require( './../features/step_definitions/notification_steps' ); + +// @chrome @en.m.wikipedia.beta.wmflabs.org @extension-echo +// @firefox @vagrant @login +describe( 'Feature: Notification', () => { + + beforeEach( () => { + iAmLoggedIntoTheMobileWebsite(); + iHaveNoNotifications(); + iClickOnTheNotificationIcon(); + iShouldSeeTheNotificationsOverlay(); + } ); + + it( 'Closing notifications (overlay button)', () => { + iClickTheNotificationsOverlayCloseButton(); + iShouldNotSeeTheNotificationsOverlay(); + } ); + + it( 'Closing notifications (browser button)', () => { + iClickTheBrowserBackButton(); + iShouldNotSeeTheNotificationsOverlay(); + } ); +} );