From da2673e02f634afb02d3f193182464f33ad3b4e1 Mon Sep 17 00:00:00 2001 From: ksvitkovsky Date: Wed, 9 Aug 2017 11:58:01 +0400 Subject: [PATCH 1/4] Test nested methods to throw an error --- spec/core/integration/EnvSpec.js | 61 ++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/spec/core/integration/EnvSpec.js b/spec/core/integration/EnvSpec.js index 53415660..39577725 100644 --- a/spec/core/integration/EnvSpec.js +++ b/spec/core/integration/EnvSpec.js @@ -1895,4 +1895,65 @@ describe("Env integration", function() { env.execute(); }); + + it('should throw on suites/specs/befores/afters nested in methods other than \'describe\'', function(done) { + var env = new jasmineUnderTest.Env(), + reporter = jasmine.createSpyObj('reporter', ['jasmineDone', 'suiteDone', 'specDone']); + + reporter.jasmineDone.and.callFake(function() { + var msg = /\'.*\' should only be used in \'describe\' function/; + + expect(reporter.specDone).toHaveFailedExpecationsForRunnable('suite describe', [msg]); + expect(reporter.specDone).toHaveFailedExpecationsForRunnable('suite xdescribe', [msg]); + expect(reporter.specDone).toHaveFailedExpecationsForRunnable('suite fdescribe', [msg]); + + expect(reporter.specDone).toHaveFailedExpecationsForRunnable('spec it', [msg]); + expect(reporter.specDone).toHaveFailedExpecationsForRunnable('spec xit', [msg]); + expect(reporter.specDone).toHaveFailedExpecationsForRunnable('spec fit', [msg]); + + expect(reporter.specDone).toHaveFailedExpecationsForRunnable('beforeAll spec', [msg]); + expect(reporter.specDone).toHaveFailedExpecationsForRunnable('beforeEach spec', [msg]); + + expect(reporter.suiteDone).toHaveFailedExpecationsForRunnable('afterAll', [msg]); + expect(reporter.specDone).toHaveFailedExpecationsForRunnable('afterEach spec', [msg]); + + done(); + }); + + env.addReporter(reporter); + + env.describe('suite', function() { + env.it('describe', function() { env.describe('inner suite', function() {}); }); + env.it('xdescribe', function() { env.xdescribe('inner suite', function() {}); }); + env.it('fdescribe', function() { env.fdescribe('inner suite', function() {}); }); + }); + + env.describe('spec', function() { + env.it('it', function() { env.it('inner spec', function() {}); }); + env.it('xit', function() { env.xit('inner spec', function() {}); }); + env.it('fit', function() { env.fit('inner spec', function() {}); }); + }); + + env.describe('beforeAll', function() { + env.beforeAll(function() { env.beforeAll(function() {}); }); + env.it('spec', function() {}); + }); + + env.describe('beforeEach', function() { + env.beforeEach(function() { env.beforeEach(function() {}); }); + env.it('spec', function() {}); + }); + + env.describe('afterAll', function() { + env.afterAll(function() { env.afterAll(function() {}); }); + env.it('spec', function() {}); + }); + + env.describe('afterEach', function() { + env.afterEach(function() { env.afterEach(function() {}); }); + env.it('spec', function() {}); + }); + + env.execute(); + }); }); From ab116fbd0f03ae1548cf3dcdc392a0ae156f6720 Mon Sep 17 00:00:00 2001 From: ksvitkovsky Date: Wed, 9 Aug 2017 12:29:02 +0400 Subject: [PATCH 2/4] Throw an error on nested methods --- src/core/Env.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/core/Env.js b/src/core/Env.js index b26c7db0..86e86ac4 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -384,6 +384,12 @@ getJasmineRequireObj().Env = function(j$) { } }; + function ensureIsNotNested(method, currentSuite) { + if (currentSuite !== undefined) { + throw new Error('\'' + method + '\' should only be used in \'describe\' function'); + } + } + var suiteFactory = function(description) { var suite = new j$.Suite({ env: self, @@ -399,6 +405,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.describe = function(description, specDefinitions) { + ensureIsNotNested('describe', currentSuite()); ensureIsFunction(specDefinitions, 'describe'); var suite = suiteFactory(description); if (specDefinitions.length > 0) { @@ -412,6 +419,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.xdescribe = function(description, specDefinitions) { + ensureIsNotNested('xdescribe', currentSuite()); ensureIsFunction(specDefinitions, 'xdescribe'); var suite = suiteFactory(description); suite.pend(); @@ -422,6 +430,7 @@ getJasmineRequireObj().Env = function(j$) { var focusedRunnables = []; this.fdescribe = function(description, specDefinitions) { + ensureIsNotNested('fdescribe', currentSuite()); ensureIsFunction(specDefinitions, 'fdescribe'); var suite = suiteFactory(description); suite.isFocused = true; @@ -519,6 +528,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.it = function(description, fn, timeout) { + ensureIsNotNested('it', currentSuite()); // it() sometimes doesn't have a fn argument, so only check the type if // it's given. if (arguments.length > 1 && typeof fn !== 'undefined') { @@ -533,6 +543,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.xit = function(description, fn, timeout) { + ensureIsNotNested('xit', currentSuite()); // xit(), like it(), doesn't always have a fn argument, so only check the // type when needed. if (arguments.length > 1 && typeof fn !== 'undefined') { @@ -544,6 +555,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.fit = function(description, fn, timeout){ + ensureIsNotNested('fit', currentSuite()); ensureIsFunctionOrAsync(fn, 'fit'); var spec = specFactory(description, fn, currentDeclarationSuite, timeout); currentDeclarationSuite.addChild(spec); @@ -561,6 +573,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.beforeEach = function(beforeEachFunction, timeout) { + ensureIsNotNested('beforeEach', currentSuite()); ensureIsFunctionOrAsync(beforeEachFunction, 'beforeEach'); currentDeclarationSuite.beforeEach({ fn: beforeEachFunction, @@ -569,6 +582,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.beforeAll = function(beforeAllFunction, timeout) { + ensureIsNotNested('beforeAll', currentSuite()); ensureIsFunctionOrAsync(beforeAllFunction, 'beforeAll'); currentDeclarationSuite.beforeAll({ fn: beforeAllFunction, @@ -577,6 +591,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.afterEach = function(afterEachFunction, timeout) { + ensureIsNotNested('afterEach', currentSuite()); ensureIsFunctionOrAsync(afterEachFunction, 'afterEach'); afterEachFunction.isCleanup = true; currentDeclarationSuite.afterEach({ @@ -586,6 +601,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.afterAll = function(afterAllFunction, timeout) { + ensureIsNotNested('afterAll', currentSuite()); ensureIsFunctionOrAsync(afterAllFunction, 'afterAll'); currentDeclarationSuite.afterAll({ fn: afterAllFunction, From 4240b3514b75b9d13a9be73daab4804105caedb1 Mon Sep 17 00:00:00 2001 From: ksvitkovsky Date: Thu, 10 Aug 2017 22:58:30 +0400 Subject: [PATCH 3/4] Check runnable instead of suite --- src/core/Env.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/core/Env.js b/src/core/Env.js index 86e86ac4..6ec112bd 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -384,8 +384,8 @@ getJasmineRequireObj().Env = function(j$) { } }; - function ensureIsNotNested(method, currentSuite) { - if (currentSuite !== undefined) { + function ensureIsNotNested(method, runnable) { + if (runnable !== null && runnable !== undefined) { throw new Error('\'' + method + '\' should only be used in \'describe\' function'); } } @@ -405,7 +405,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.describe = function(description, specDefinitions) { - ensureIsNotNested('describe', currentSuite()); + ensureIsNotNested('describe', currentRunnable()); ensureIsFunction(specDefinitions, 'describe'); var suite = suiteFactory(description); if (specDefinitions.length > 0) { @@ -419,7 +419,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.xdescribe = function(description, specDefinitions) { - ensureIsNotNested('xdescribe', currentSuite()); + ensureIsNotNested('xdescribe', currentRunnable()); ensureIsFunction(specDefinitions, 'xdescribe'); var suite = suiteFactory(description); suite.pend(); @@ -430,7 +430,7 @@ getJasmineRequireObj().Env = function(j$) { var focusedRunnables = []; this.fdescribe = function(description, specDefinitions) { - ensureIsNotNested('fdescribe', currentSuite()); + ensureIsNotNested('fdescribe', currentRunnable()); ensureIsFunction(specDefinitions, 'fdescribe'); var suite = suiteFactory(description); suite.isFocused = true; @@ -528,7 +528,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.it = function(description, fn, timeout) { - ensureIsNotNested('it', currentSuite()); + ensureIsNotNested('it', currentRunnable()); // it() sometimes doesn't have a fn argument, so only check the type if // it's given. if (arguments.length > 1 && typeof fn !== 'undefined') { @@ -543,7 +543,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.xit = function(description, fn, timeout) { - ensureIsNotNested('xit', currentSuite()); + ensureIsNotNested('xit', currentRunnable()); // xit(), like it(), doesn't always have a fn argument, so only check the // type when needed. if (arguments.length > 1 && typeof fn !== 'undefined') { @@ -555,7 +555,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.fit = function(description, fn, timeout){ - ensureIsNotNested('fit', currentSuite()); + ensureIsNotNested('fit', currentRunnable()); ensureIsFunctionOrAsync(fn, 'fit'); var spec = specFactory(description, fn, currentDeclarationSuite, timeout); currentDeclarationSuite.addChild(spec); @@ -573,7 +573,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.beforeEach = function(beforeEachFunction, timeout) { - ensureIsNotNested('beforeEach', currentSuite()); + ensureIsNotNested('beforeEach', currentRunnable()); ensureIsFunctionOrAsync(beforeEachFunction, 'beforeEach'); currentDeclarationSuite.beforeEach({ fn: beforeEachFunction, @@ -582,7 +582,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.beforeAll = function(beforeAllFunction, timeout) { - ensureIsNotNested('beforeAll', currentSuite()); + ensureIsNotNested('beforeAll', currentRunnable()); ensureIsFunctionOrAsync(beforeAllFunction, 'beforeAll'); currentDeclarationSuite.beforeAll({ fn: beforeAllFunction, @@ -591,7 +591,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.afterEach = function(afterEachFunction, timeout) { - ensureIsNotNested('afterEach', currentSuite()); + ensureIsNotNested('afterEach', currentRunnable()); ensureIsFunctionOrAsync(afterEachFunction, 'afterEach'); afterEachFunction.isCleanup = true; currentDeclarationSuite.afterEach({ @@ -601,7 +601,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.afterAll = function(afterAllFunction, timeout) { - ensureIsNotNested('afterAll', currentSuite()); + ensureIsNotNested('afterAll', currentRunnable()); ensureIsFunctionOrAsync(afterAllFunction, 'afterAll'); currentDeclarationSuite.afterAll({ fn: afterAllFunction, From 7ca571a74657d2e408581b2fc3d5d4810d4d88d1 Mon Sep 17 00:00:00 2001 From: ksvitkovsky Date: Thu, 10 Aug 2017 23:08:59 +0400 Subject: [PATCH 4/4] Remove runnable parameter for simplicity --- src/core/Env.js | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/core/Env.js b/src/core/Env.js index 6ec112bd..9655f15d 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -384,7 +384,8 @@ getJasmineRequireObj().Env = function(j$) { } }; - function ensureIsNotNested(method, runnable) { + function ensureIsNotNested(method) { + var runnable = currentRunnable(); if (runnable !== null && runnable !== undefined) { throw new Error('\'' + method + '\' should only be used in \'describe\' function'); } @@ -405,7 +406,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.describe = function(description, specDefinitions) { - ensureIsNotNested('describe', currentRunnable()); + ensureIsNotNested('describe'); ensureIsFunction(specDefinitions, 'describe'); var suite = suiteFactory(description); if (specDefinitions.length > 0) { @@ -419,7 +420,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.xdescribe = function(description, specDefinitions) { - ensureIsNotNested('xdescribe', currentRunnable()); + ensureIsNotNested('xdescribe'); ensureIsFunction(specDefinitions, 'xdescribe'); var suite = suiteFactory(description); suite.pend(); @@ -430,7 +431,7 @@ getJasmineRequireObj().Env = function(j$) { var focusedRunnables = []; this.fdescribe = function(description, specDefinitions) { - ensureIsNotNested('fdescribe', currentRunnable()); + ensureIsNotNested('fdescribe'); ensureIsFunction(specDefinitions, 'fdescribe'); var suite = suiteFactory(description); suite.isFocused = true; @@ -528,7 +529,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.it = function(description, fn, timeout) { - ensureIsNotNested('it', currentRunnable()); + ensureIsNotNested('it'); // it() sometimes doesn't have a fn argument, so only check the type if // it's given. if (arguments.length > 1 && typeof fn !== 'undefined') { @@ -543,7 +544,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.xit = function(description, fn, timeout) { - ensureIsNotNested('xit', currentRunnable()); + ensureIsNotNested('xit'); // xit(), like it(), doesn't always have a fn argument, so only check the // type when needed. if (arguments.length > 1 && typeof fn !== 'undefined') { @@ -555,7 +556,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.fit = function(description, fn, timeout){ - ensureIsNotNested('fit', currentRunnable()); + ensureIsNotNested('fit'); ensureIsFunctionOrAsync(fn, 'fit'); var spec = specFactory(description, fn, currentDeclarationSuite, timeout); currentDeclarationSuite.addChild(spec); @@ -573,7 +574,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.beforeEach = function(beforeEachFunction, timeout) { - ensureIsNotNested('beforeEach', currentRunnable()); + ensureIsNotNested('beforeEach'); ensureIsFunctionOrAsync(beforeEachFunction, 'beforeEach'); currentDeclarationSuite.beforeEach({ fn: beforeEachFunction, @@ -582,7 +583,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.beforeAll = function(beforeAllFunction, timeout) { - ensureIsNotNested('beforeAll', currentRunnable()); + ensureIsNotNested('beforeAll'); ensureIsFunctionOrAsync(beforeAllFunction, 'beforeAll'); currentDeclarationSuite.beforeAll({ fn: beforeAllFunction, @@ -591,7 +592,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.afterEach = function(afterEachFunction, timeout) { - ensureIsNotNested('afterEach', currentRunnable()); + ensureIsNotNested('afterEach'); ensureIsFunctionOrAsync(afterEachFunction, 'afterEach'); afterEachFunction.isCleanup = true; currentDeclarationSuite.afterEach({ @@ -601,7 +602,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.afterAll = function(afterAllFunction, timeout) { - ensureIsNotNested('afterAll', currentRunnable()); + ensureIsNotNested('afterAll'); ensureIsFunctionOrAsync(afterAllFunction, 'afterAll'); currentDeclarationSuite.afterAll({ fn: afterAllFunction,