Repository: unexpectedjs/unexpected
Pull request #622 (2019-04-21 13:42:58)

Don't trigger the footgun protection when a promise is .then()ed but not fully awaited
Eg. via `expect.promise.any`

Fixes the flaky doc test: `example #2 (documentation/api/promise-any.md:39:1) should succeed`

https://github.com/unexpectedjs/unexpected/pull/621#issuecomment-485232318


Patch:
diff --git a/lib/createTopLevelExpect.js b/lib/createTopLevelExpect.js
index 8d3d74a4..90d81995 100644
--- a/lib/createTopLevelExpect.js
+++ b/lib/createTopLevelExpect.js
@@ -1469,13 +1469,13 @@ expectPrototype._expect = function expect(context, args) {
     if (utils.isPromise(result)) {
       result = wrapPromiseIfNecessary(result);
       if (result.isPending()) {
-        this.notifyPendingPromise(result);
         result = result.then(undefined, e => {
           if (e && e._isUnexpected && context.level === 0) {
             this.setErrorMessage(e);
           }
           throw e;
         });
+        this.notifyPendingPromise(result);
       }
     } else {
       result = makePromise.resolve(result);
diff --git a/lib/notifyPendingPromise.js b/lib/notifyPendingPromise.js
index 02ed0083..499061f9 100644
--- a/lib/notifyPendingPromise.js
+++ b/lib/notifyPendingPromise.js
@@ -18,7 +18,10 @@ if (typeof jasmine === 'object') {
 
 function isPendingOrHasUnhandledRejection(promise) {
   return (
-    promise.isPending() || (promise.isRejected() && promise.reason().uncaught)
+    !promise._fulfillmentHandler0 &&
+    !promise._rejectionHandler0 &&
+    !promise._receiver0 &&
+    (promise.isPending() || (promise.isRejected() && promise.reason().uncaught))
   );
 }
 


Test patch:
diff --git a/externaltests/avoidPromiseAnyFootgunFalsePositive.spec.js b/externaltests/avoidPromiseAnyFootgunFalsePositive.spec.js
new file mode 100644
index 00000000..2bf339bc
--- /dev/null
+++ b/externaltests/avoidPromiseAnyFootgunFalsePositive.spec.js
@@ -0,0 +1,26 @@
+var expect = require('../lib')
+  .clone()
+  .addAssertion('<any> to be a number after a (short|long) delay', function(
+    expect,
+    subject
+  ) {
+    return expect.promise(function(run) {
+      setTimeout(
+        run(function() {
+          expect(subject, 'to be a number');
+        }),
+        expect.alternations[0] === 'short' ? 0 : 10
+      );
+    });
+  });
+
+it('should succeed', () => {
+  return expect.promise.any({
+    foo: [expect('42', 'to be a number after a long delay')],
+    bar: expect(
+      [0, 1, 2],
+      'to have items satisfying',
+      expect.it('to be a number after a short delay')
+    )
+  });
+});
diff --git a/externaltests/avoidPromiseRaceFootgunFalsePositive.spec.js b/externaltests/avoidPromiseRaceFootgunFalsePositive.spec.js
new file mode 100644
index 00000000..c32d0748
--- /dev/null
+++ b/externaltests/avoidPromiseRaceFootgunFalsePositive.spec.js
@@ -0,0 +1,22 @@
+var expect = require('../lib')
+  .clone()
+  .addAssertion('<any> to be a number after a (short|long) delay', function(
+    expect,
+    subject
+  ) {
+    return expect.promise(function(run) {
+      setTimeout(
+        run(function() {
+          expect(subject, 'to be a number');
+        }),
+        expect.alternations[0] === 'short' ? 0 : 10
+      );
+    });
+  });
+
+it('should succeed', () => {
+  return expect.promise.race([
+    expect(42, 'to be a number after a long delay'),
+    expect(42, 'to be a number after a short delay')
+  ]);
+});
diff --git a/test/external.spec.js b/test/external.spec.js
index 291a9357..7756ca02 100644
--- a/test/external.spec.js
+++ b/test/external.spec.js
@@ -119,6 +119,24 @@ if (typeof process === 'object') {
         });
       });
 
+      it('should not fail when an unresolved promise was used in a expect.promise.any construct', () => {
+        return expect(
+          'avoidPromiseAnyFootgunFalsePositive',
+          'executed through mocha'
+        ).then(([err]) => {
+          expect(err, 'to be falsy');
+        });
+      });
+
+      it('should not fail when an unresolved promise was used in a expect.promise.race construct', () => {
+        return expect(
+          'avoidPromiseRaceFootgunFalsePositive',
+          'executed through mocha'
+        ).then(([err]) => {
+          expect(err, 'to be falsy');
+        });
+      });
+
       describe('with a test suite spanning multiple files', () => {
         it('should report that a promise was created, but not returned by the it block in the first test', () => {
           return expect(
@@ -357,6 +375,24 @@ if (typeof process === 'object') {
         });
       });
 
+      it('should not fail when an unresolved promise was used in a expect.promise.any construct', () => {
+        return expect(
+          'avoidPromiseAnyFootgunFalsePositive',
+          'executed through jest'
+        ).then(([err]) => {
+          expect(err, 'to be falsy');
+        });
+      });
+
+      it('should not fail when an unresolved promise was used in a expect.promise.race construct', () => {
+        return expect(
+          'avoidPromiseRaceFootgunFalsePositive',
+          'executed through jest'
+        ).then(([err]) => {
+          expect(err, 'to be falsy');
+        });
+      });
+
       describe('with a test suite spanning multiple files', () => {
         it('should report that a promise was created, but not returned by the it block in the first test', () => {
           return expect(