Skip to content

Commit

Permalink
[js] Change Deferred#cancel() to silently no-op if the deferred has a…
Browse files Browse the repository at this point in the history
…lready been resolved.

Fixes issue 6686
  • Loading branch information
jleyba committed Dec 9, 2013
1 parent 41441f8 commit e12da96
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 14 deletions.
5 changes: 5 additions & 0 deletions javascript/node/selenium-webdriver/CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## v2.38.1

* FIXED: 6686: Changed `webdriver.promise.Deferred#cancel()` to silently no-op
if the deferred has already been resolved.

## v2.38.0

* When a promise is rejected, always annotate the stacktrace with the parent
Expand Down
2 changes: 1 addition & 1 deletion javascript/node/selenium-webdriver/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "selenium-webdriver",
"version": "2.38.0",
"version": "2.38.1",
"description": "The official WebDriver JavaScript bindings from the Selenium project",
"keywords": [
"automation",
Expand Down
10 changes: 5 additions & 5 deletions javascript/webdriver/promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ webdriver.promise.Deferred = function(opt_canceller, opt_flow) {
*/
function removeAll() {
if (!isPending()) {
throw new Error('This Deferred has already been resolved.');
throw new Error('This Deferred has already been resolved. (1)');
}
listeners = [];
}
Expand All @@ -288,7 +288,7 @@ webdriver.promise.Deferred = function(opt_canceller, opt_flow) {
*/
function notifyAll(newState, newValue) {
if (!isPending()) {
throw new Error('This Deferred has already been resolved.');
throw new Error('This Deferred has already been resolved. (2)');
}

state = newState;
Expand Down Expand Up @@ -431,13 +431,13 @@ webdriver.promise.Deferred = function(opt_canceller, opt_flow) {
}

/**
* Cancels the computation of this promise's value and flags the promise as a
* rejected value.
* Attempts to cancel the computation of this instance's value. This attempt
* will silently fail if this instance has already resolved.
* @param {*=} opt_reason The reason for cancelling this promise.
*/
function cancel(opt_reason) {
if (!isPending()) {
throw Error('This Deferred has already been resolved.');
return;
}

if (opt_canceller) {
Expand Down
38 changes: 31 additions & 7 deletions javascript/webdriver/test/promise_test.html
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@
throwStubError = webdriver.test.testutil.throwStubError,
STUB_ERROR = webdriver.test.testutil.STUB_ERROR;

var app, clock, uncaughtException;
var app, clock, uncaughtExceptions;

function setUp() {
clock = webdriver.test.testutil.createMockClock();
uncaughtException = callbackHelper();
uncaughtExceptions = [];

app = webdriver.promise.controlFlow();
app.on(webdriver.promise.ControlFlow.EventType.UNCAUGHT_EXCEPTION,
uncaughtException);
goog.bind(uncaughtExceptions.push, uncaughtExceptions));

// The application timer defaults to a protected copies of the native
// timing functions. Reset it to use |window|, which has been modified
Expand All @@ -58,7 +58,8 @@
webdriver.test.testutil.consumeTimeouts();
clock.dispose();
app.reset();
uncaughtException.assertNotCalled();
assertArrayEquals(
'Did not expect any uncaught exceptions', [], uncaughtExceptions);
}


Expand Down Expand Up @@ -1132,9 +1133,32 @@
}


function testCancel_cannotCancelADeferredThatHasBeenResolved() {
var d = webdriver.promise.fulfilled(123);
assertThrows(d.cancel);
function testCancel_cancelIsANoopOnceAPromiseHasBeenFulfilled() {
var p = webdriver.promise.fulfilled(123);
p.cancel();

var pair = callbackPair(goog.partial(assertEquals, 123));
p.then(pair.callback, pair.errback);
pair.assertCallback();
}


function testCancel_cancelIsANoopOnceAPromiseHasBeenRejected() {
var p = webdriver.promise.rejected(STUB_ERROR);
p.cancel();

var pair = callbackPair(null, assertIsStubError);
p.then(pair.callback, pair.errback);
pair.assertErrback();
}


function testCancel_noopCancelTriggeredOnCallbackOfResolvedPromise() {
var d = webdriver.promise.defer();
var p = d.then();

d.fulfill();
p.cancel(); // This should not throw.
}


Expand Down
20 changes: 19 additions & 1 deletion javascript/webdriver/test/webdriver_test.html
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@

function expectedError(code, message) {
return function(e) {
assertEquals('Wrong error code', code, e.code);
assertEquals('Wrong error message', message, e.message);
assertEquals('Wrong error code', code, e.code);
};
}

Expand Down Expand Up @@ -1532,6 +1532,24 @@
}


function testFindElement_elementNotFoundInACallback() {
var testHelper = TestHelper.
expectingFailure(
expectedError(ECode.NO_SUCH_ELEMENT, 'Unable to find element')).
expect(CName.FIND_ELEMENT, {'using':'id', 'value':'foo'}).
andReturnError(
ECode.NO_SUCH_ELEMENT, {'message':'Unable to find element'}).
replayAll();

var driver = testHelper.createDriver();
webdriver.promise.fulfilled().then(function() {
var element = driver.findElement(By.id('foo'));
return element.click(); // Should not execute.
});
testHelper.execute();
}


function testFindElement_elementFound() {
var testHelper = TestHelper.expectingSuccess().
expect(CName.FIND_ELEMENT, {'using':'id', 'value':'foo'}).
Expand Down

0 comments on commit e12da96

Please sign in to comment.