Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promise.prototype.finally: add tests #1156

Merged
merged 3 commits into from
Aug 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,9 @@
console/TestCases
github-deploy-key
github-deploy-key.pub
node_modules

# Only apps should have lockfiles
package-lock.json
yarn.lock
npm-shrinkwrap.json
1 change: 1 addition & 0 deletions .npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package-lock=false
4 changes: 4 additions & 0 deletions features.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
#
# https://github.com/tc39/process-document

# Promise.prototype.finally
# https://github.com/tc39/proposal-promise-finally
Promise.prototype.finally
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thanks!


# Async Iteration and Generators
# https://github.com/tc39/proposal-async-iteration
async-iteration
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright (C) 2017 Jordan Harband. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
author: Jordan Harband
description: Promise.prototype.finally invokes `then` method
esid: sec-promise.prototype.finally
features: [Promise.prototype.finally]
---*/

var target = new Promise(function () {});
var returnValue = {};
var callCount = 0;
var thisValue = null;
var argCount = null;
var firstArg = null;
var secondArg = null;

target.then = function(a, b) {
callCount += 1;

thisValue = this;
argCount = arguments.length;
firstArg = a;
secondArg = b;

return returnValue;
};

var originalFinallyHandler = function () {};

var result = Promise.prototype.finally.call(target, originalFinallyHandler, 2, 3);

assert.sameValue(callCount, 1, 'Invokes `then` method exactly once');
assert.sameValue(
thisValue,
target,
'Invokes `then` method with the instance as the `this` value'
);
assert.sameValue(argCount, 2, 'Invokes `then` method with exactly two single arguments');
assert.sameValue(
typeof firstArg,
'function',
'Invokes `then` method with a function as the first argument'
);
assert.notSameValue(firstArg, originalFinallyHandler, 'Invokes `then` method with a different fulfillment handler');
assert.sameValue(
typeof secondArg,
'function',
'Invokes `then` method with a function as the second argument'
);
assert.notSameValue(secondArg, originalFinallyHandler, 'Invokes `then` method with a different fulfillment handler');

assert.sameValue(result, returnValue, 'Returns the result of the invocation of `then`');
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright (C) 2017 Jordan Harband. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
author: Jordan Harband
description: Promise.prototype.finally invokes `then` method
esid: sec-promise.prototype.finally
features: [Promise.prototype.finally]
---*/

var target = new Promise(function () {});
var returnValue = {};
var callCount = 0;
var thisValue = null;
var argCount = null;
var firstArg = null;
var secondArg = null;
var result = null;

target.then = function(a, b) {
callCount += 1;

thisValue = this;
argCount = arguments.length;
firstArg = a;
secondArg = b;

return returnValue;
};

result = Promise.prototype.finally.call(target, 1, 2, 3);

assert.sameValue(callCount, 1, 'Invokes `then` method exactly once');
assert.sameValue(
thisValue,
target,
'Invokes `then` method with the instance as the `this` value'
);
assert.sameValue(argCount, 2, 'Invokes `then` method with exactly two single arguments');
assert.sameValue(
firstArg,
1,
'Invokes `then` method with the provided non-callable first argument'
);
assert.sameValue(
secondArg,
1,
'Invokes `then` method with the provided non-callable first argument'
);
assert.sameValue(result, returnValue, 'Returns the result of the invocation of `then`');
20 changes: 20 additions & 0 deletions test/built-ins/Promise/prototype/finally/is-a-function.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (C) 2017 Jordan Harband. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
author: Jordan Harband
description: Promise.prototype.finally is a function
esid: sec-promise.prototype.finally
features: [Promise.prototype.finally]
---*/

assert.sameValue(
Promise.prototype.finally instanceof Function,
true,
'Expected Promise.prototype.finally to be instanceof Function'
);

assert.sameValue(
typeof Promise.prototype.finally,
'function',
'Expected Promise.prototype.finally to be a function'
);
16 changes: 16 additions & 0 deletions test/built-ins/Promise/prototype/finally/is-a-method.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright (C) 2017 Jordan Harband. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
author: Jordan Harband
description: finally is a method on a Promise
esid: sec-promise.prototype.finally
features: [Promise.prototype.finally]
---*/

var p = Promise.resolve(3);

assert.sameValue(
p.finally,
Promise.prototype.finally,
'Expected the `finally` method on a Promise to be `Promise.prototype.finally`'
);
28 changes: 28 additions & 0 deletions test/built-ins/Promise/prototype/finally/length.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (C) 2017 Jordan Harband. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
author: Jordan Harband
description: Promise.prototype.finally `length` property
esid: sec-promise.prototype.finally
info: >
ES6 Section 17:
Every built-in Function object, including constructors, has a length
property whose value is an integer. Unless otherwise specified, this value
is equal to the largest number of named arguments shown in the subclause
headings for the function description, including optional parameters.

[...]

Unless otherwise specified, the length property of a built-in Function
object has the attributes { [[Writable]]: false, [[Enumerable]]: false,
[[Configurable]]: true }.
includes: [propertyHelper.js]
features: [Promise.prototype.finally]
---*/

verifyProperty(Promise.prototype.finally, "length", {
value: 1,
enumerable: false,
configurable: true,
writable: false,
});
28 changes: 28 additions & 0 deletions test/built-ins/Promise/prototype/finally/name.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (C) 2017 Jordan Harband. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
author: Jordan Harband
description: Promise.prototype.finally `name` property
esid: sec-promise.prototype.finally
info: >
ES Section 17:

Every built-in Function object, including constructors, that is not
identified as an anonymous function has a name property whose value is a
String. Unless otherwise specified, this value is the name that is given to
the function in this specification.

[...]

