From d5e7bc9fd62acf65eb6cd2fe778e9f2255014d57 Mon Sep 17 00:00:00 2001 From: Steve Gravrock Date: Sun, 10 Nov 2024 09:32:43 -0800 Subject: [PATCH] Optionally enforce uniqueness of spec and suite names This is off by default for backwards compatibility but can be enabled by setting the forbidDuplicateNames env config property to true. Fixes #1633. --- lib/jasmine-core/jasmine.js | 44 +++++++++++- spec/core/SuiteBuilderSpec.js | 111 +++++++++++++++++++++++++++++++ spec/core/SuiteSpec.js | 27 ++++++++ spec/core/integration/EnvSpec.js | 9 +++ src/core/Env.js | 12 +++- src/core/Suite.js | 10 +++ src/core/SuiteBuilder.js | 22 ++++++ 7 files changed, 233 insertions(+), 2 deletions(-) diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index 2df1c885..0e1af983 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -1295,6 +1295,15 @@ getJasmineRequireObj().Env = function(j$) { * @default true */ autoCleanClosures: true, + /** + * Whether to forbid duplicate spec or suite names. If set to true, using + * the same name multiple times in the same immediate parent suite is an + * error. + * @name Configuration#forbidDuplicateNames + * @type boolean + * @default false + */ + forbidDuplicateNames: false, /** * Whether or not to issue warnings for certain deprecated functionality * every time it's used. If not set or set to false, deprecation warnings @@ -1343,7 +1352,8 @@ getJasmineRequireObj().Env = function(j$) { 'hideDisabled', 'stopOnSpecFailure', 'stopSpecOnExpectationFailure', - 'autoCleanClosures' + 'autoCleanClosures', + 'forbidDuplicateNames', ]; booleanProps.forEach(function(prop) { @@ -10272,6 +10282,16 @@ getJasmineRequireObj().Suite = function(j$) { ); }; + Suite.prototype.hasChildWithDescription = function(description) { + for (const child of this.children) { + if (child.description === description) { + return true; + } + } + + return false; + }; + Object.defineProperty(Suite.prototype, 'metadata', { get: function() { if (!this.metadata_) { @@ -10509,6 +10529,8 @@ getJasmineRequireObj().SuiteBuilder = function(j$) { j$.util.validateTimeout(timeout); } + this.checkDuplicate_(description, 'spec'); + const spec = this.specFactory_(description, fn, timeout, filename); if (this.currentDeclarationSuite_.markedExcluding) { spec.exclude(); @@ -10518,7 +10540,27 @@ getJasmineRequireObj().SuiteBuilder = function(j$) { return spec; } + checkDuplicate_(description, type) { + if (!this.env_.configuration().forbidDuplicateNames) { + return; + } + + if (this.currentDeclarationSuite_.hasChildWithDescription(description)) { + const parentDesc = + this.currentDeclarationSuite_ === this.topSuite + ? 'top suite' + : `"${this.currentDeclarationSuite_.getFullName()}"`; + throw new Error( + `Duplicate ${type} name "${description}" found in ${parentDesc}` + ); + } + } + suiteFactory_(description, filename) { + if (this.topSuite) { + this.checkDuplicate_(description, 'suite'); + } + const config = this.env_.configuration(); const parentSuite = this.currentDeclarationSuite_; const reportedParentSuiteId = diff --git a/spec/core/SuiteBuilderSpec.js b/spec/core/SuiteBuilderSpec.js index 082d3b38..0bde3a43 100644 --- a/spec/core/SuiteBuilderSpec.js +++ b/spec/core/SuiteBuilderSpec.js @@ -176,6 +176,117 @@ describe('SuiteBuilder', function() { }; } + describe('Duplicate name handling', function() { + describe('When forbidDuplicateNames is true', function() { + let env; + + beforeEach(function() { + env = { configuration: () => ({ forbidDuplicateNames: true }) }; + }); + + it('forbids duplicate spec names', function() { + const suiteBuilder = new jasmineUnderTest.SuiteBuilder({ env }); + + expect(function() { + suiteBuilder.describe('a suite', function() { + suiteBuilder.describe('a nested suite', function() { + suiteBuilder.it('a spec'); + suiteBuilder.it('a spec'); + }); + }); + }).toThrowError( + 'Duplicate spec name "a spec" found in "a suite a nested suite"' + ); + }); + + it('forbids duplicate spec names in the top suite', function() { + const suiteBuilder = new jasmineUnderTest.SuiteBuilder({ env }); + + expect(function() { + suiteBuilder.it('another spec'); + suiteBuilder.it('another spec'); + }).toThrowError( + 'Duplicate spec name "another spec" found in top suite' + ); + }); + + it('forbids duplicate suite names', function() { + const suiteBuilder = new jasmineUnderTest.SuiteBuilder({ env }); + + expect(function() { + suiteBuilder.describe('a suite', function() { + suiteBuilder.describe('a nested suite', function() { + suiteBuilder.describe('another suite', function() { + suiteBuilder.it('a spec'); + }); + suiteBuilder.describe('another suite', function() { + suiteBuilder.it('a spec'); + }); + }); + }); + }).toThrowError( + 'Duplicate suite name "another suite" found in "a suite a nested suite"' + ); + }); + + it('forbids duplicate suite names in the top suite', function() { + const suiteBuilder = new jasmineUnderTest.SuiteBuilder({ env }); + + expect(function() { + suiteBuilder.describe('a suite', function() { + suiteBuilder.it('a spec'); + }); + suiteBuilder.describe('a suite', function() { + suiteBuilder.it('a spec'); + }); + }).toThrowError('Duplicate suite name "a suite" found in top suite'); + }); + + it('allows spec and suite names to be duplicated in different suites', function() { + const suiteBuilder = new jasmineUnderTest.SuiteBuilder({ env }); + + expect(function() { + suiteBuilder.describe('suite a', function() { + suiteBuilder.describe('dupe suite', function() { + suiteBuilder.it('dupe spec'); + suiteBuilder.describe('child suite', function() { + suiteBuilder.it('dupe spec'); + }); + }); + }); + suiteBuilder.describe('suite b', function() { + suiteBuilder.describe('dupe suite', function() { + suiteBuilder.it('dupe spec'); + }); + }); + }).not.toThrow(); + }); + }); + + describe('When forbidDuplicateNames is false', function() { + let env; + + beforeEach(function() { + env = { configuration: () => ({ forbidDuplicateNames: false }) }; + }); + + it('allows duplicate spec and suite names', function() { + const suiteBuilder = new jasmineUnderTest.SuiteBuilder({ env }); + + expect(function() { + suiteBuilder.describe('dupe suite', function() { + suiteBuilder.it('dupe spec'); + suiteBuilder.it('dupe spec'); + }); + suiteBuilder.describe('dupe suite', function() { + suiteBuilder.it('dupe spec'); + suiteBuilder.it('dupe spec'); + }); + }).not.toThrow(); + }); + }); + }); + describe('#parallelReset', function() { it('resets the top suite result', function() { jasmineUnderTest.Suite.prototype.handleException.and.callThrough(); diff --git a/spec/core/SuiteSpec.js b/spec/core/SuiteSpec.js index e811723a..a3f7dc71 100644 --- a/spec/core/SuiteSpec.js +++ b/spec/core/SuiteSpec.js @@ -378,4 +378,31 @@ describe('Suite', function() { ); }); }); + + describe('#hasChildWithDescription', function() { + it('returns true if there is a child with the given description', function() { + const subject = new jasmineUnderTest.Suite({}); + const description = 'a spec'; + subject.addChild({ description }); + + expect(subject.hasChildWithDescription(description)).toBeTrue(); + }); + + it('returns false if there is no child with the given description', function() { + const subject = new jasmineUnderTest.Suite({}); + subject.addChild({ description: 'a different spec' }); + + expect(subject.hasChildWithDescription('a spec')).toBeFalse(); + }); + + it('does not recurse into child suites', function() { + const subject = new jasmineUnderTest.Suite({}); + const childSuite = new jasmineUnderTest.Suite({}); + subject.addChild(childSuite); + const description = 'a spec'; + childSuite.addChild(description); + + expect(subject.hasChildWithDescription('a spec')).toBeFalse(); + }); + }); }); diff --git a/spec/core/integration/EnvSpec.js b/spec/core/integration/EnvSpec.js index c380f93d..e50647e5 100644 --- a/spec/core/integration/EnvSpec.js +++ b/spec/core/integration/EnvSpec.js @@ -4444,6 +4444,15 @@ describe('Env integration', function() { }); }); + it('forbids duplicates when forbidDuplicateNames is true', function() { + env.configure({ forbidDuplicateNames: true }); + env.it('a spec'); + + expect(function() { + env.it('a spec'); + }).toThrowError('Duplicate spec name "a spec" found in top suite'); + }); + function browserEventMethods() { return { listeners_: { error: [], unhandledrejection: [] }, diff --git a/src/core/Env.js b/src/core/Env.js index e48e6395..d546dc4e 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -138,6 +138,15 @@ getJasmineRequireObj().Env = function(j$) { * @default true */ autoCleanClosures: true, + /** + * Whether to forbid duplicate spec or suite names. If set to true, using + * the same name multiple times in the same immediate parent suite is an + * error. + * @name Configuration#forbidDuplicateNames + * @type boolean + * @default false + */ + forbidDuplicateNames: false, /** * Whether or not to issue warnings for certain deprecated functionality * every time it's used. If not set or set to false, deprecation warnings @@ -186,7 +195,8 @@ getJasmineRequireObj().Env = function(j$) { 'hideDisabled', 'stopOnSpecFailure', 'stopSpecOnExpectationFailure', - 'autoCleanClosures' + 'autoCleanClosures', + 'forbidDuplicateNames' ]; booleanProps.forEach(function(prop) { diff --git a/src/core/Suite.js b/src/core/Suite.js index eda79d6c..106666df 100644 --- a/src/core/Suite.js +++ b/src/core/Suite.js @@ -251,6 +251,16 @@ getJasmineRequireObj().Suite = function(j$) { ); }; + Suite.prototype.hasChildWithDescription = function(description) { + for (const child of this.children) { + if (child.description === description) { + return true; + } + } + + return false; + }; + Object.defineProperty(Suite.prototype, 'metadata', { get: function() { if (!this.metadata_) { diff --git a/src/core/SuiteBuilder.js b/src/core/SuiteBuilder.js index 24fea487..ef6010c9 100644 --- a/src/core/SuiteBuilder.js +++ b/src/core/SuiteBuilder.js @@ -161,6 +161,8 @@ getJasmineRequireObj().SuiteBuilder = function(j$) { j$.util.validateTimeout(timeout); } + this.checkDuplicate_(description, 'spec'); + const spec = this.specFactory_(description, fn, timeout, filename); if (this.currentDeclarationSuite_.markedExcluding) { spec.exclude(); @@ -170,7 +172,27 @@ getJasmineRequireObj().SuiteBuilder = function(j$) { return spec; } + checkDuplicate_(description, type) { + if (!this.env_.configuration().forbidDuplicateNames) { + return; + } + + if (this.currentDeclarationSuite_.hasChildWithDescription(description)) { + const parentDesc = + this.currentDeclarationSuite_ === this.topSuite + ? 'top suite' + : `"${this.currentDeclarationSuite_.getFullName()}"`; + throw new Error( + `Duplicate ${type} name "${description}" found in ${parentDesc}` + ); + } + } + suiteFactory_(description, filename) { + if (this.topSuite) { + this.checkDuplicate_(description, 'suite'); + } + const config = this.env_.configuration(); const parentSuite = this.currentDeclarationSuite_; const reportedParentSuiteId =