From 4201fd848f95b527f0a81069eba92409d3595edf Mon Sep 17 00:00:00 2001 From: Steve Gravrock Date: Sat, 18 Oct 2025 17:22:48 -0700 Subject: [PATCH] Require spec/suite property keys to be strings, not just anything that's cloneable and serializable This matches the jsdoc. --- lib/jasmine-core/jasmine.js | 10 ++++++++-- spec/core/SpecSpec.js | 18 +++--------------- spec/core/SuiteSpec.js | 16 +++++++++++++--- src/core/Spec.js | 5 ++++- src/core/Suite.js | 5 ++++- 5 files changed, 32 insertions(+), 22 deletions(-) diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index 6decfa50..a287f50f 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -849,8 +849,11 @@ getJasmineRequireObj().Spec = function(j$) { // Key and value will eventually be cloned during reporting. The error // thrown at that point if they aren't cloneable isn't very helpful. // Throw a better one now. - j$.private.util.assertReporterCloneable(key, 'Key'); + if (!j$.private.isString(key)) { + throw new Error('Key must be a string'); + } j$.private.util.assertReporterCloneable(value, 'Value'); + this.#executionState.properties = this.#executionState.properties || {}; this.#executionState.properties[key] = value; } @@ -10851,8 +10854,11 @@ getJasmineRequireObj().Suite = function(j$) { // Key and value will eventually be cloned during reporting. The error // thrown at that point if they aren't cloneable isn't very helpful. // Throw a better one now. - j$.private.util.assertReporterCloneable(key, 'Key'); + if (!j$.private.isString(key)) { + throw new Error('Key must be a string'); + } j$.private.util.assertReporterCloneable(value, 'Value'); + this.#result.properties = this.#result.properties || {}; this.#result.properties[key] = value; } diff --git a/spec/core/SpecSpec.js b/spec/core/SpecSpec.js index 8969acf0..48b2ecef 100644 --- a/spec/core/SpecSpec.js +++ b/spec/core/SpecSpec.js @@ -103,26 +103,14 @@ describe('Spec', function() { }); }); - it('throws if the key is not structured-cloneable', function() { + it('throws if the key is not a string', function() { const spec = new privateUnderTest.Spec({ queueableFn: { fn: () => {} } }); expect(function() { - spec.setSpecProperty(new Promise(() => {}), ''); - }).toThrowError("Key can't be cloned"); - }); - - it('throws if the key is not JSON-serializable', function() { - const spec = new privateUnderTest.Spec({ - queueableFn: { fn: () => {} } - }); - - expect(function() { - const k = {}; - k.self = k; - spec.setSpecProperty(k, ''); - }).toThrowError("Key can't be cloned"); + spec.setSpecProperty({}, ''); + }).toThrowError('Key must be a string'); }); it('throws if the value is not structured-cloneable', function() { diff --git a/spec/core/SuiteSpec.js b/spec/core/SuiteSpec.js index 8c8065cd..dc771bde 100644 --- a/spec/core/SuiteSpec.js +++ b/spec/core/SuiteSpec.js @@ -409,12 +409,12 @@ describe('Suite', function() { }); describe('#setSuiteProperty', function() { - it('throws if the key is not structured-cloneable', function() { + it('throws if the key is not a string', function() { const suite = new privateUnderTest.Suite({}); expect(function() { - suite.setSuiteProperty(new Promise(() => {}), ''); - }).toThrowError("Key can't be cloned"); + suite.setSuiteProperty({}, ''); + }).toThrowError('Key must be a string'); }); it('throws if the value is not structured-cloneable', function() { @@ -424,6 +424,16 @@ describe('Suite', function() { suite.setSuiteProperty('k', new Promise(() => {})); }).toThrowError("Value can't be cloned"); }); + + it('throws if the value is not JSON-serializable', function() { + const suite = new privateUnderTest.Suite({}); + + expect(function() { + const v = {}; + v.self = v; + suite.setSuiteProperty('k', v); + }).toThrowError("Value can't be cloned"); + }); }); describe('#startedEvent', function() { diff --git a/src/core/Spec.js b/src/core/Spec.js index e69d53a7..8e39f044 100644 --- a/src/core/Spec.js +++ b/src/core/Spec.js @@ -72,8 +72,11 @@ getJasmineRequireObj().Spec = function(j$) { // Key and value will eventually be cloned during reporting. The error // thrown at that point if they aren't cloneable isn't very helpful. // Throw a better one now. - j$.private.util.assertReporterCloneable(key, 'Key'); + if (!j$.private.isString(key)) { + throw new Error('Key must be a string'); + } j$.private.util.assertReporterCloneable(value, 'Value'); + this.#executionState.properties = this.#executionState.properties || {}; this.#executionState.properties[key] = value; } diff --git a/src/core/Suite.js b/src/core/Suite.js index 6314e12c..0210e07e 100644 --- a/src/core/Suite.js +++ b/src/core/Suite.js @@ -37,8 +37,11 @@ getJasmineRequireObj().Suite = function(j$) { // Key and value will eventually be cloned during reporting. The error // thrown at that point if they aren't cloneable isn't very helpful. // Throw a better one now. - j$.private.util.assertReporterCloneable(key, 'Key'); + if (!j$.private.isString(key)) { + throw new Error('Key must be a string'); + } j$.private.util.assertReporterCloneable(value, 'Value'); + this.#result.properties = this.#result.properties || {}; this.#result.properties[key] = value; }