Unless otherwise specified, the name property of a built-in Function
object, if it exists, has the attributes { [[Writable]]: false,
[[Enumerable]]: false, [[Configurable]]: true }.
includes: [propertyHelper.js]
features: [Promise.prototype.finally]
---*/

assert.sameValue(Promise.prototype.finally.name, 'finally');

verifyNotEnumerable(Promise.prototype.finally, 'name');
verifyNotWritable(Promise.prototype.finally, 'name');
verifyConfigurable(Promise.prototype.finally, 'name');
19 changes: 19 additions & 0 deletions test/built-ins/Promise/prototype/finally/prop-desc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright (C) 2017 Jordan Harband. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
author: Jordan Harband
description: Promise.prototype.finally property descriptor
esid: sec-promise.prototype.finally
info: >
Every other data property described in clauses 18 through 26 and in Annex
B.2 has the attributes { [[Writable]]: true, [[Enumerable]]: false,
[[Configurable]]: true } unless otherwise specified.
includes: [propertyHelper.js]
features: [Promise.prototype.finally]
---*/

assert.sameValue(typeof Promise.prototype.finally, 'function');

verifyNotEnumerable(Promise.prototype, 'finally');
verifyWritable(Promise.prototype, 'finally');
verifyConfigurable(Promise.prototype, 'finally');
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright (C) 2017 Jordan Harband. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
author: Jordan Harband
description: finally observably calls .then
esid: sec-promise.prototype.finally
features: [Promise.prototype.finally]
flags: [async]
---*/

var initialThenCount = 0;
var noReason = {};
var no = Promise.reject(noReason);
no.then = function () {
initialThenCount += 1;
return Promise.prototype.then.apply(this, arguments);
};

var onFinallyThenCount = 0;
var yesValue = {};
var yes = Promise.resolve(yesValue);
yes.then = function () {
onFinallyThenCount += 1;
return Promise.prototype.then.apply(this, arguments);
};

var finallyCalled = false;
var catchCalled = false;

no.catch(function (e) {
assert.sameValue(e, noReason);
throw e;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add cases with thow e, within fulfillment and rejection handlers i.e.:

Promise.resolve(1)
    .then(function () { throw new Error('');})
    .finally(/* Check if finally is reached */);

Promise.reject(1)
    .then(function () {}, function () { throw new Error('');})
    .finally(/* Check if finally is reached */)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that we need test that we raise TypeError when we use finally with new i.e.

const p = new Promise((resolve)=>{});

assert.throws(TypeError, function() {
  const f = new p.finally();
}, 'no constructor');

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm - I can add that test, but p.finally() returns a Promise, so I have no idea why it'd ever work with new, and .catch() lacks the same test. I'm pretty sure we don't test every returned object value to ensure it doesn't work with new - would checking that typeof wasn't "function" suffice?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh, I see. You're right.
So it will be enough to check if finally returns Promise. Not sure that I can find such test, or I missed something.
If there is no such test we can use Symbol.toStringTag:

const p = Promise.resolve(1);
assert.sameValue(p.finally()[Symbol.toStringTag], 'Promise');

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd think chaining a .then or .catch on the end would be sufficient, and I think some of the tests already do that.

}).finally(function () {
finallyCalled = true;
return yes;
}).catch(function (e) {
catchCalled = true;
assert.sameValue(e, noReason);
}).then(function () {
assert.sameValue(finallyCalled, true, 'initial finally was called');
assert.sameValue(initialThenCount, 1, 'initial finally invokes .then once');

assert.sameValue(catchCalled, true, 'catch was called');
assert.sameValue(onFinallyThenCount, 1, 'onFinally return promise has .then invoked once');
$DONE();
}).catch($ERROR);
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (C) 2017 Jordan Harband. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
author: Jordan Harband
description: finally on a rejected promise can not convert to a fulfillment
esid: sec-promise.prototype.finally
features: [Promise.prototype.finally]
flags: [async]
---*/

var original = {};
var replacement = {};

var p = Promise.reject(original);

p.finally(function () {
assert.sameValue(arguments.length, 0, 'onFinally receives zero args');
return replacement;
}).then(function () {
$ERROR('promise is rejected pre-finally; onFulfill should not be called');
}).catch(function (reason) {
assert.sameValue(reason, original, 'onFinally can not override the rejection value by returning');
}).then($DONE).catch($ERROR);
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright (C) 2017 Jordan Harband. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
author: Jordan Harband
description: finally on a rejected promise can override the rejection reason
esid: sec-promise.prototype.finally
features: [Promise.prototype.finally]
flags: [async]
---*/

var original = {};
var thrown = {};

var p = Promise.reject(original);

p.finally(function () {
assert.sameValue(arguments.length, 0, 'onFinally receives zero args');
throw thrown;
}).then(function () {
$ERROR('promise is rejected; onFulfill should not be called');
}).catch(function (reason) {
assert.sameValue(reason, thrown, 'onFinally can override the rejection reason by throwing');
}).then($DONE).catch($ERROR);
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (C) 2017 Jordan Harband. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.
/*---
author: Jordan Harband
description: finally on a fulfilled promise can not override the resolution value
esid: sec-promise.prototype.finally
features: [Promise.prototype.finally]
flags: [async]
---*/

var obj = {};

var p = Promise.resolve(obj);

p.finally(function () {
assert.sameValue(arguments.length, 0, 'onFinally receives zero args');
return {};
}).then(function (x) {
assert.sameValue(x, obj, 'onFinally can not override the resolution value');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the onFinally and the onFulfilled callbacks are skipped, the tests would still pass right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I see other tests also that don't verify if the onFulfilled/onRejected/onFinally has actually run.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that's true. i can make a followup PR that adds counters to ensure they're called; that's a good idea

}).then($DONE).catch($ERROR);
Loading