From e14d9c4be3c31548a904ddf2b417a7c3f363cbe3 Mon Sep 17 00:00:00 2001 From: Steve Gravrock Date: Tue, 11 Oct 2022 19:43:25 -0700 Subject: [PATCH] Parallel: forbid beforeAll/afterAll at the top level Either running these once total or running them once per process would be the wrong choice for a significant chunk of users, so do neither. Later we'll add a new API for exactly-once setup and teardown in parallel mode. --- lib/jasmine-core/jasmine.js | 18 +++++++-- spec/core/EnvSpec.js | 79 +++++++++++++++++++++++++++++++++++++ src/core/Env.js | 18 +++++++-- 3 files changed, 107 insertions(+), 8 deletions(-) diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index 04bb65eb..f752cdb9 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -1188,7 +1188,7 @@ getJasmineRequireObj().Env = function(j$) { let reporter; let topSuite; let runner; - let parallelLodingState = null; // 'specs', 'helpers', or null for non-parallel + let parallelLoadingState = null; // 'specs', 'helpers', or null for non-parallel /** * This represents the available options to configure Jasmine. @@ -1636,7 +1636,7 @@ getJasmineRequireObj().Env = function(j$) { }); this.setParallelLoadingState = function(state) { - parallelLodingState = state; + parallelLoadingState = state; }; this.parallelReset = function() { @@ -1807,13 +1807,21 @@ getJasmineRequireObj().Env = function(j$) { } function ensureNonParallel(method) { - if (parallelLodingState) { + if (parallelLoadingState) { throw new Error(`'${method}' is not available in parallel mode`); } } + function ensureNonParallelOrInDescribe(method) { + if (parallelLoadingState && !suiteBuilder.inDescribe()) { + throw new Error( + `In parallel mode, '${method}' must be in a describe block` + ); + } + } + function ensureNonParallelOrInHelperOrInDescribe(method) { - if (parallelLodingState === 'specs' && !suiteBuilder.inDescribe()) { + if (parallelLoadingState === 'specs' && !suiteBuilder.inDescribe()) { throw new Error( 'In parallel mode, ' + method + @@ -1951,6 +1959,7 @@ getJasmineRequireObj().Env = function(j$) { this.beforeAll = function(beforeAllFunction, timeout) { ensureIsNotNested('beforeAll'); + ensureNonParallelOrInDescribe('beforeAll'); suiteBuilder.beforeAll(beforeAllFunction, timeout); }; @@ -1962,6 +1971,7 @@ getJasmineRequireObj().Env = function(j$) { this.afterAll = function(afterAllFunction, timeout) { ensureIsNotNested('afterAll'); + ensureNonParallelOrInDescribe('afterAll'); suiteBuilder.afterAll(afterAllFunction, timeout); }; diff --git a/spec/core/EnvSpec.js b/spec/core/EnvSpec.js index 7a0c2229..a2d25a9c 100644 --- a/spec/core/EnvSpec.js +++ b/spec/core/EnvSpec.js @@ -1,5 +1,6 @@ // TODO: Fix these unit tests! describe('Env', function() { + beforeAll(function() {}); let env; beforeEach(function() { env = new jasmineUnderTest.Env(); @@ -454,6 +455,45 @@ describe('Env', function() { env.beforeAll(function() {}, 2147483648); }).toThrowError('Timeout value cannot be greater than 2147483647'); }); + + describe('in parallel mode', function() { + it('throws an error when called at the top level', function() { + env.setParallelLoadingState('helpers'); + check(); + env.setParallelLoadingState('specs'); + check(); + + function check() { + expect(function() { + env.beforeAll(function() {}); + }).toThrowError( + "In parallel mode, 'beforeAll' must be in a describe block" + ); + } + }); + + it('does not throw an error when called in a describe', function() { + env.setParallelLoadingState('helpers'); + check(); + env.setParallelLoadingState('specs'); + check(); + + function check() { + let done = false; + + env.describe('a suite', function() { + expect(function() { + env.it('a spec'); + env.beforeAll(function() {}); + }).not.toThrow(); + + done = true; + }); + + expect(done).toBeTrue(); + } + }); + }); }); describe('#afterEach', function() { @@ -520,6 +560,45 @@ describe('Env', function() { env.afterAll(function() {}, 2147483648); }).toThrowError('Timeout value cannot be greater than 2147483647'); }); + + describe('in parallel mode', function() { + it('throws an error when called at the top level', function() { + env.setParallelLoadingState('helpers'); + check(); + env.setParallelLoadingState('specs'); + check(); + + function check() { + expect(function() { + env.afterAll(function() {}); + }).toThrowError( + "In parallel mode, 'afterAll' must be in a describe block" + ); + } + }); + + it('does not throw an error when called in a describe', function() { + env.setParallelLoadingState('helpers'); + check(); + env.setParallelLoadingState('specs'); + check(); + + function check() { + let done = false; + + env.describe('a suite', function() { + expect(function() { + env.it('a spec'); + env.afterAll(function() {}); + }).not.toThrow(); + + done = true; + }); + + expect(done).toBeTrue(); + } + }); + }); }); describe('when not constructed with suppressLoadErrors: true', function() { diff --git a/src/core/Env.js b/src/core/Env.js index 84507056..6f6a1387 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -46,7 +46,7 @@ getJasmineRequireObj().Env = function(j$) { let reporter; let topSuite; let runner; - let parallelLodingState = null; // 'specs', 'helpers', or null for non-parallel + let parallelLoadingState = null; // 'specs', 'helpers', or null for non-parallel /** * This represents the available options to configure Jasmine. @@ -494,7 +494,7 @@ getJasmineRequireObj().Env = function(j$) { }); this.setParallelLoadingState = function(state) { - parallelLodingState = state; + parallelLoadingState = state; }; this.parallelReset = function() { @@ -665,13 +665,21 @@ getJasmineRequireObj().Env = function(j$) { } function ensureNonParallel(method) { - if (parallelLodingState) { + if (parallelLoadingState) { throw new Error(`'${method}' is not available in parallel mode`); } } + function ensureNonParallelOrInDescribe(method) { + if (parallelLoadingState && !suiteBuilder.inDescribe()) { + throw new Error( + `In parallel mode, '${method}' must be in a describe block` + ); + } + } + function ensureNonParallelOrInHelperOrInDescribe(method) { - if (parallelLodingState === 'specs' && !suiteBuilder.inDescribe()) { + if (parallelLoadingState === 'specs' && !suiteBuilder.inDescribe()) { throw new Error( 'In parallel mode, ' + method + @@ -809,6 +817,7 @@ getJasmineRequireObj().Env = function(j$) { this.beforeAll = function(beforeAllFunction, timeout) { ensureIsNotNested('beforeAll'); + ensureNonParallelOrInDescribe('beforeAll'); suiteBuilder.beforeAll(beforeAllFunction, timeout); }; @@ -820,6 +829,7 @@ getJasmineRequireObj().Env = function(j$) { this.afterAll = function(afterAllFunction, timeout) { ensureIsNotNested('afterAll'); + ensureNonParallelOrInDescribe('afterAll'); suiteBuilder.afterAll(afterAllFunction, timeout); };