Encapsulate spec status

lib/jasmine-core/jasmine.js

@@ -781,6 +781,9 @@ getJasmineRequireObj().Spec = function(j$) {
     #throwOnExpectationFailure;
     #timer;
     #metadata;
+    // TODO: better naming. Don't make 'excluded' mean two things.
+    #dynamicallyExcluded;
+    #requireExpectations;
 
     constructor(attrs) {
       this.expectationFactory = attrs.expectationFactory;
@@ -829,11 +832,6 @@ getJasmineRequireObj().Spec = function(j$) {
           this.onLateError(expectationResult);
         } else {
           this.result.failedExpectations.push(expectationResult);
-
-          // TODO: refactor so that we don't need to override cached status
-          if (this.result.status) {
-            this.result.status = 'failed';
-          }
         }
 
         if (this.#throwOnExpectationFailure && !isError) {
@@ -862,18 +860,34 @@ getJasmineRequireObj().Spec = function(j$) {
     }
 
     executionFinished(excluded, failSpecWithNoExp) {
+      this.#dynamicallyExcluded = excluded;
+      this.#requireExpectations = failSpecWithNoExp;
+
       if (this.#autoCleanClosures) {
         this.queueableFn.fn = null;
       }
 
-      this.result.status = this.#status(excluded, failSpecWithNoExp);
       this.result.duration = this.#timer.elapsed();
 
-      if (this.result.status !== 'failed') {
+      if (this.status() !== 'failed') {
         this.result.debugLogs = null;
       }
     }
 
+    hadBeforeAllFailure() {
+      this.addExpectationResult(
+        false,
+        {
+          passed: false,
+          message:
+            'Not run because a beforeAll function failed. The ' +
+            'beforeAll failure will be reported on the suite that ' +
+            'caused it.'
+        },
+        true
+      );
+    }
+
     reset() {
       this.result = {
         id: this.id,
@@ -891,6 +905,8 @@ getJasmineRequireObj().Spec = function(j$) {
       };
       this.markedPending = this.markedExcluding;
       this.reportedDone = false;
+      this.#dynamicallyExcluded = false;
+      this.#requireExpectations = false;
     }
 
     startedEvent() {
@@ -935,14 +951,14 @@ getJasmineRequireObj().Spec = function(j$) {
        * @since 6.0.0
        */
       const event = {
-        ...this.#commonEventFields()
+        ...this.#commonEventFields(),
+        status: this.status()
       };
       const toCopy = [
         'failedExpectations',
         'passedExpectations',
         'deprecationWarnings',
         'pendingReason',
-        'status',
         'duration',
         'properties',
         'debugLogs'
@@ -1005,13 +1021,13 @@ getJasmineRequireObj().Spec = function(j$) {
 
     // TODO: ensure that all access to result goes through .getResult()
     // so that the status is correct.
+    // Step 1: fix things so getResult() always returns correct status
     getResult() {
-      this.result.status = this.#status();
       return this.result;
     }
 
-    #status(excluded, failSpecWithNoExpectations) {
+    status() {
-      if (excluded === true) {
+      if (this.#dynamicallyExcluded) {
         return 'excluded';
       }
 
@@ -1021,7 +1037,7 @@ getJasmineRequireObj().Spec = function(j$) {
 
       if (
         this.result.failedExpectations.length > 0 ||
-        (failSpecWithNoExpectations &&
+        (this.#requireExpectations &&
           this.result.failedExpectations.length +
             this.result.passedExpectations.length ===
             0)
@@ -1424,7 +1440,7 @@ getJasmineRequireObj().Env = function(j$) {
             expectationResult.globalErrorType = 'lateError';
           }
 
-          r.result.failedExpectations.push(expectationResult);
+          r.addExpectationResult(false, expectationResult);
           return;
         }
       }
@@ -11620,7 +11636,7 @@ getJasmineRequireObj().TreeRunner = function(j$) {
           );
         },
         onComplete: () => {
-          if (spec.result.status === 'failed') {
+          if (spec.status() === 'failed') {
             specOverallDone(new j$.private.StopExecutionError('spec failed'));
           } else {
             specOverallDone();
@@ -11652,7 +11668,7 @@ getJasmineRequireObj().TreeRunner = function(j$) {
       const complete = {
         fn(done) {
           spec.executionFinished(excluded, config.failSpecWithNoExpectations);
-          resultCallback(spec.result, done);
+          resultCallback(spec.doneEvent(), done);
         },
         type: 'specCleanup'
       };
@@ -11789,7 +11805,7 @@ getJasmineRequireObj().TreeRunner = function(j$) {
       this.#runableResources.clearForRunable(spec.id);
       this.#currentRunableTracker.setCurrentSpec(null);
 
-      if (spec.result.status === 'failed') {
+      if (spec.status() === 'failed') {
         this.#hasFailures = true;
       }
 
@@ -11815,19 +11831,7 @@ getJasmineRequireObj().TreeRunner = function(j$) {
         } else {
           /* a spec */
           await this.#reportDispatcher.specStarted(child.startedEvent());
-
+          child.hadBeforeAllFailure();
-          child.addExpectationResult(
-            false,
-            {
-              passed: false,
-              message:
-                'Not run because a beforeAll function failed. The ' +
-                'beforeAll failure will be reported on the suite that ' +
-                'caused it.'
-            },
-            true
-          );
-          child.result.status = 'failed';
           await this.#reportSpecDone(child);
         }
       }

spec/core/SpecSpec.js

@@ -149,11 +149,16 @@ describe('Spec', function() {
   });
 
   describe('status', function() {
-    it('is "passed" by default', function() {
+    it('returns "passed" by default', function() {
       const spec = new privateUnderTest.Spec({
         queueableFn: { fn: () => {} }
       });
-      expect(spec.getResult().status).toBe('passed');
+      expect(spec.status())
+        .withContext('status()')
+        .toBe('passed');
+      expect(spec.doneEvent().status)
+        .withContext('doneEvent().status')
+        .toBe('passed');
     });
 
     it('is "passed" if all expectations passed', function() {
@@ -163,7 +168,12 @@ describe('Spec', function() {
 
       spec.addExpectationResult(true, {});
 
-      expect(spec.getResult().status).toBe('passed');
+      expect(spec.status())
+        .withContext('status()')
+        .toBe('passed');
+      expect(spec.doneEvent().status)
+        .withContext('doneEvent().status')
+        .toBe('passed');
     });
 
     it('is "failed" if any expectation failed', function() {
@@ -174,7 +184,12 @@ describe('Spec', function() {
       spec.addExpectationResult(true, {});
       spec.addExpectationResult(false, {});
 
-      expect(spec.getResult().status).toBe('failed');
+      expect(spec.status())
+        .withContext('status()')
+        .toBe('failed');
+      expect(spec.doneEvent().status)
+        .withContext('doneEvent().status')
+        .toBe('failed');
     });
 
     it('is "pending" if created without a function body', function() {
@@ -186,7 +201,65 @@ describe('Spec', function() {
           resultCallback: resultCallback
         });
 
-      expect(spec.getResult().status).toBe('pending');
+      expect(spec.status())
+        .withContext('status()')
+        .toBe('pending');
+      expect(spec.doneEvent().status)
+        .withContext('doneEvent().status')
+        .toBe('pending');
+    });
+
+    describe('after a call to executionFinished()', function() {
+      describe('with excluded true', function() {
+        it("is 'excluded'", function() {
+          const spec = new privateUnderTest.Spec({
+            queueableFn: { fn: () => {} }
+          });
+
+          spec.executionFinished(true, false);
+
+          expect(spec.status())
+            .withContext('status()')
+            .toBe('excluded');
+          expect(spec.doneEvent().status)
+            .withContext('doneEvent().status')
+            .toBe('excluded');
+        });
+      });
+
+      describe('with failSpecWithNoExp true', function() {
+        it("is 'failed' if there were no expectations", function() {
+          const spec = new privateUnderTest.Spec({
+            queueableFn: { fn: () => {} }
+          });
+
+          spec.executionFinished(false, true);
+
+          expect(spec.status())
+            .withContext('status()')
+            .toBe('failed');
+          expect(spec.doneEvent().status)
+            .withContext('doneEvent().status')
+            .toBe('failed');
+        });
+      });
+    });
+
+    describe('after a call to hadBeforeAllFailure()', function() {
+      it("is 'failed'", function() {
+        const spec = new privateUnderTest.Spec({
+          queueableFn: { fn: () => {} }
+        });
+
+        spec.hadBeforeAllFailure();
+
+        expect(spec.status())
+          .withContext('status()')
+          .toBe('failed');
+        expect(spec.doneEvent().status)
+          .withContext('doneEvent().status')
+          .toBe('failed');
+      });
     });
   });
 

spec/core/TreeRunnerSpec.js

@@ -116,8 +116,8 @@ describe('TreeRunner', function() {
       expect(specRunQueueArgs.queueableFns[1]).toEqual(queueableFn);
       queueableFn.fn();
 
-      expect(spec.getResult().status).toEqual('pending');
+      expect(spec.doneEvent().status).toEqual('pending');
-      expect(spec.getResult().pendingReason).toEqual('');
+      expect(spec.doneEvent().pendingReason).toEqual('');
     });
 
     it('marks specs pending at runtime with a message', function() {
@@ -137,8 +137,8 @@ describe('TreeRunner', function() {
       expect(specRunQueueArgs.queueableFns[1]).toEqual(queueableFn);
       queueableFn.fn();
 
-      expect(spec.getResult().status).toEqual('pending');
+      expect(spec.doneEvent().status).toEqual('pending');
-      expect(spec.getResult().pendingReason).toEqual('some reason');
+      expect(spec.doneEvent().pendingReason).toEqual('some reason');
     });
 
     it('passes failSpecWithNoExp to Spec#executionFinished', async function() {

src/core/Env.js

@@ -240,7 +240,7 @@ getJasmineRequireObj().Env = function(j$) {
             expectationResult.globalErrorType = 'lateError';
           }
 
-          r.result.failedExpectations.push(expectationResult);
+          r.addExpectationResult(false, expectationResult);
           return;
         }
       }

src/core/Spec.js

@@ -4,6 +4,9 @@ getJasmineRequireObj().Spec = function(j$) {
     #throwOnExpectationFailure;
     #timer;
     #metadata;
+    // TODO: better naming. Don't make 'excluded' mean two things.
+    #dynamicallyExcluded;
+    #requireExpectations;
 
     constructor(attrs) {
       this.expectationFactory = attrs.expectationFactory;
@@ -52,11 +55,6 @@ getJasmineRequireObj().Spec = function(j$) {
           this.onLateError(expectationResult);
         } else {
           this.result.failedExpectations.push(expectationResult);
-
-          // TODO: refactor so that we don't need to override cached status
-          if (this.result.status) {
-            this.result.status = 'failed';
-          }
         }
 
         if (this.#throwOnExpectationFailure && !isError) {
@@ -85,18 +83,34 @@ getJasmineRequireObj().Spec = function(j$) {
     }
 
     executionFinished(excluded, failSpecWithNoExp) {
+      this.#dynamicallyExcluded = excluded;
+      this.#requireExpectations = failSpecWithNoExp;
+
       if (this.#autoCleanClosures) {
         this.queueableFn.fn = null;
       }
 
-      this.result.status = this.#status(excluded, failSpecWithNoExp);
       this.result.duration = this.#timer.elapsed();
 
-      if (this.result.status !== 'failed') {
+      if (this.status() !== 'failed') {
         this.result.debugLogs = null;
       }
     }
 
+    hadBeforeAllFailure() {
+      this.addExpectationResult(
+        false,
+        {
+          passed: false,
+          message:
+            'Not run because a beforeAll function failed. The ' +
+            'beforeAll failure will be reported on the suite that ' +
+            'caused it.'
+        },
+        true
+      );
+    }
+
     reset() {
       this.result = {
         id: this.id,
@@ -114,6 +128,8 @@ getJasmineRequireObj().Spec = function(j$) {
       };
       this.markedPending = this.markedExcluding;
       this.reportedDone = false;
+      this.#dynamicallyExcluded = false;
+      this.#requireExpectations = false;
     }
 
     startedEvent() {
@@ -158,14 +174,14 @@ getJasmineRequireObj().Spec = function(j$) {
        * @since 6.0.0
        */
       const event = {
-        ...this.#commonEventFields()
+        ...this.#commonEventFields(),
+        status: this.status()
       };
       const toCopy = [
         'failedExpectations',
         'passedExpectations',
         'deprecationWarnings',
         'pendingReason',
-        'status',
         'duration',
         'properties',
         'debugLogs'
@@ -228,13 +244,13 @@ getJasmineRequireObj().Spec = function(j$) {
 
     // TODO: ensure that all access to result goes through .getResult()
     // so that the status is correct.
+    // Step 1: fix things so getResult() always returns correct status
     getResult() {
-      this.result.status = this.#status();
       return this.result;
     }
 
-    #status(excluded, failSpecWithNoExpectations) {
+    status() {
-      if (excluded === true) {
+      if (this.#dynamicallyExcluded) {
         return 'excluded';
       }
 
@@ -244,7 +260,7 @@ getJasmineRequireObj().Spec = function(j$) {
 
       if (
         this.result.failedExpectations.length > 0 ||
-        (failSpecWithNoExpectations &&
+        (this.#requireExpectations &&
           this.result.failedExpectations.length +
             this.result.passedExpectations.length ===
             0)

src/core/TreeRunner.js

@@ -77,7 +77,7 @@ getJasmineRequireObj().TreeRunner = function(j$) {
           );
         },
         onComplete: () => {
-          if (spec.result.status === 'failed') {
+          if (spec.status() === 'failed') {
             specOverallDone(new j$.private.StopExecutionError('spec failed'));
           } else {
             specOverallDone();
@@ -109,7 +109,7 @@ getJasmineRequireObj().TreeRunner = function(j$) {
       const complete = {
         fn(done) {
           spec.executionFinished(excluded, config.failSpecWithNoExpectations);
-          resultCallback(spec.result, done);
+          resultCallback(spec.doneEvent(), done);
         },
         type: 'specCleanup'
       };
@@ -246,7 +246,7 @@ getJasmineRequireObj().TreeRunner = function(j$) {
       this.#runableResources.clearForRunable(spec.id);
       this.#currentRunableTracker.setCurrentSpec(null);
 
-      if (spec.result.status === 'failed') {
+      if (spec.status() === 'failed') {
         this.#hasFailures = true;
       }
 
@@ -272,19 +272,7 @@ getJasmineRequireObj().TreeRunner = function(j$) {
         } else {
           /* a spec */
           await this.#reportDispatcher.specStarted(child.startedEvent());
-
+          child.hadBeforeAllFailure();
-          child.addExpectationResult(
-            false,
-            {
-              passed: false,
-              message:
-                'Not run because a beforeAll function failed. The ' +
-                'beforeAll failure will be reported on the suite that ' +
-                'caused it.'
-            },
-            true
-          );
-          child.result.status = 'failed';
           await this.#reportSpecDone(child);
         }
       }