From: Timo Tijhof Date: Wed, 2 May 2018 03:48:36 +0000 (+0100) Subject: selenium: Minor clean-up in preparation for packaging X-Git-Tag: 1.34.0-rc.0~5498^2 X-Git-Url: https://git.cyclocoop.org/%27.WWW_URL.%27admin/?a=commitdiff_plain;h=2729bf4653ca87c427f696f53624217d62096a25;p=lhc%2Fweb%2Fwiklou.git selenium: Minor clean-up in preparation for packaging This is functionally a no-op, purely refactoring (mostly style). * Consistently require packages at the top of a file. (e.g. MWBot in edit.page.js). * Remove unused .call(this) from mwbot interaction closures, which didn't use 'this'. * Use Node.js regular Promise chaining with then(), instead of complex bluebird.coroutine generator function yields, which are intended to emulate async-await, but the syntax is quite error-prone for inexperienced developers and hard to debug. Once we require Node 7+ for the selenium tests, we can use async-await here natively, but until then, might as well use regular then() syntax, which we already use elsewhere in the tests, and is also what MWBot documentation uses. * Also applied some minor whitespace changes for consistency among these files and other MediaWiki JS. E.g. no empty line before the first statement of a function. Add a new line between different methods, and between the end of a class and the export statement. * Remove 'use strict' from test files. The patterns that would expose the bad non-strict behaviour are mostly already forbidden by ESLint, and the run-time optimisation to disable non-strict can't be noticed in tests (more useful in prod where e.g. the same process would run a function 1 million times). Main reason here is to keep things simple for new-comers and reduce boilerplate, given that these tests will mainly be worked on by browser-JS developers, not Node.js devs, and we don't currently use strict mode in our front-end code, either. * Remove unused bluebird dependency. Bug: T193088 Change-Id: I59f9211299e8e884c28c7733bcee3b7b28542610 --- diff --git a/package.json b/package.json index 7b779a6795..d6fd1b9e95 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,6 @@ "selenium-test": "wdio ./tests/selenium/wdio.conf.js" }, "devDependencies": { - "bluebird": "3.5.1", "deepmerge": "1.3.2", "eslint": "4.9.0", "eslint-config-wikimedia": "0.5.0", diff --git a/tests/selenium/pageobjects/createaccount.page.js b/tests/selenium/pageobjects/createaccount.page.js index 105f40924e..a0b70a32b9 100644 --- a/tests/selenium/pageobjects/createaccount.page.js +++ b/tests/selenium/pageobjects/createaccount.page.js @@ -1,5 +1,6 @@ -'use strict'; -const Page = require( './page' ); +const Page = require( './page' ), + // https://github.com/Fannon/mwbot + MWBot = require( 'mwbot' ); class CreateAccountPage extends Page { @@ -22,18 +23,14 @@ class CreateAccountPage extends Page { } apiCreateAccount( username, password ) { - - const MWBot = require( 'mwbot' ), // https://github.com/Fannon/mwbot - Promise = require( 'bluebird' ); let bot = new MWBot(); - return Promise.coroutine( function* () { - yield bot.loginGetCreateaccountToken( { - apiUrl: `${browser.options.baseUrl}/api.php`, - username: browser.options.username, - password: browser.options.password - } ); - yield bot.request( { + return bot.loginGetCreateaccountToken( { + apiUrl: `${browser.options.baseUrl}/api.php`, + username: browser.options.username, + password: browser.options.password + } ).then( function () { + return bot.request( { action: 'createaccount', createreturnurl: browser.options.baseUrl, createtoken: bot.createaccountToken, @@ -41,9 +38,8 @@ class CreateAccountPage extends Page { password: password, retype: password } ); - } ).call( this ); - + } ); } - } + module.exports = new CreateAccountPage(); diff --git a/tests/selenium/pageobjects/delete.page.js b/tests/selenium/pageobjects/delete.page.js index d43cb9f612..ec034092de 100644 --- a/tests/selenium/pageobjects/delete.page.js +++ b/tests/selenium/pageobjects/delete.page.js @@ -1,8 +1,8 @@ -'use strict'; -const Page = require( './page' ); +const Page = require( './page' ), + // https://github.com/Fannon/mwbot + MWBot = require( 'mwbot' ); class DeletePage extends Page { - get reason() { return browser.element( '#wpReason' ); } get watch() { return browser.element( '#wpWatch' ); } get submit() { return browser.element( '#wpConfirmB' ); } @@ -19,21 +19,16 @@ class DeletePage extends Page { } apiDelete( name, reason ) { - - const MWBot = require( 'mwbot' ), // https://github.com/Fannon/mwbot - Promise = require( 'bluebird' ); let bot = new MWBot(); - return Promise.coroutine( function* () { - yield bot.loginGetEditToken( { - apiUrl: `${browser.options.baseUrl}/api.php`, - username: browser.options.username, - password: browser.options.password - } ); - yield bot.delete( name, reason ); - } ).call( this ); - + return bot.loginGetEditToken( { + apiUrl: `${browser.options.baseUrl}/api.php`, + username: browser.options.username, + password: browser.options.password + } ).then( function () { + return bot.delete( name, reason ); + } ); } - } + module.exports = new DeletePage(); diff --git a/tests/selenium/pageobjects/edit.page.js b/tests/selenium/pageobjects/edit.page.js index 33a27f0f8c..a1784f4a78 100644 --- a/tests/selenium/pageobjects/edit.page.js +++ b/tests/selenium/pageobjects/edit.page.js @@ -1,8 +1,8 @@ -'use strict'; -const Page = require( './page' ); +const Page = require( './page' ), + // https://github.com/Fannon/mwbot + MWBot = require( 'mwbot' ); class EditPage extends Page { - get content() { return browser.element( '#wpTextbox1' ); } get displayedContent() { return browser.element( '#mw-content-text' ); } get heading() { return browser.element( '#firstHeading' ); } @@ -19,21 +19,16 @@ class EditPage extends Page { } apiEdit( name, content ) { - - const MWBot = require( 'mwbot' ), // https://github.com/Fannon/mwbot - Promise = require( 'bluebird' ); let bot = new MWBot(); - return Promise.coroutine( function* () { - yield bot.loginGetEditToken( { - apiUrl: `${browser.options.baseUrl}/api.php`, - username: browser.options.username, - password: browser.options.password - } ); - yield bot.edit( name, content, `Created page with "${content}"` ); - } ).call( this ); - + return bot.loginGetEditToken( { + apiUrl: `${browser.options.baseUrl}/api.php`, + username: browser.options.username, + password: browser.options.password + } ).then( function () { + return bot.edit( name, content, `Created page with "${content}"` ); + } ); } - } + module.exports = new EditPage(); diff --git a/tests/selenium/pageobjects/history.page.js b/tests/selenium/pageobjects/history.page.js index 869484e627..60d7fd4a0b 100644 --- a/tests/selenium/pageobjects/history.page.js +++ b/tests/selenium/pageobjects/history.page.js @@ -1,13 +1,11 @@ -'use strict'; const Page = require( './page' ); class HistoryPage extends Page { - get comment() { return browser.element( '#pagehistory .comment' ); } open( name ) { super.open( name + '&action=history' ); } - } + module.exports = new HistoryPage(); diff --git a/tests/selenium/pageobjects/page.js b/tests/selenium/pageobjects/page.js index 77bb1f4ec7..0974086158 100644 --- a/tests/selenium/pageobjects/page.js +++ b/tests/selenium/pageobjects/page.js @@ -1,8 +1,11 @@ -// From http://webdriver.io/guide/testrunner/pageobjects.html -'use strict'; +/** + * Based on http://webdriver.io/guide/testrunner/pageobjects.html + */ + class Page { open( path ) { browser.url( browser.options.baseUrl + '/index.php?title=' + path ); } } + module.exports = Page; diff --git a/tests/selenium/pageobjects/preferences.page.js b/tests/selenium/pageobjects/preferences.page.js index 98b87fe9cb..9456b61dd0 100644 --- a/tests/selenium/pageobjects/preferences.page.js +++ b/tests/selenium/pageobjects/preferences.page.js @@ -1,8 +1,6 @@ -'use strict'; const Page = require( './page' ); class PreferencesPage extends Page { - get realName() { return browser.element( '#mw-input-wprealname' ); } get save() { return browser.element( '#prefcontrol' ); } @@ -15,6 +13,6 @@ class PreferencesPage extends Page { this.realName.setValue( realName ); this.save.click(); } - } + module.exports = new PreferencesPage(); diff --git a/tests/selenium/pageobjects/restore.page.js b/tests/selenium/pageobjects/restore.page.js index 071f7f9883..be5be8c6c2 100644 --- a/tests/selenium/pageobjects/restore.page.js +++ b/tests/selenium/pageobjects/restore.page.js @@ -1,4 +1,3 @@ -'use strict'; const Page = require( './page' ); class RestorePage extends Page { @@ -16,6 +15,6 @@ class RestorePage extends Page { this.reason.setValue( reason ); this.submit.click(); } - } + module.exports = new RestorePage(); diff --git a/tests/selenium/pageobjects/userlogin.page.js b/tests/selenium/pageobjects/userlogin.page.js index 0061d0c258..557fb6b5a7 100644 --- a/tests/selenium/pageobjects/userlogin.page.js +++ b/tests/selenium/pageobjects/userlogin.page.js @@ -1,8 +1,6 @@ -'use strict'; const Page = require( './page' ); class UserLoginPage extends Page { - get username() { return browser.element( '#wpName1' ); } get password() { return browser.element( '#wpPassword1' ); } get loginButton() { return browser.element( '#wpLoginAttempt' ); } @@ -22,6 +20,6 @@ class UserLoginPage extends Page { loginAdmin() { this.login( browser.options.username, browser.options.password ); } - } + module.exports = new UserLoginPage(); diff --git a/tests/selenium/specs/page.js b/tests/selenium/specs/page.js index 376dce5975..197a235af4 100644 --- a/tests/selenium/specs/page.js +++ b/tests/selenium/specs/page.js @@ -1,4 +1,3 @@ -'use strict'; const assert = require( 'assert' ), DeletePage = require( '../pageobjects/delete.page' ), RestorePage = require( '../pageobjects/restore.page' ), @@ -7,7 +6,6 @@ const assert = require( 'assert' ), UserLoginPage = require( '../pageobjects/userlogin.page' ); describe( 'Page', function () { - var content, name; @@ -28,14 +26,12 @@ describe( 'Page', function () { } ); it( 'should be creatable', function () { - // create EditPage.edit( name, content ); // check assert.equal( EditPage.heading.getText(), name ); assert.equal( EditPage.displayedContent.getText(), content ); - } ); it( 'should be re-creatable', function () { @@ -61,7 +57,6 @@ describe( 'Page', function () { } ); it( 'should be editable', function () { - // create browser.call( function () { return EditPage.apiEdit( name, content ); @@ -73,11 +68,9 @@ describe( 'Page', function () { // check assert.equal( EditPage.heading.getText(), name ); assert.equal( EditPage.displayedContent.getText(), content ); - } ); it( 'should have history', function () { - // create browser.call( function () { return EditPage.apiEdit( name, content ); @@ -86,11 +79,9 @@ describe( 'Page', function () { // check HistoryPage.open( name ); assert.equal( HistoryPage.comment.getText(), `(Created page with "${content}")` ); - } ); it( 'should be deletable', function () { - // login UserLoginPage.loginAdmin(); @@ -107,11 +98,9 @@ describe( 'Page', function () { DeletePage.displayedContent.getText(), '"' + name + '" has been deleted. See deletion log for a record of recent deletions.\nReturn to Main Page.' ); - } ); it( 'should be restorable', function () { - // login UserLoginPage.loginAdmin(); @@ -130,7 +119,5 @@ describe( 'Page', function () { // check assert.equal( RestorePage.displayedContent.getText(), name + ' has been restored\nConsult the deletion log for a record of recent deletions and restorations.' ); - } ); - } ); diff --git a/tests/selenium/specs/user.js b/tests/selenium/specs/user.js index 3f3872dc7d..62aac05491 100644 --- a/tests/selenium/specs/user.js +++ b/tests/selenium/specs/user.js @@ -1,11 +1,9 @@ -'use strict'; const assert = require( 'assert' ), CreateAccountPage = require( '../pageobjects/createaccount.page' ), PreferencesPage = require( '../pageobjects/preferences.page' ), UserLoginPage = require( '../pageobjects/userlogin.page' ); describe( 'User', function () { - var password, username; @@ -22,17 +20,14 @@ describe( 'User', function () { } ); it( 'should be able to create account', function () { - // create CreateAccountPage.createAccount( username, password ); // check assert.equal( CreateAccountPage.heading.getText(), `Welcome, ${username}!` ); - } ); it( 'should be able to log in', function () { - // create browser.call( function () { return CreateAccountPage.apiCreateAccount( username, password ); @@ -43,11 +38,9 @@ describe( 'User', function () { // check assert.equal( UserLoginPage.userPage.getText(), username ); - } ); it( 'should be able to change preferences', function () { - var realName = Math.random().toString(); // create @@ -63,7 +56,5 @@ describe( 'User', function () { // check assert.equal( PreferencesPage.realName.getValue(), realName ); - } ); - } ); diff --git a/tests/selenium/wdio.conf.js b/tests/selenium/wdio.conf.js index ca9f8465c4..5399fa4938 100644 --- a/tests/selenium/wdio.conf.js +++ b/tests/selenium/wdio.conf.js @@ -1,5 +1,3 @@ -'use strict'; - const fs = require( 'fs' ), path = require( 'path' ), logPath = process.env.LOG_DIR || './log/';