From 4059ab7ba6638b2611361ead9318ec68c288bf14 Mon Sep 17 00:00:00 2001
From: Steve Gravrock <sdg@panix.com>
Date: Sat, 19 Feb 2022 12:22:55 -0800
Subject: [PATCH] Don't report a deprecation when a promise is resolved to
 something

beforeEach(() => somePromiseReturningFn()) is likely a common idiom
and we don't want to treat it as an error.

* Fixes #1958
---
 lib/jasmine-core/jasmine.js  | 12 +++++-
 spec/core/QueueRunnerSpec.js | 78 ++++++++++++++++++++++++++++++++++++
 src/core/QueueRunner.js      | 12 +++++-
 3 files changed, 100 insertions(+), 2 deletions(-)

diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js
index 4846279a..09abf136 100644
--- a/lib/jasmine-core/jasmine.js
+++ b/lib/jasmine-core/jasmine.js
@@ -8656,7 +8656,7 @@ getJasmineRequireObj().QueueRunner = function(j$) {
         maybeThenable = queueableFn.fn.call(self.userContext);
 
         if (maybeThenable && j$.isFunction_(maybeThenable.then)) {
-          maybeThenable.then(next, onPromiseRejection);
+          maybeThenable.then(wrapInPromiseResolutionHandler(next), onPromiseRejection);
           completedSynchronously = false;
           return { completedSynchronously: false };
         }
@@ -8747,6 +8747,16 @@ getJasmineRequireObj().QueueRunner = function(j$) {
     }
   };
 
+  function wrapInPromiseResolutionHandler(fn) {
+    return function(maybeArg) {
+      if (j$.isError_(maybeArg)) {
+        fn(maybeArg);
+      } else {
+        fn();
+      }
+    };
+  }
+
   return QueueRunner;
 };
 
diff --git a/spec/core/QueueRunnerSpec.js b/spec/core/QueueRunnerSpec.js
index 302e535e..a7dbadf5 100644
--- a/spec/core/QueueRunnerSpec.js
+++ b/spec/core/QueueRunnerSpec.js
@@ -254,6 +254,84 @@ describe('QueueRunner', function() {
           expect(queueableFn2.fn).toHaveBeenCalled();
         });
       });
+
+      describe('as a result of a promise', function() {
+        describe('and the argument is an Error', function() {
+          // Since promise support was added, Jasmine has failed specs that
+          // return a promise that resolves to an error. That's probably not
+          // the desired behavior but it's also not something we should change
+          // except on a major release and with a deprecation warning in
+          // advance.
+          it('explicitly fails and moves to the next function', function(done) {
+            jasmine.getEnv().requirePromises();
+            var err = new Error('foo'),
+              queueableFn1 = {
+                fn: function() {
+                  return Promise.resolve(err);
+                }
+              },
+              queueableFn2 = { fn: jasmine.createSpy('fn2') },
+              failFn = jasmine.createSpy('fail'),
+              queueRunner = new jasmineUnderTest.QueueRunner({
+                queueableFns: [queueableFn1, queueableFn2],
+                fail: failFn,
+                onComplete: function() {
+                  expect(failFn).toHaveBeenCalledWith(err);
+                  expect(queueableFn2.fn).toHaveBeenCalled();
+                  done();
+                }
+              });
+
+            queueRunner.execute();
+          });
+
+          it('does not log a deprecation', function(done) {
+            jasmine.getEnv().requirePromises();
+            var err = new Error('foo'),
+              queueableFn1 = {
+                fn: function() {
+                  return Promise.resolve(err);
+                }
+              },
+              deprecated = jasmine.createSpy('deprecated'),
+              queueRunner = new jasmineUnderTest.QueueRunner({
+                queueableFns: [queueableFn1],
+                deprecated: deprecated,
+                onComplete: function() {
+                  expect(deprecated).not.toHaveBeenCalled();
+                  done();
+                }
+              });
+
+            queueRunner.execute();
+          });
+        });
+
+        describe('and the argument is not an Error', function() {
+          it('does not log a deprecation or report a failure', function(done) {
+            jasmine.getEnv().requirePromises();
+            var queueableFn1 = {
+                fn: function() {
+                  return Promise.resolve('not an error');
+                }
+              },
+              failFn = jasmine.createSpy('fail'),
+              deprecated = jasmine.createSpy('deprecated'),
+              queueRunner = new jasmineUnderTest.QueueRunner({
+                queueableFns: [queueableFn1],
+                deprecated: deprecated,
+                fail: failFn,
+                onComplete: function() {
+                  expect(deprecated).not.toHaveBeenCalled();
+                  expect(failFn).not.toHaveBeenCalled();
+                  done();
+                }
+              });
+
+            queueRunner.execute();
+          });
+        });
+      });
     });
 
     it('does not cause an explicit fail if execution is being stopped', function() {
diff --git a/src/core/QueueRunner.js b/src/core/QueueRunner.js
index 7b6ebdd2..486ddb5b 100644
--- a/src/core/QueueRunner.js
+++ b/src/core/QueueRunner.js
@@ -196,7 +196,7 @@ getJasmineRequireObj().QueueRunner = function(j$) {
         maybeThenable = queueableFn.fn.call(self.userContext);
 
         if (maybeThenable && j$.isFunction_(maybeThenable.then)) {
-          maybeThenable.then(next, onPromiseRejection);
+          maybeThenable.then(wrapInPromiseResolutionHandler(next), onPromiseRejection);
           completedSynchronously = false;
           return { completedSynchronously: false };
         }
@@ -287,5 +287,15 @@ getJasmineRequireObj().QueueRunner = function(j$) {
     }
   };
 
+  function wrapInPromiseResolutionHandler(fn) {
+    return function(maybeArg) {
+      if (j$.isError_(maybeArg)) {
+        fn(maybeArg);
+      } else {
+        fn();
+      }
+    };
+  }
+
   return QueueRunner;
 };