diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index f4d804e5..7e91e3ec 100644 --- a/lib/jasmine-core/jasmine.js +++ b/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) { - if (excluded === true) { + status() { + 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.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'; + child.hadBeforeAllFailure(); await this.#reportSpecDone(child); } } diff --git a/spec/core/SpecSpec.js b/spec/core/SpecSpec.js index 12370d6d..a15cf9d9 100644 --- a/spec/core/SpecSpec.js +++ b/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'); + }); }); }); diff --git a/spec/core/TreeRunnerSpec.js b/spec/core/TreeRunnerSpec.js index d9771a2c..6241b9e2 100644 --- a/spec/core/TreeRunnerSpec.js +++ b/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.getResult().pendingReason).toEqual(''); + expect(spec.doneEvent().status).toEqual('pending'); + 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.getResult().pendingReason).toEqual('some reason'); + expect(spec.doneEvent().status).toEqual('pending'); + expect(spec.doneEvent().pendingReason).toEqual('some reason'); }); it('passes failSpecWithNoExp to Spec#executionFinished', async function() { diff --git a/src/core/Env.js b/src/core/Env.js index ba69fc66..3b189bf2 100644 --- a/src/core/Env.js +++ b/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; } } diff --git a/src/core/Spec.js b/src/core/Spec.js index 1a990cc9..618cff3e 100644 --- a/src/core/Spec.js +++ b/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) { - if (excluded === true) { + status() { + 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) diff --git a/src/core/TreeRunner.js b/src/core/TreeRunner.js index ece555d9..628b1500 100644 --- a/src/core/TreeRunner.js +++ b/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.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'; + child.hadBeforeAllFailure(); await this.#reportSpecDone(child); } }