diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index e906c5b2..278a4393 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -161,7 +161,11 @@ getJasmineRequireObj().base = function(j$, jasmineGlobal) { }; j$.isA_ = function(typeName, value) { - return Object.prototype.toString.apply(value) === '[object ' + typeName + ']'; + return j$.getType_(value) === '[object ' + typeName + ']'; + }; + + j$.getType_ = function(value) { + return Object.prototype.toString.apply(value); }; j$.isDomNode = function(obj) { @@ -829,6 +833,12 @@ getJasmineRequireObj().Env = function(j$) { return spyRegistry.spyOnProperty.apply(spyRegistry, arguments); }; + var ensureIsFunction = function(fn, caller) { + if (!j$.isFunction_(fn)) { + throw new Error(caller + ' expects a function argument; received ' + j$.getType_(fn)); + } + }; + var suiteFactory = function(description) { var suite = new j$.Suite({ env: self, @@ -844,6 +854,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.describe = function(description, specDefinitions) { + ensureIsFunction(specDefinitions, 'describe'); var suite = suiteFactory(description); if (specDefinitions.length > 0) { throw new Error('describe does not expect any arguments'); @@ -856,6 +867,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.xdescribe = function(description, specDefinitions) { + ensureIsFunction(specDefinitions, 'xdescribe'); var suite = suiteFactory(description); suite.pend(); addSpecsToSuite(suite, specDefinitions); @@ -865,6 +877,7 @@ getJasmineRequireObj().Env = function(j$) { var focusedRunnables = []; this.fdescribe = function(description, specDefinitions) { + ensureIsFunction(specDefinitions, 'fdescribe'); var suite = suiteFactory(description); suite.isFocused = true; @@ -961,6 +974,11 @@ getJasmineRequireObj().Env = function(j$) { }; this.it = function(description, fn, timeout) { + // it() sometimes doesn't have a fn argument, so only check the type if + // it's given. + if (arguments.length > 1) { + ensureIsFunction(fn, 'it'); + } var spec = specFactory(description, fn, currentDeclarationSuite, timeout); if (currentDeclarationSuite.markedPending) { spec.pend(); @@ -969,13 +987,19 @@ getJasmineRequireObj().Env = function(j$) { return spec; }; - this.xit = function() { + this.xit = function(description, fn, timeout) { + // xit(), like it(), doesn't always have a fn argument, so only check the + // type when needed. + if (arguments.length > 1) { + ensureIsFunction(fn, 'xit'); + } var spec = this.it.apply(this, arguments); spec.pend('Temporarily disabled with xit'); return spec; }; this.fit = function(description, fn, timeout){ + ensureIsFunction(fn, 'fit'); var spec = specFactory(description, fn, currentDeclarationSuite, timeout); currentDeclarationSuite.addChild(spec); focusedRunnables.push(spec.id); @@ -992,6 +1016,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.beforeEach = function(beforeEachFunction, timeout) { + ensureIsFunction(beforeEachFunction, 'beforeEach'); currentDeclarationSuite.beforeEach({ fn: beforeEachFunction, timeout: function() { return timeout || j$.DEFAULT_TIMEOUT_INTERVAL; } @@ -999,6 +1024,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.beforeAll = function(beforeAllFunction, timeout) { + ensureIsFunction(beforeAllFunction, 'beforeAll'); currentDeclarationSuite.beforeAll({ fn: beforeAllFunction, timeout: function() { return timeout || j$.DEFAULT_TIMEOUT_INTERVAL; } @@ -1006,6 +1032,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.afterEach = function(afterEachFunction, timeout) { + ensureIsFunction(afterEachFunction, 'afterEach'); currentDeclarationSuite.afterEach({ fn: afterEachFunction, timeout: function() { return timeout || j$.DEFAULT_TIMEOUT_INTERVAL; } @@ -1013,6 +1040,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.afterAll = function(afterAllFunction, timeout) { + ensureIsFunction(afterAllFunction, 'afterAll'); currentDeclarationSuite.afterAll({ fn: afterAllFunction, timeout: function() { return timeout || j$.DEFAULT_TIMEOUT_INTERVAL; } diff --git a/spec/core/EnvSpec.js b/spec/core/EnvSpec.js index 896e5f56..935f48d0 100644 --- a/spec/core/EnvSpec.js +++ b/spec/core/EnvSpec.js @@ -26,15 +26,6 @@ describe("Env", function() { }); }); - describe('#describe', function () { - var spec = function(done){}; - it("throws the error", function() { - expect(function() { - env.describe('done method', spec); - }).toThrow(new Error('describe does not expect any arguments')); - }); - }); - it('can configure specs to throw errors on expectation failures', function() { env.throwOnExpectationFailure(true); @@ -55,14 +46,113 @@ describe("Env", function() { })); }); + describe('#describe', function () { + it("throws an error when given arguments", function() { + expect(function() { + env.describe('done method', function(done) {}); + }).toThrowError('describe does not expect any arguments'); + }); + + it('throws an error when it receives a non-fn argument', function() { + // Some versions of PhantomJS return [object DOMWindow] when + // Object.prototype.toString.apply is called with `undefined` or `null`. + // In a similar fashion, IE8 gives [object Object] for both `undefined` + // and `null`. We mostly just want these tests to check that using + // anything other than a function throws an error. + expect(function() { + env.describe('undefined arg', undefined); + }).toThrowError(/describe expects a function argument; received \[object (Undefined|DOMWindow|Object)\]/); + expect(function() { + env.describe('null arg', null); + }).toThrowError(/describe expects a function argument; received \[object (Null|DOMWindow|Object)\]/); + + expect(function() { + env.describe('array arg', []); + }).toThrowError('describe expects a function argument; received [object Array]'); + expect(function() { + env.describe('object arg', {}); + }).toThrowError('describe expects a function argument; received [object Object]'); + + expect(function() { + env.describe('fn arg', function() {}); + }).not.toThrowError('describe expects a function argument; received [object Function]'); + }); + }); + + describe('#it', function () { + it('throws an error when it receives a non-fn argument', function() { + expect(function() { + env.it('undefined arg', undefined); + }).toThrowError(/it expects a function argument; received \[object (Undefined|DOMWindow|Object)\]/); + }); + + it('does not throw when it is not given a fn argument', function() { + expect(function() { + env.it('pending spec'); + }).not.toThrow(); + }); + }); + describe('#xit', function() { it('calls spec.pend with "Temporarily disabled with xit"', function() { var pendSpy = jasmine.createSpy(); spyOn(env, 'it').and.returnValue({ pend: pendSpy }); - env.xit(); + env.xit('foo', function() {}); expect(pendSpy).toHaveBeenCalledWith('Temporarily disabled with xit'); }); + + it('throws an error when it receives a non-fn argument', function() { + expect(function() { + env.xit('undefined arg', undefined); + }).toThrowError(/xit expects a function argument; received \[object (Undefined|DOMWindow|Object)\]/); + }); + + it('does not throw when it is not given a fn argument', function() { + expect(function() { + env.xit('pending spec'); + }).not.toThrow(); + }); + }); + + describe('#fit', function () { + it('throws an error when it receives a non-fn argument', function() { + expect(function() { + env.fit('undefined arg', undefined); + }).toThrowError(/fit expects a function argument; received \[object (Undefined|DOMWindow|Object)\]/); + }); + }); + + describe('#beforeEach', function () { + it('throws an error when it receives a non-fn argument', function() { + expect(function() { + env.beforeEach(undefined); + }).toThrowError(/beforeEach expects a function argument; received \[object (Undefined|DOMWindow|Object)\]/); + }); + }); + + describe('#beforeAll', function () { + it('throws an error when it receives a non-fn argument', function() { + expect(function() { + env.beforeAll(undefined); + }).toThrowError(/beforeAll expects a function argument; received \[object (Undefined|DOMWindow|Object)\]/); + }); + }); + + describe('#afterEach', function () { + it('throws an error when it receives a non-fn argument', function() { + expect(function() { + env.afterEach(undefined); + }).toThrowError(/afterEach expects a function argument; received \[object (Undefined|DOMWindow|Object)\]/); + }); + }); + + describe('#afterAll', function () { + it('throws an error when it receives a non-fn argument', function() { + expect(function() { + env.afterAll(undefined); + }).toThrowError(/afterAll expects a function argument; received \[object (Undefined|DOMWindow|Object)\]/); + }); }); }); diff --git a/src/core/Env.js b/src/core/Env.js index 33c4b9f9..b7337774 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -292,6 +292,12 @@ getJasmineRequireObj().Env = function(j$) { return spyRegistry.spyOnProperty.apply(spyRegistry, arguments); }; + var ensureIsFunction = function(fn, caller) { + if (!j$.isFunction_(fn)) { + throw new Error(caller + ' expects a function argument; received ' + j$.getType_(fn)); + } + }; + var suiteFactory = function(description) { var suite = new j$.Suite({ env: self, @@ -307,6 +313,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.describe = function(description, specDefinitions) { + ensureIsFunction(specDefinitions, 'describe'); var suite = suiteFactory(description); if (specDefinitions.length > 0) { throw new Error('describe does not expect any arguments'); @@ -319,6 +326,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.xdescribe = function(description, specDefinitions) { + ensureIsFunction(specDefinitions, 'xdescribe'); var suite = suiteFactory(description); suite.pend(); addSpecsToSuite(suite, specDefinitions); @@ -328,6 +336,7 @@ getJasmineRequireObj().Env = function(j$) { var focusedRunnables = []; this.fdescribe = function(description, specDefinitions) { + ensureIsFunction(specDefinitions, 'fdescribe'); var suite = suiteFactory(description); suite.isFocused = true; @@ -424,6 +433,11 @@ getJasmineRequireObj().Env = function(j$) { }; this.it = function(description, fn, timeout) { + // it() sometimes doesn't have a fn argument, so only check the type if + // it's given. + if (arguments.length > 1) { + ensureIsFunction(fn, 'it'); + } var spec = specFactory(description, fn, currentDeclarationSuite, timeout); if (currentDeclarationSuite.markedPending) { spec.pend(); @@ -432,13 +446,19 @@ getJasmineRequireObj().Env = function(j$) { return spec; }; - this.xit = function() { + this.xit = function(description, fn, timeout) { + // xit(), like it(), doesn't always have a fn argument, so only check the + // type when needed. + if (arguments.length > 1) { + ensureIsFunction(fn, 'xit'); + } var spec = this.it.apply(this, arguments); spec.pend('Temporarily disabled with xit'); return spec; }; this.fit = function(description, fn, timeout){ + ensureIsFunction(fn, 'fit'); var spec = specFactory(description, fn, currentDeclarationSuite, timeout); currentDeclarationSuite.addChild(spec); focusedRunnables.push(spec.id); @@ -455,6 +475,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.beforeEach = function(beforeEachFunction, timeout) { + ensureIsFunction(beforeEachFunction, 'beforeEach'); currentDeclarationSuite.beforeEach({ fn: beforeEachFunction, timeout: function() { return timeout || j$.DEFAULT_TIMEOUT_INTERVAL; } @@ -462,6 +483,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.beforeAll = function(beforeAllFunction, timeout) { + ensureIsFunction(beforeAllFunction, 'beforeAll'); currentDeclarationSuite.beforeAll({ fn: beforeAllFunction, timeout: function() { return timeout || j$.DEFAULT_TIMEOUT_INTERVAL; } @@ -469,6 +491,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.afterEach = function(afterEachFunction, timeout) { + ensureIsFunction(afterEachFunction, 'afterEach'); currentDeclarationSuite.afterEach({ fn: afterEachFunction, timeout: function() { return timeout || j$.DEFAULT_TIMEOUT_INTERVAL; } @@ -476,6 +499,7 @@ getJasmineRequireObj().Env = function(j$) { }; this.afterAll = function(afterAllFunction, timeout) { + ensureIsFunction(afterAllFunction, 'afterAll'); currentDeclarationSuite.afterAll({ fn: afterAllFunction, timeout: function() { return timeout || j$.DEFAULT_TIMEOUT_INTERVAL; } diff --git a/src/core/base.js b/src/core/base.js index a7c40604..51311a47 100644 --- a/src/core/base.js +++ b/src/core/base.js @@ -38,7 +38,11 @@ getJasmineRequireObj().base = function(j$, jasmineGlobal) { }; j$.isA_ = function(typeName, value) { - return Object.prototype.toString.apply(value) === '[object ' + typeName + ']'; + return j$.getType_(value) === '[object ' + typeName + ']'; + }; + + j$.getType_ = function(value) { + return Object.prototype.toString.apply(value); }; j$.isDomNode = function(obj) {