From 979cf274e91fa5ec997cc04930a4601ddbff04e7 Mon Sep 17 00:00:00 2001 From: Chris Egner Date: Wed, 9 Apr 2014 16:13:45 -0500 Subject: [PATCH 1/9] Standardize return - important for callbacks. --- lib/selenium/commands/waitForElementPresent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/selenium/commands/waitForElementPresent.js b/lib/selenium/commands/waitForElementPresent.js index 5a5dded19c..cdd1e27221 100644 --- a/lib/selenium/commands/waitForElementPresent.js +++ b/lib/selenium/commands/waitForElementPresent.js @@ -50,7 +50,7 @@ WaitForElementPresent.prototype.elementNotFound = function(result, now) { } var defaultMsg = 'Timed out while waiting for element <%s> to be present for %d milliseconds.'; - return this.fail(false, 'not found', this.expectedValue, defaultMsg); + return this.fail({value:false}, 'not found', this.expectedValue, defaultMsg); }; module.exports = WaitForElementPresent; From a49d51844c909733c8f07110773f2a8aa0db40f0 Mon Sep 17 00:00:00 2001 From: Jonathan Haggarty Date: Tue, 15 Apr 2014 11:56:45 +0100 Subject: [PATCH 2/9] add global timeout for waitFor commands. Fixes #129 --- lib/selenium/commands/_waitForElement.js | 29 +++++++++++++++---- .../src/commands/testWaitForElementVisible.js | 9 ++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/lib/selenium/commands/_waitForElement.js b/lib/selenium/commands/_waitForElement.js index ba147fcf12..e0921a8c4c 100644 --- a/lib/selenium/commands/_waitForElement.js +++ b/lib/selenium/commands/_waitForElement.js @@ -27,6 +27,7 @@ util.inherits(WaitForElement, events.EventEmitter); /*! * The public command function which will be called by the test runner. Arguments can be passed in a variety of ways: * + * waitForElement('body') * waitForElement('body', 500) * waitForElement('body', 500, 'test message) * waitForElement('body', 500, function() { .. }); @@ -41,10 +42,8 @@ util.inherits(WaitForElement, events.EventEmitter); * @returns {WaitForElement} */ WaitForElement.prototype.command = function(selector, milliseconds, callbackOrAbort) { - if (typeof milliseconds !== 'number') { - throw new Error('waitForElement expects second parameter to be number; ' + - typeof (milliseconds) + ' given'); - } + assertMilliseconds.call(this, milliseconds) + this.startTimer = new Date().getTime(); if (typeof arguments[2] === 'boolean') { @@ -69,7 +68,7 @@ WaitForElement.prototype.command = function(selector, milliseconds, callbackOrAb this.message = lastArgument; } - this.ms = milliseconds; + this.ms = milliseconds || this.client.api.globals.waitForConditionTimeout; this.selector = selector; this.checkElement(); return this; @@ -201,5 +200,23 @@ function format(f) { return str; } +/** + * @param {number} [timeoutMs] + * @throws Will throw an error if the argument is invalid + * @returns {number} + */ +function assertMilliseconds(timeoutMs) { + var globalTimeout = this.client.api.globals.waitForConditionTimeout; + + if (!timeoutMs && typeof globalTimeout !== 'number') { + throw new Error('waitForElement expects second parameter to have a global default ' + + '(waitForConditionTimeout) to be specified if not passed as the second parameter '); + } + else if (timeoutMs && typeof timeoutMs !== 'number') { + throw new Error('waitForElement expects second parameter to be number; ' + + typeof (timeoutMs) + ' given'); + } +} + -module.exports = WaitForElement; +module.exports = WaitForElement; \ No newline at end of file diff --git a/tests/src/commands/testWaitForElementVisible.js b/tests/src/commands/testWaitForElementVisible.js index 97b74bd0f2..b9774fb8af 100644 --- a/tests/src/commands/testWaitForElementVisible.js +++ b/tests/src/commands/testWaitForElementVisible.js @@ -32,6 +32,15 @@ module.exports = { }); }, + testGlobalTimeoutSetting: function(test) { + var command = function() { + this.client.api.waitForElementVisible('foo'); + }; + + test.throws(command, Error, 'Timeout should be picked up globally if not provided'); + test.done(); + }, + tearDown : function(callback) { this.client = null; // clean up From 3379b3e8a79bf0c077d94a71b433ed0530cd4848 Mon Sep 17 00:00:00 2001 From: Jonathan Haggarty Date: Tue, 15 Apr 2014 12:03:12 +0100 Subject: [PATCH 3/9] add missing semicolon --- lib/selenium/commands/_waitForElement.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/selenium/commands/_waitForElement.js b/lib/selenium/commands/_waitForElement.js index e0921a8c4c..f42598bd21 100644 --- a/lib/selenium/commands/_waitForElement.js +++ b/lib/selenium/commands/_waitForElement.js @@ -42,7 +42,7 @@ util.inherits(WaitForElement, events.EventEmitter); * @returns {WaitForElement} */ WaitForElement.prototype.command = function(selector, milliseconds, callbackOrAbort) { - assertMilliseconds.call(this, milliseconds) + assertMilliseconds.call(this, milliseconds); this.startTimer = new Date().getTime(); From 2cd90c664b886c30c1828766b61c661786efefb9 Mon Sep 17 00:00:00 2001 From: Keith Bingman Date: Tue, 22 Apr 2014 14:41:14 -0700 Subject: [PATCH 4/9] added to array of DO_NOT_LOG_ERRORS --- lib/http/request.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/http/request.js b/lib/http/request.js index 60d18b5899..804ffd9a11 100644 --- a/lib/http/request.js +++ b/lib/http/request.js @@ -15,7 +15,8 @@ module.exports = (function() { }; var DO_NOT_LOG_ERRORS = [ - 'Unable to locate element' + 'Unable to locate element', + 'no such element' ]; function HttpRequest(options) { From 257e771b430b466c5fd766e63186b83f24d47bd1 Mon Sep 17 00:00:00 2001 From: andreiebuddy Date: Wed, 23 Apr 2014 11:50:34 +0200 Subject: [PATCH 5/9] Fixed an issue when the selenium server wasn't being stopped after a session create error --- lib/index.js | 10 +++++++--- lib/runner/run.js | 3 +++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/index.js b/lib/index.js index 723c7370cc..5600779862 100644 --- a/lib/index.js +++ b/lib/index.js @@ -154,7 +154,13 @@ Nightwatch.prototype.loadKeyCodes = function() { Nightwatch.prototype.start = function() { if (!this.sessionId) { - this.startSession().once('selenium:session_create', this.start); + this + .once('selenium:session_create', this.start) + .once('error', function(data, error) { + console.error(Logger.colors.red('Connection refused!'), + 'Is selenium server started?'); + }) + .startSession(); return this; } @@ -378,8 +384,6 @@ Nightwatch.prototype.startSession = function () { } }) .on('error', function(data, err) { - console.error(Logger.colors.red('Connection refused!'), - 'Is selenium server started?'); self.emit('error', data, err); }) .send(); diff --git a/lib/runner/run.js b/lib/runner/run.js index 09f4373bd7..9d8d54fda6 100644 --- a/lib/runner/run.js +++ b/lib/runner/run.js @@ -75,6 +75,9 @@ module.exports = new (function() { client.once('nightwatch:finished', function(results, errors) { onComplete(results, errors); }); + client.once('error', function(data, error) { + finishCallback({message: 'Unable to start a new session.'}, false); + }); clientFn.call(context, context.client); client.start(); }; From e64e03b5088577802fc7d14af2d9832493d8abea Mon Sep 17 00:00:00 2001 From: andreiebuddy Date: Wed, 23 Apr 2014 12:19:03 +0200 Subject: [PATCH 6/9] Fixed an issue where the set locateStrategy wasn't used correctly in the case of xpath --- lib/selenium/element-commands.js | 2 +- tests/mocks.json | 5 +++++ tests/src/commands/testClick.js | 21 +++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/selenium/element-commands.js b/lib/selenium/element-commands.js index ee608b56ab..5cd2c9dffc 100644 --- a/lib/selenium/element-commands.js +++ b/lib/selenium/element-commands.js @@ -311,7 +311,6 @@ module.exports = function(client) { elementCommands.submitForm = 'submit'; function addElementCommand(protocolAction, extraArgs) { - var defaultUsing = client.locateStrategy || 'css selector'; var self = this; extraArgs = extraArgs || 0; var expectedArgs = 3 + extraArgs; @@ -322,6 +321,7 @@ module.exports = function(client) { args.push(noopFn); } + var defaultUsing = client.locateStrategy || 'css selector'; if (expectedArgs - args.length === 1) { args.unshift(defaultUsing); } diff --git a/tests/mocks.json b/tests/mocks.json index 67f1f8d649..82a98b64f8 100644 --- a/tests/mocks.json +++ b/tests/mocks.json @@ -49,6 +49,11 @@ "postdata" : "{\"using\":\"css selector\",\"value\":\"#weblogin\"}", "response" : "{\"sessionId\":\"1352110219202\",\"status\":0,\"value\":{\"ELEMENT\":\"0\"},\"class\":\"org.openqa.selenium.remote.Response\",\"hCode\":604376696}" }, + { + "url" : "/wd/hub/session/1352110219202/element", + "postdata" : "{\"using\":\"xpath\",\"value\":\"//weblogin\"}", + "response" : "{\"sessionId\":\"1352110219202\",\"status\":0,\"value\":{\"ELEMENT\":\"0\"},\"class\":\"org.openqa.selenium.remote.Response\",\"hCode\":604376696}" + }, { "url" : "/wd/hub/session/1352110219202/buttondown", "postdata" : "{\"button\":0}", diff --git a/tests/src/commands/testClick.js b/tests/src/commands/testClick.js index 5814d54a09..1092bd8f87 100644 --- a/tests/src/commands/testClick.js +++ b/tests/src/commands/testClick.js @@ -23,7 +23,28 @@ module.exports = { test.equals(result.status, 0) test.done(); }); + }, + + testClickCommandWithXpath : function(test) { + var client = this.client.api; + + MockServer.addMock({ + "url" : "/wd/hub/session/1352110219202/element/0/click", + "response" : JSON.stringify({ + sessionId: "1352110219202", + status:0 + }) + }); + client + .useXpath() + .click('//weblogin', function callback(result) { + test.equals(result.status, 0) + }) + .click('css selector', '#weblogin', function callback(result) { + test.equals(result.status, 0) + test.done(); + }); }, tearDown : function(callback) { From 33974d85b618079f2f963f99bdb6a1c2fcc00702 Mon Sep 17 00:00:00 2001 From: andreiebuddy Date: Thu, 24 Apr 2014 12:19:26 +0200 Subject: [PATCH 7/9] added waitForConditionPollInterval also, changed the set milliseconds logic a bit and updated the api doc --- lib/selenium/commands/_waitForElement.js | 139 +++++++++++++----- .../src/commands/testWaitForElementVisible.js | 30 +++- 2 files changed, 132 insertions(+), 37 deletions(-) diff --git a/lib/selenium/commands/_waitForElement.js b/lib/selenium/commands/_waitForElement.js index f42598bd21..b2b388f3d4 100644 --- a/lib/selenium/commands/_waitForElement.js +++ b/lib/selenium/commands/_waitForElement.js @@ -18,57 +18,124 @@ function WaitForElement() { this.abortOnFailure = true; this.selector = null; this.locateStrategy = this.client.locateStrategy || 'css selector'; - this.rescheduleInterval = 500; //ms + this.rescheduleInterval = this.client.api.globals.waitForConditionPollInterval || 500; //ms this.protocol = require('../protocol.js')(this.client); } util.inherits(WaitForElement, events.EventEmitter); /*! - * The public command function which will be called by the test runner. Arguments can be passed in a variety of ways: + * The public command function which will be called by the test runner. Arguments can be passed in a variety of ways. * - * waitForElement('body') - * waitForElement('body', 500) - * waitForElement('body', 500, 'test message) + * The custom message always is last and the callback is always before the message or last if a message is not passed. + * + * The second argument is always the time in milliseconds. The third argument can be either of: + * - abortOnFailure: this can overwrite the default behaviour of aborting the test if the condition is not met within the specified time + * - rescheduleInterval: this can overwrite the default polling interval (currently 500ms) + * The above can be supplied also together, in which case the rescheduleInterval is specified before the abortOnFailure. + * + * Some of the multiple usage possibilities: + * --------------------------------------------------------------------------- + * - with no arguments; in this case a global default timeout is expected + * waitForElement('body'); + * + * - with a global default timeout and a callback + * waitForElement('body', function() {}); + * + * - with a global default timeout, a callback and a custom message + * waitForElement('body', function() {}, 'test message'); + * + * - with only the timeout + * waitForElement('body', 500); + * + * - with a timeout and a custom message + * waitForElement('body', 500, 'test message); + * + * - with a timeout and a callback * waitForElement('body', 500, function() { .. }); + * + * - with a timeout and a custom abortOnFailure * waitForElement('body', 500, true); + * + * - with a timeout, a custom abortOnFailure and a custom message * waitForElement('body', 500, true, 'test message'); + * + * - with a timeout, a custom abortOnFailure and a callback * waitForElement('body', 500, true, function() { .. }); + * + * - with a timeout, a custom abortOnFailure, a callback and a custom message * waitForElement('body', 500, true, function() { .. }, 'test message'); * + * - with a timeout, a custom reschedule interval and a callback + * waitForElement('body', 500, 100, function() { .. }); + * + * - with a timeout, a custom rescheduleInterval and a custom abortOnFailure + * waitForElement('body', 500, 100, false); + * + * * @param {string} selector - * @param {number} milliseconds - * @param {function|boolean|string} callbackOrAbort + * @param {number|function|string} milliseconds + * @param {function|boolean|string|number} callbackOrAbort * @returns {WaitForElement} */ WaitForElement.prototype.command = function(selector, milliseconds, callbackOrAbort) { - assertMilliseconds.call(this, milliseconds); - this.startTimer = new Date().getTime(); + this.ms = this.setMilliseconds(milliseconds); - if (typeof arguments[2] === 'boolean') { + if (typeof arguments[1] === 'function') { + //////////////////////////////////////////////// + // The command was called with an implied global timeout: + // + // waitForElement('body', function() {}); + // waitForElement('body', function() {}, 'custom message'); + //////////////////////////////////////////////// + this.cb = arguments[1]; + } else if (typeof arguments[2] === 'boolean') { + //////////////////////////////////////////////// + // The command was called with a custom abortOnFailure: + // + // waitForElement('body', 500, false); + //////////////////////////////////////////////// this.abortOnFailure = arguments[2]; + + // The optional callback is the 4th argument now this.cb = arguments[3] || function() {}; } else if (typeof arguments[2] === 'number') { + //////////////////////////////////////////////// + // The command was called with a custom rescheduleInterval: + // + // waitForElement('body', 500, 100); + //////////////////////////////////////////////// this.rescheduleInterval = arguments[2]; + if (typeof arguments[3] === 'boolean') { + //////////////////////////////////////////////// + // The command was called with a custom rescheduleInterval and custom abortOnFailure: + // + // waitForElement('body', 500, 100, false); + //////////////////////////////////////////////// this.abortOnFailure = arguments[3]; + + // The optional callback is the 5th argument now this.cb = arguments[4] || function() {}; } else { + // The optional callback is the 4th argument now this.cb = arguments[3] || function() {}; } } else { + // The optional callback is the 3th argument now this.cb = (typeof callbackOrAbort === 'function' && callbackOrAbort) || function() {}; } // support for a custom message this.message = null; - var lastArgument = arguments[arguments.length - 1]; - if (typeof lastArgument === 'string') { - this.message = lastArgument; + if (arguments.length > 1) { + var lastArgument = arguments[arguments.length - 1]; + if (typeof lastArgument === 'string') { + this.message = lastArgument; + } } - this.ms = milliseconds || this.client.api.globals.waitForConditionTimeout; this.selector = selector; this.checkElement(); return this; @@ -169,6 +236,27 @@ WaitForElement.prototype.formatMessage = function (defaultMsg, timeMs) { return format(this.message || defaultMsg, this.selector, timeMs || this.ms); }; +/** + * Set the time in milliseconds to wait for the condition, accepting a given value or a globally defined default + * + * @param {number} [timeoutMs] + * @throws Will throw an error if the global default is undefined or a non-number + * @returns {number} + */ +WaitForElement.prototype.setMilliseconds = function (timeoutMs) { + if (timeoutMs && typeof timeoutMs === 'number') { + return timeoutMs; + } + + var globalTimeout = this.client.api.globals.waitForConditionTimeout; + if (typeof globalTimeout !== 'number') { + throw new Error('waitForElement expects second parameter to have a global default ' + + '(waitForConditionTimeout) to be specified if not passed as the second parameter '); + } + + return globalTimeout; +}; + /** * A smaller version of util.format that doesn't support json and * if a placeholder is missing, it is omitted instead of appended @@ -180,7 +268,7 @@ function format(f) { var i = 1; var args = arguments; var len = args.length; - var str = String(f).replace(formatRegExp, function(x) { + return String(f).replace(formatRegExp, function(x) { if (x === '%%') { return '%'; } @@ -196,27 +284,6 @@ function format(f) { return x; } }); - - return str; } -/** - * @param {number} [timeoutMs] - * @throws Will throw an error if the argument is invalid - * @returns {number} - */ -function assertMilliseconds(timeoutMs) { - var globalTimeout = this.client.api.globals.waitForConditionTimeout; - - if (!timeoutMs && typeof globalTimeout !== 'number') { - throw new Error('waitForElement expects second parameter to have a global default ' + - '(waitForConditionTimeout) to be specified if not passed as the second parameter '); - } - else if (timeoutMs && typeof timeoutMs !== 'number') { - throw new Error('waitForElement expects second parameter to be number; ' + - typeof (timeoutMs) + ' given'); - } -} - - module.exports = WaitForElement; \ No newline at end of file diff --git a/tests/src/commands/testWaitForElementVisible.js b/tests/src/commands/testWaitForElementVisible.js index b9774fb8af..54afdab8a3 100644 --- a/tests/src/commands/testWaitForElementVisible.js +++ b/tests/src/commands/testWaitForElementVisible.js @@ -32,7 +32,35 @@ module.exports = { }); }, - testGlobalTimeoutSetting: function(test) { + 'test not visible with global timeout default': function(test) { + var assertion = []; + this.client.assertion = function(result, actual, expected, msg, abortObFailure) { + Array.prototype.unshift.apply(assertion, arguments); + }; + this.client.api.globals.waitForConditionTimeout = 55; + this.client.api.waitForElementVisible('#weblogin', function callback(result, instance) { + test.equal(assertion[0], false); + test.equal(assertion[3], 'Timed out while waiting for element <#weblogin> to be visible for 55 milliseconds.'); + test.equal(assertion[1], 'not visible'); + test.equal(assertion[4], true); + + test.done(); + }); + }, + + 'test not visible with global timeout default and custom message': function(test) { + var assertion = []; + this.client.assertion = function(result, actual, expected, msg, abortObFailure) { + Array.prototype.unshift.apply(assertion, arguments); + }; + this.client.api.globals.waitForConditionTimeout = 55; + this.client.api.waitForElementVisible('#weblogin', function callback(result, instance) { + test.equal(assertion[3], 'Test message <#weblogin> and a global 55 ms.'); + test.done(); + }, 'Test message <%s> and a global %s ms.'); + }, + + 'test not visible with no args and global timeout not set': function(test) { var command = function() { this.client.api.waitForElementVisible('foo'); }; From 5ae2a82fcf0d882d49532854e1dd32699568244a Mon Sep 17 00:00:00 2001 From: andreiebuddy Date: Thu, 24 Apr 2014 13:17:30 +0200 Subject: [PATCH 8/9] updated tests for waitForElementPresent --- tests/src/commands/testWaitForElementPresent.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/src/commands/testWaitForElementPresent.js b/tests/src/commands/testWaitForElementPresent.js index 0562691caf..7642120f61 100644 --- a/tests/src/commands/testWaitForElementPresent.js +++ b/tests/src/commands/testWaitForElementPresent.js @@ -16,24 +16,24 @@ module.exports = { }); }, - testSuccessWithCustomMessage : function(test) { + testFailureWithCustomMessage : function(test) { this.client.api.waitForElementPresent('.weblogin', 100, function callback(result, instance) { test.equal(instance.message, 'Element .weblogin found in 100 milliseconds'); - test.equal(result, false); + test.equal(result.value, false); test.done(); }, 'Element %s found in %d milliseconds'); }, testFailure : function(test) { this.client.api.waitForElementPresent('.weblogin', 600, function callback(result) { - test.equal(result, false); + test.equal(result.value, false); test.done(); }); }, testFailureNoAbort : function(test) { this.client.api.waitForElementPresent('.weblogin', 600, false, function callback(result) { - test.equal(result, false); + test.equal(result.value, false); test.done(); }); }, From 3e70c58af073aed1699ab552c95a5e9093058c01 Mon Sep 17 00:00:00 2001 From: andreiebuddy Date: Thu, 24 Apr 2014 16:29:48 +0200 Subject: [PATCH 9/9] added results and errors properties on the test object available from inside tearDown to satisfy #135 --- lib/runner/run.js | 27 ++++++++++++++++++++++----- tests/sampletests/mixed/sample.js | 5 +++++ tests/src/runner/testRunner.js | 2 +- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/lib/runner/run.js b/lib/runner/run.js index 9d8d54fda6..f7e10f5686 100644 --- a/lib/runner/run.js +++ b/lib/runner/run.js @@ -149,14 +149,11 @@ module.exports = new (function() { tearDown = function(context, clientFn, client, onTestComplete) { if (module.tearDown.length === 0) { startClient(context, clientFn, client, function(results, errors) { - module.tearDown(); - onTestComplete(results, errors); + callTearDown(module, results, errors, onTestComplete, false); }); } else if (module.tearDown.length >= 0) { startClient(context, clientFn, client, function(results, errors) { - module.tearDown(function() { - onTestComplete(results, errors); - }); + callTearDown(module, results, errors, onTestComplete, true); }); } }; @@ -169,6 +166,26 @@ module.exports = new (function() { setTimeout(next, 0); } + function callTearDown(module, results, errors, onTestComplete, hasCallback) { + module.results = results; + module.errors = errors; + + var tearDownFn = module.tearDown; + var args = []; + var callbackFn = function() { + onTestComplete(results, errors); + }; + if (hasCallback) { + args.push(callbackFn); + } + + tearDownFn.apply(module, args); + + if (!hasCallback) { + callbackFn(); + } + } + function printResults(testresults, modulekeys) { var elapsedTime = new Date().getTime() - globalStartTime; if (testresults.passed > 0 && testresults.errors === 0 && testresults.failed === 0) { diff --git a/tests/sampletests/mixed/sample.js b/tests/sampletests/mixed/sample.js index fdcb705032..3657f1badd 100644 --- a/tests/sampletests/mixed/sample.js +++ b/tests/sampletests/mixed/sample.js @@ -5,8 +5,13 @@ module.exports = { tearDown : function(callback) { var client = this.client; + var self = this; setTimeout(function() { client.globals.test.ok('tearDown callback called.'); + client.globals.test.deepEqual(self.results, { + passed: 0, failed: 0, errors: 0, skipped: 0, tests: [] + }); + client.globals.test.deepEqual(self.errors, []); callback(); }, 100); } diff --git a/tests/src/runner/testRunner.js b/tests/src/runner/testRunner.js index 5abf6082e2..eaa957e2e6 100644 --- a/tests/src/runner/testRunner.js +++ b/tests/src/runner/testRunner.js @@ -87,7 +87,7 @@ module.exports = { }, testRunMixed : function(test) { - test.expect(4); + test.expect(6); Runner.run([process.cwd() + '/sampletests/mixed'], { seleniumPort : 10195,