[ESnext] Implement Promise.prototype.finally

test/mjsunit/harmony/promise-prototype-finally.js

Index: test/mjsunit/harmony/promise-prototype-finally.js
diff --git a/test/mjsunit/harmony/promise-prototype-finally.js b/test/mjsunit/harmony/promise-prototype-finally.js
new file mode 100644
index 0000000000000000000000000000000000000000..e61e75b0f599d9a1a0d12ac610ae15a05e39b995
--- /dev/null
+++ b/test/mjsunit/harmony/promise-prototype-finally.js
@@ -0,0 +1,164 @@
+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --harmony-promise-finally --allow-natives-syntax
+
+var asyncAssertsExpected = 0;
+
+function assertAsyncRan() {
+  ++asyncAssertsExpected;
+}
+
+function assertAsync(b, s) {
+  if (b) {
+    print(s, "succeeded");
+  } else {
+    %AbortJS(s + " FAILED!");
+  }
+  --asyncAssertsExpected;
+}
+
+function assertEqualsAsync(b, s) {
+  if (b === s) {
+    print(b, "===", s, "succeeded");
+  } else {
+    %AbortJS(b + "===" + s + " FAILED!");
+  }
+  --asyncAssertsExpected;
+}
+
+function assertAsyncDone(iteration) {
+  var iteration = iteration || 0;
+  %EnqueueMicrotask(function() {
+    if (asyncAssertsExpected === 0)
+      assertAsync(true, "all");
+    else if (
+      iteration > 10 // Shouldn't take more.
+    )
+      assertAsync(false, "all... " + asyncAssertsExpected);
+    else
+      assertAsyncDone(iteration + 1);
+  });
+}
+
+(function() {
+  Promise.resolve(3)
+    .then(x => {
+      assertEqualsAsync(3, x);
+      return x;
+    })

[neis, 2017/02/15 12:40:40]

Is this .then call useful at all?

[gsathya, 2017/02/16 15:05:30]

Removed

[neis, 2017/02/17 10:22:54]

Still there.

[gsathya, 2017/02/17 11:26:16]

Can you comment on the latest patch set? I can't find this. There's one test
which does resolve/then/finally/then which I think is useful.

+    .finally()
+    .then(
+      x => {
+        assertEqualsAsync(3, x);
+      },
+      assertUnreachable

[neis, 2017/02/15 12:40:40]

assertUnreachable here is wrong since it would just reject the promise (even if
perhaps here it doesn't matter).

[gsathya, 2017/02/16 15:05:30]

assertUnreachable now aborts the test

+    );
+  assertAsyncRan();
+  assertAsyncRan();
+})();
+
+(function() {
+  Promise.reject(3)
+    .catch(e => {
+      assertEqualsAsync(3, e);
+      throw e;
+    })
+    .finally()
+    .then(assertUnreachable, reason => {
+      assertEqualsAsync(3, reason);
+    });
+  assertAsyncRan();
+  assertAsyncRan();
+})();
+
+(function() {
+  Promise.resolve(3)
+    .then(x => {
+      assertEqualsAsync(3, x);
+      return x;
+    })
+    .finally(function onFinally() {
+      assertAsync(arguments.length === 0);

[neis, 2017/02/15 12:40:39]

Equals?

[gsathya, 2017/02/16 15:05:30]

assertAsync is fine here since we check if true? Or do you want me to change all
these to be assertEqualAsync(0, arguments.length)?

[neis, 2017/02/17 10:22:54]

Yeah I don't see why you don't use assertEqualsAsync here (which you use almost
everywhere else).

[gsathya, 2017/02/17 11:26:15]

Done.

+      throw 1;
+    })
+    .then(assertUnreachable, function onRejected(reason) {
+      assertEqualsAsync(1, reason);
+    });
+  assertAsyncRan();
+  assertAsyncRan();
+  assertAsyncRan();
+})();
+
+(function() {
+  Promise.reject(3)
+    .finally(function onFinally() {
+      assertAsync(arguments.length === 0);
+      throw 1;
+    })
+    .then(assertUnreachable, function onRejected(reason) {
+      assertEqualsAsync(1, reason);
+    });
+  assertAsyncRan();
+  assertAsyncRan();
+})();
+
+(function() {
+  Promise.resolve(3)
+    .then(x => {
+      assertEqualsAsync(3, x);
+      return x;
+    })
+    .finally(function onFinally() {
+      assertAsync(arguments.length === 0);
+      return 4;
+    })
+    .then(
+      function onFulfilled(x) {
+        assertEqualsAsync(x, 3);
+      },
+      assertUnreachable
+    );
+  assertAsyncRan();
+  assertAsyncRan();
+  assertAsyncRan();
+})();
+
+(function() {
+  Promise.reject(3)
+    .catch(e => {
+      assertEqualsAsync(3, e);
+      throw e;
+    })
+    .finally(function onFinally() {
+      assertAsync(arguments.length === 0);
+      throw 4;
+    })
+    .then(assertUnreachable, function onRejected(e) {
+      assertEqualsAsync(4, e);
+    });
+  assertAsyncRan();
+  assertAsyncRan();
+  assertAsyncRan();
+})();
+
+(function() {
+  Promise.resolve(3)
+    .then(x => {
+      assertEqualsAsync(3, x);
+      return x;
+    })
+    .finally(function onFinally() {
+      assertAsync(arguments.length === 0);
+      return Promise.reject(4);
+    })
+    .then(assertUnreachable, e => {
+      assertEqualsAsync(4, e);
+    });
+  assertAsyncRan();
+  assertAsyncRan();
+  assertAsyncRan();
+})();
+
+assertAsyncDone();

[neis, 2017/02/15 12:40:39]

Please add some tests that check that .finally just calls .then (using a custom
.then).  One of these should pass an argument to .finally that is neither
undefined nor a function and make sure that it's being passed on to .then
unmodified.

Also add a test that checks where .finally is called on a rejected promise and
its handler returns a resolved promise.

Add some tests involving multiple .finally calls.

[gsathya, 2017/02/16 15:05:30]

Added a whole bunch of tests. PTAL.

[neis, 2017/02/17 10:22:54]

I'm still missing a test passing an argument to .finally that is neither
undefined nor a function and makes sure that it's being passed on to .then
unmodified.

[gsathya, 2017/02/17 11:26:15]

You mean, makes sure that it's not being passed on to .then, right? Done.