From 0738ba64620040d9415c05ebb5b2b4feb82410f3 Mon Sep 17 00:00:00 2001 From: Steve Gravrock Date: Mon, 22 Sep 2025 17:21:10 -0700 Subject: [PATCH] Omit irrelevant properties from suiteStarted --- lib/jasmine-core/jasmine.js | 98 ++++++++++++++++++++++++-------- spec/core/RunnerSpec.js | 21 +++++-- spec/core/SuiteSpec.js | 30 ++++++++++ spec/core/integration/EnvSpec.js | 12 ++-- src/core/Runner.js | 2 +- src/core/Spec.js | 4 +- src/core/Suite.js | 75 ++++++++++++++++++++---- src/core/TreeRunner.js | 20 +++---- src/core/reporterEvents.js | 4 +- 9 files changed, 206 insertions(+), 60 deletions(-) diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index f5eb6b5c..b3199f52 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -916,7 +916,7 @@ getJasmineRequireObj().Spec = function(j$) { * same call stack height as the originals. This property may be removed in * a future version unless there is enough user interest in keeping it. * See {@link https://github.com/jasmine/jasmine/issues/2065}. - * @since 6.0.0 + * @since 2.0.0 */ return this.#commonEventFields(); } @@ -942,7 +942,7 @@ getJasmineRequireObj().Spec = function(j$) { * @property {number} duration - The time in ms used by the spec execution, including any before/afterEach. * @property {Object} properties - User-supplied properties, if any, that were set using {@link Env#setSpecProperty} * @property {DebugLogEntry[]|null} debugLogs - Messages, if any, that were logged using {@link jasmine.debugLog} during a failing spec. - * @since 6.0.0 + * @since 2.0.0 */ const event = { ...this.#commonEventFields(), @@ -8766,7 +8766,7 @@ getJasmineRequireObj().reporterEvents = function(j$) { * `suiteStarted` is invoked when a `describe` starts to run * @function * @name Reporter#suiteStarted - * @param {SuiteResult} result Information about the individual {@link describe} being run + * @param {SuiteStartedEvent} result Information about the individual {@link describe} being run * @param {Function} [done] Used to specify to Jasmine that this callback is asynchronous and Jasmine should wait until it has been called before moving on. * @returns {} Optionally return a Promise instead of using `done` to cause Jasmine to wait for completion. * @see async @@ -8778,7 +8778,7 @@ getJasmineRequireObj().reporterEvents = function(j$) { * While jasmine doesn't require any specific functions, not defining a `suiteDone` will make it impossible for a reporter to know when a suite has failures in an `afterAll`. * @function * @name Reporter#suiteDone - * @param {SuiteResult} result + * @param {SuiteDoneEvent} result * @param {Function} [done] Used to specify to Jasmine that this callback is asynchronous and Jasmine should wait until it has been called before moving on. * @returns {} Optionally return a Promise instead of using `done` to cause Jasmine to wait for completion. * @see async @@ -9617,7 +9617,7 @@ getJasmineRequireObj().Runner = function(j$) { this.#currentRunableTracker.popSuite(); let overallStatus, incompleteReason, incompleteCode; - if (hasFailures || this.#topSuite.result.failedExpectations.length > 0) { + if (hasFailures || this.#topSuite.hasOwnFailedExpectations()) { overallStatus = 'failed'; } else if (this.#getFocusedRunables().length > 0) { overallStatus = 'incomplete'; @@ -10711,8 +10711,45 @@ getJasmineRequireObj().Suite = function(j$) { } reset() { + this.result = { + id: this.id, + description: this.description, + fullName: this.getFullName(), + parentSuiteId: this.#reportedParentSuiteId, + filename: this.filename, + failedExpectations: [], + deprecationWarnings: [], + duration: null, + properties: null + }; + this.markedPending = this.markedExcluding; + this.children.forEach(function(child) { + child.reset(); + }); + this.reportedDone = false; + } + + startedEvent() { /** - * @typedef SuiteResult + * @typedef SuiteStartedEvent + * @property {String} id - The unique id of this suite. + * @property {String} description - The description text passed to the {@link describe} that made this suite. + * @property {String} fullName - The full description including all ancestors of this suite. + * @property {String|null} parentSuiteId - The ID of the suite containing this suite, or null if this is not in another describe(). + * @property {String} filename - Deprecated. The name of the file the suite was defined in. + * Note: The value may be incorrect if zone.js is installed or + * `describe`/`fdescribe`/`xdescribe` have been replaced with versions that + * don't maintain the same call stack height as the originals. This property + * may be removed in a future version unless there is enough user interest + * in keeping it. See {@link https://github.com/jasmine/jasmine/issues/2065}. + * @since 6.0.0 + */ + return this.#commonEventFields(); + } + + doneEvent() { + /** + * @typedef SuiteDoneEvent * @property {String} id - The unique id of this suite. * @property {String} description - The description text passed to the {@link describe} that made this suite. * @property {String} fullName - The full description including all ancestors of this suite. @@ -10730,22 +10767,32 @@ getJasmineRequireObj().Suite = function(j$) { * @property {Object} properties - User-supplied properties, if any, that were set using {@link Env#setSuiteProperty} * @since 2.0.0 */ - this.result = { + const event = { + ...this.#commonEventFields(), + status: this.#status() + }; + const toCopy = [ + 'failedExpectations', + 'deprecationWarnings', + 'duration', + 'properties' + ]; + + for (const k of toCopy) { + event[k] = this.result[k]; + } + + return event; + } + + #commonEventFields() { + return { id: this.id, description: this.description, fullName: this.getFullName(), parentSuiteId: this.#reportedParentSuiteId, - filename: this.filename, - failedExpectations: [], - deprecationWarnings: [], - duration: null, - properties: null + filename: this.filename }; - this.markedPending = this.markedExcluding; - this.children.forEach(function(child) { - child.reset(); - }); - this.reportedDone = false; } removeChildren() { @@ -10768,6 +10815,10 @@ getJasmineRequireObj().Suite = function(j$) { } } + hasOwnFailedExpectations() { + return this.result.failedExpectations.length > 0; + } + getResult() { this.result.status = this.#status(); return this.result; @@ -11713,7 +11764,7 @@ getJasmineRequireObj().TreeRunner = function(j$) { this.#runQueue({ onComplete: maybeError => { - this.#suiteSegmentComplete(suite, suite.getResult(), () => { + this.#suiteSegmentComplete(suite, () => { done(maybeError); }); }, @@ -11750,11 +11801,13 @@ getJasmineRequireObj().TreeRunner = function(j$) { #suiteSegmentStart(suite, next) { this.#currentRunableTracker.pushSuite(suite); this.#runableResources.initForRunable(suite.id, suite.parentSuite.id); - this.#reportDispatcher.suiteStarted(suite.result).then(next); + this.#reportDispatcher.suiteStarted(suite.startedEvent()).then(next); suite.startTimer(); } - #suiteSegmentComplete(suite, result, next) { + #suiteSegmentComplete(suite, next) { + suite.endTimer(); + const result = suite.doneEvent(); const isTopSuite = suite === this.#executionTree.topSuite; if (!isTopSuite) { @@ -11773,7 +11826,6 @@ getJasmineRequireObj().TreeRunner = function(j$) { if (result.status === 'failed') { this.#hasFailures = true; } - suite.endTimer(); } const finish = isTopSuite @@ -11811,14 +11863,14 @@ getJasmineRequireObj().TreeRunner = function(j$) { async #reportChildrenOfBeforeAllFailure(suite) { for (const child of suite.children) { if (child instanceof j$.private.Suite) { - await this.#reportDispatcher.suiteStarted(child.result); + await this.#reportDispatcher.suiteStarted(child.startedEvent()); await this.#reportChildrenOfBeforeAllFailure(child); // Marking the suite passed is consistent with how suites that // contain failed specs but no suite-level failures are reported. child.result.status = 'passed'; - await this.#reportDispatcher.suiteDone(child.result); + await this.#reportDispatcher.suiteDone(child.doneEvent()); } else { /* a spec */ await this.#reportDispatcher.specStarted(child.startedEvent()); diff --git a/spec/core/RunnerSpec.js b/spec/core/RunnerSpec.js index 83f324b0..433f117d 100644 --- a/spec/core/RunnerSpec.js +++ b/spec/core/RunnerSpec.js @@ -37,11 +37,16 @@ describe('Runner', function() { this.sharedUserContext = function() { return attrs.userContext || {}; }; + // TODO remove this.result = { id: this.id, failedExpectations: [] }; - this.getResult = jasmine.createSpy('getResult'); + this.startedEvent = jasmine.createSpy('startedEvent'); + this.doneEvent = jasmine.createSpy('doneEvent'); + this.hasOwnFailedExpectations = jasmine.createSpy( + 'hasOwnFailedExpectations' + ); this.beforeAllFns = attrs.beforeAllFns || []; this.afterAllFns = attrs.afterAllFns || []; this.cleanupBeforeAfter = function() {}; @@ -182,10 +187,13 @@ describe('Runner', function() { SkipPolicy: privateUnderTest.SkipAfterBeforeAllErrorPolicy }); + suite.startedEvent.and.returnValue('suite started event'); runQueue.calls.mostRecent().args[0].queueableFns[0].fn('foo'); - expect(reportDispatcher.suiteStarted).toHaveBeenCalledWith(suite.result); + expect(reportDispatcher.suiteStarted).toHaveBeenCalledWith( + 'suite started event' + ); - suite.getResult.and.returnValue({ my: 'result' }); + suite.doneEvent.and.returnValue({ my: 'result' }); runQueue.calls.mostRecent().args[0].onComplete(); expect(reportDispatcher.suiteDone).toHaveBeenCalledWith({ my: 'result' }); @@ -233,12 +241,15 @@ describe('Runner', function() { queueableFns = runQueue.calls.mostRecent().args[0].queueableFns; expect(queueableFns.length).toBe(2); + parent.startedEvent.and.returnValue('parent suite started event'); queueableFns[0].fn(); - expect(reportDispatcher.suiteStarted).toHaveBeenCalledWith(parent.result); + expect(reportDispatcher.suiteStarted).toHaveBeenCalledWith( + 'parent suite started event' + ); verifyAndFinishSpec(spec, queueableFns[1], true); - parent.getResult.and.returnValue(parent.result); + parent.doneEvent.and.returnValue(parent.result); runQueue.calls.argsFor(1)[0].onComplete(); expect(reportDispatcher.suiteDone).toHaveBeenCalledWith(parent.result); await expectAsync(promise).toBePending(); diff --git a/spec/core/SuiteSpec.js b/spec/core/SuiteSpec.js index 4a32fab1..117d5ca4 100644 --- a/spec/core/SuiteSpec.js +++ b/spec/core/SuiteSpec.js @@ -425,4 +425,34 @@ describe('Suite', function() { }).toThrowError("Value can't be cloned"); }); }); + + describe('#startedEvent', function() { + it('includes only properties that are known before execution', function() { + const topSuite = new privateUnderTest.Suite({}); + const parentSuite = new privateUnderTest.Suite({ + id: 'suite1', + parentSuite: topSuite, + description: 'a parent suite' + }); + const suite = new privateUnderTest.Suite({ + id: 'suite2', + parentSuite, + reportedParentSuiteId: parentSuite.id, + description: 'a suite', + filename: 'somefile.js', + getPath() { + return ['a parent suite', 'a spec']; + }, + queueableFn: { fn: () => {} } + }); + + expect(suite.startedEvent()).toEqual({ + id: 'suite2', + parentSuiteId: 'suite1', + description: 'a suite', + fullName: 'a parent suite a suite', + filename: 'somefile.js' + }); + }); + }); }); diff --git a/spec/core/integration/EnvSpec.js b/spec/core/integration/EnvSpec.js index 55e8fd67..f37c0bba 100644 --- a/spec/core/integration/EnvSpec.js +++ b/spec/core/integration/EnvSpec.js @@ -1765,10 +1765,12 @@ describe('Env integration', function() { const baseSuiteEvent = { id: jasmine.any(String), - filename: jasmine.any(String), + filename: jasmine.any(String) + }; + const baseSuiteDoneEvent = { + ...baseSuiteEvent, failedExpectations: [], deprecationWarnings: [], - duration: null, properties: null }; @@ -1779,7 +1781,7 @@ describe('Env integration', function() { parentSuiteId: null }); expect(reporter.suiteDone.calls.argsFor(2)[0]).toEqual({ - ...baseSuiteEvent, + ...baseSuiteDoneEvent, description: 'A Suite', fullName: 'A Suite', status: 'passed', @@ -1794,7 +1796,7 @@ describe('Env integration', function() { parentSuiteId: suiteFullNameToId['A Suite'] }); expect(reporter.suiteDone.calls.argsFor(0)[0]).toEqual({ - ...baseSuiteEvent, + ...baseSuiteDoneEvent, description: 'with a nested suite', status: 'passed', fullName: 'A Suite with a nested suite', @@ -1809,7 +1811,7 @@ describe('Env integration', function() { parentSuiteId: suiteFullNameToId['A Suite'] }); expect(reporter.suiteDone.calls.argsFor(1)[0]).toEqual({ - ...baseSuiteEvent, + ...baseSuiteDoneEvent, description: 'with only non-executable specs', status: 'passed', fullName: 'A Suite with only non-executable specs', diff --git a/src/core/Runner.js b/src/core/Runner.js index 9c71d7e5..5f5d7381 100644 --- a/src/core/Runner.js +++ b/src/core/Runner.js @@ -123,7 +123,7 @@ getJasmineRequireObj().Runner = function(j$) { this.#currentRunableTracker.popSuite(); let overallStatus, incompleteReason, incompleteCode; - if (hasFailures || this.#topSuite.result.failedExpectations.length > 0) { + if (hasFailures || this.#topSuite.hasOwnFailedExpectations()) { overallStatus = 'failed'; } else if (this.#getFocusedRunables().length > 0) { overallStatus = 'incomplete'; diff --git a/src/core/Spec.js b/src/core/Spec.js index 02fc561c..2a284aab 100644 --- a/src/core/Spec.js +++ b/src/core/Spec.js @@ -139,7 +139,7 @@ getJasmineRequireObj().Spec = function(j$) { * same call stack height as the originals. This property may be removed in * a future version unless there is enough user interest in keeping it. * See {@link https://github.com/jasmine/jasmine/issues/2065}. - * @since 6.0.0 + * @since 2.0.0 */ return this.#commonEventFields(); } @@ -165,7 +165,7 @@ getJasmineRequireObj().Spec = function(j$) { * @property {number} duration - The time in ms used by the spec execution, including any before/afterEach. * @property {Object} properties - User-supplied properties, if any, that were set using {@link Env#setSpecProperty} * @property {DebugLogEntry[]|null} debugLogs - Messages, if any, that were logged using {@link jasmine.debugLog} during a failing spec. - * @since 6.0.0 + * @since 2.0.0 */ const event = { ...this.#commonEventFields(), diff --git a/src/core/Suite.js b/src/core/Suite.js index c7b63e56..dd5cff31 100644 --- a/src/core/Suite.js +++ b/src/core/Suite.js @@ -100,8 +100,45 @@ getJasmineRequireObj().Suite = function(j$) { } reset() { + this.result = { + id: this.id, + description: this.description, + fullName: this.getFullName(), + parentSuiteId: this.#reportedParentSuiteId, + filename: this.filename, + failedExpectations: [], + deprecationWarnings: [], + duration: null, + properties: null + }; + this.markedPending = this.markedExcluding; + this.children.forEach(function(child) { + child.reset(); + }); + this.reportedDone = false; + } + + startedEvent() { /** - * @typedef SuiteResult + * @typedef SuiteStartedEvent + * @property {String} id - The unique id of this suite. + * @property {String} description - The description text passed to the {@link describe} that made this suite. + * @property {String} fullName - The full description including all ancestors of this suite. + * @property {String|null} parentSuiteId - The ID of the suite containing this suite, or null if this is not in another describe(). + * @property {String} filename - Deprecated. The name of the file the suite was defined in. + * Note: The value may be incorrect if zone.js is installed or + * `describe`/`fdescribe`/`xdescribe` have been replaced with versions that + * don't maintain the same call stack height as the originals. This property + * may be removed in a future version unless there is enough user interest + * in keeping it. See {@link https://github.com/jasmine/jasmine/issues/2065}. + * @since 6.0.0 + */ + return this.#commonEventFields(); + } + + doneEvent() { + /** + * @typedef SuiteDoneEvent * @property {String} id - The unique id of this suite. * @property {String} description - The description text passed to the {@link describe} that made this suite. * @property {String} fullName - The full description including all ancestors of this suite. @@ -119,22 +156,32 @@ getJasmineRequireObj().Suite = function(j$) { * @property {Object} properties - User-supplied properties, if any, that were set using {@link Env#setSuiteProperty} * @since 2.0.0 */ - this.result = { + const event = { + ...this.#commonEventFields(), + status: this.#status() + }; + const toCopy = [ + 'failedExpectations', + 'deprecationWarnings', + 'duration', + 'properties' + ]; + + for (const k of toCopy) { + event[k] = this.result[k]; + } + + return event; + } + + #commonEventFields() { + return { id: this.id, description: this.description, fullName: this.getFullName(), parentSuiteId: this.#reportedParentSuiteId, - filename: this.filename, - failedExpectations: [], - deprecationWarnings: [], - duration: null, - properties: null + filename: this.filename }; - this.markedPending = this.markedExcluding; - this.children.forEach(function(child) { - child.reset(); - }); - this.reportedDone = false; } removeChildren() { @@ -157,6 +204,10 @@ getJasmineRequireObj().Suite = function(j$) { } } + hasOwnFailedExpectations() { + return this.result.failedExpectations.length > 0; + } + getResult() { this.result.status = this.#status(); return this.result; diff --git a/src/core/TreeRunner.js b/src/core/TreeRunner.js index 628b1500..fd30a6ef 100644 --- a/src/core/TreeRunner.js +++ b/src/core/TreeRunner.js @@ -163,7 +163,7 @@ getJasmineRequireObj().TreeRunner = function(j$) { this.#runQueue({ onComplete: maybeError => { - this.#suiteSegmentComplete(suite, suite.getResult(), () => { + this.#suiteSegmentComplete(suite, () => { done(maybeError); }); }, @@ -200,11 +200,12 @@ getJasmineRequireObj().TreeRunner = function(j$) { #suiteSegmentStart(suite, next) { this.#currentRunableTracker.pushSuite(suite); this.#runableResources.initForRunable(suite.id, suite.parentSuite.id); - this.#reportDispatcher.suiteStarted(suite.result).then(next); + this.#reportDispatcher.suiteStarted(suite.startedEvent()).then(next); suite.startTimer(); } - #suiteSegmentComplete(suite, result, next) { + #suiteSegmentComplete(suite, next) { + suite.endTimer(); const isTopSuite = suite === this.#executionTree.topSuite; if (!isTopSuite) { @@ -220,15 +221,14 @@ getJasmineRequireObj().TreeRunner = function(j$) { this.#runableResources.clearForRunable(suite.id); this.#currentRunableTracker.popSuite(); - if (result.status === 'failed') { + if (suite.doneEvent().status === 'failed') { this.#hasFailures = true; } - suite.endTimer(); } const finish = isTopSuite ? next - : () => this.#reportSuiteDone(suite, result, next); + : () => this.#reportSuiteDone(suite, next); if (suite.hadBeforeAllFailure) { this.#reportChildrenOfBeforeAllFailure(suite).then(finish); @@ -237,9 +237,9 @@ getJasmineRequireObj().TreeRunner = function(j$) { } } - #reportSuiteDone(suite, result, next) { + #reportSuiteDone(suite, next) { suite.reportedDone = true; - this.#reportDispatcher.suiteDone(result).then(next); + this.#reportDispatcher.suiteDone(suite.doneEvent()).then(next); } async #specComplete(spec) { @@ -261,14 +261,14 @@ getJasmineRequireObj().TreeRunner = function(j$) { async #reportChildrenOfBeforeAllFailure(suite) { for (const child of suite.children) { if (child instanceof j$.private.Suite) { - await this.#reportDispatcher.suiteStarted(child.result); + await this.#reportDispatcher.suiteStarted(child.startedEvent()); await this.#reportChildrenOfBeforeAllFailure(child); // Marking the suite passed is consistent with how suites that // contain failed specs but no suite-level failures are reported. child.result.status = 'passed'; - await this.#reportDispatcher.suiteDone(child.result); + await this.#reportDispatcher.suiteDone(child.doneEvent()); } else { /* a spec */ await this.#reportDispatcher.specStarted(child.startedEvent()); diff --git a/src/core/reporterEvents.js b/src/core/reporterEvents.js index 4f563d94..e215c324 100644 --- a/src/core/reporterEvents.js +++ b/src/core/reporterEvents.js @@ -50,7 +50,7 @@ getJasmineRequireObj().reporterEvents = function(j$) { * `suiteStarted` is invoked when a `describe` starts to run * @function * @name Reporter#suiteStarted - * @param {SuiteResult} result Information about the individual {@link describe} being run + * @param {SuiteStartedEvent} result Information about the individual {@link describe} being run * @param {Function} [done] Used to specify to Jasmine that this callback is asynchronous and Jasmine should wait until it has been called before moving on. * @returns {} Optionally return a Promise instead of using `done` to cause Jasmine to wait for completion. * @see async @@ -62,7 +62,7 @@ getJasmineRequireObj().reporterEvents = function(j$) { * While jasmine doesn't require any specific functions, not defining a `suiteDone` will make it impossible for a reporter to know when a suite has failures in an `afterAll`. * @function * @name Reporter#suiteDone - * @param {SuiteResult} result + * @param {SuiteDoneEvent} result * @param {Function} [done] Used to specify to Jasmine that this callback is asynchronous and Jasmine should wait until it has been called before moving on. * @returns {} Optionally return a Promise instead of using `done` to cause Jasmine to wait for completion. * @see async