From 7263a38c3fb04d7fb27c468cdec8ec3eb53e0766 Mon Sep 17 00:00:00 2001 From: Dmitriy T Date: Thu, 22 Aug 2019 16:08:43 -0400 Subject: [PATCH] Adds new configuration option to failSpecWithNoExpectations that will report specs without expectations as failures if enabled --- .github/CONTRIBUTING.md | 2 +- package.json | 1 + spec/core/TreeProcessorSpec.js | 43 +++++++++++++++++---- spec/core/integration/EnvSpec.js | 35 +++++++++++++++++ spec/html/HtmlReporterSpec.js | 66 ++++++++++++++++++++------------ src/core/Env.js | 16 ++++++++ src/core/Spec.js | 18 ++++++--- src/core/TreeProcessor.js | 7 +++- src/html/HtmlReporter.js | 32 +++++++++++----- 9 files changed, 172 insertions(+), 48 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 2ee4cbd3..686c71de 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -118,7 +118,7 @@ The easiest way to run the tests in **Internet Explorer** is to run a VM that ha 1. Ensure all specs are green in browser *and* node 1. Ensure eslint and prettier are clean as part of your `npm test` command. You can run `npm run cleanup` to have prettier re-write the files. -1. Build `jasmine.js` with `grunt buildDistribution` and run all specs again - this ensures that your changes self-test well +1. Build `jasmine.js` with `npm run build` and run all specs again - this ensures that your changes self-test well 1. Revert your changes to `jasmine.js` and `jasmine-html.js` * We do this because `jasmine.js` and `jasmine-html.js` are auto-generated (as you've seen in the previous steps) and accepting multiple pull requests when this auto-generated file changes causes lots of headaches * When we accept your pull request, we will generate these files as a separate commit and merge the entire branch into master diff --git a/package.json b/package.json index 618a10bb..f5cb97b9 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "posttest": "eslint src/**/*.js spec/**/*.js && prettier --check src/**/*.js spec/**/*.js", "test": "grunt execSpecsInNode", "cleanup": "prettier --write src/**/*.js spec/**/*.js", + "build": "grunt buildDistribution", "serve": "node spec/support/localJasmineBrowser.js", "serve:performance": "node spec/support/localJasmineBrowser.js jasmine-browser-performance.json", "ci": "node spec/support/ci.js", diff --git a/spec/core/TreeProcessorSpec.js b/spec/core/TreeProcessorSpec.js index 18f4267f..5c31f100 100644 --- a/spec/core/TreeProcessorSpec.js +++ b/spec/core/TreeProcessorSpec.js @@ -296,7 +296,7 @@ describe('TreeProcessor', function() { queueRunner.calls.mostRecent().args[0].queueableFns[0].fn('foo'); - expect(leaf.execute).toHaveBeenCalledWith('foo', false); + expect(leaf.execute).toHaveBeenCalledWith('foo', false, false); }); it('runs a node with no children', function() { @@ -368,10 +368,10 @@ describe('TreeProcessor', function() { expect(queueableFns.length).toBe(3); queueableFns[1].fn('foo'); - expect(leaf1.execute).toHaveBeenCalledWith('foo', false); + expect(leaf1.execute).toHaveBeenCalledWith('foo', false, false); queueableFns[2].fn('bar'); - expect(leaf2.execute).toHaveBeenCalledWith('bar', false); + expect(leaf2.execute).toHaveBeenCalledWith('bar', false, false); }); it('cascades errors up the tree', function() { @@ -397,7 +397,7 @@ describe('TreeProcessor', function() { expect(queueableFns.length).toBe(2); queueableFns[1].fn('foo'); - expect(leaf.execute).toHaveBeenCalledWith('foo', false); + expect(leaf.execute).toHaveBeenCalledWith('foo', false, false); queueRunner.calls.mostRecent().args[0].onComplete('things'); expect(nodeComplete).toHaveBeenCalled(); @@ -433,7 +433,7 @@ describe('TreeProcessor', function() { expect(nodeStart).toHaveBeenCalledWith(node, 'bar'); queueableFns[1].fn('foo'); - expect(leaf1.execute).toHaveBeenCalledWith('foo', true); + expect(leaf1.execute).toHaveBeenCalledWith('foo', true, false); node.getResult.and.returnValue({ im: 'disabled' }); @@ -445,6 +445,35 @@ describe('TreeProcessor', function() { ); }); + it('should execute node with correct arguments when failSpecWithNoExpectations option is set', function() { + var leaf = new Leaf(), + node = new Node({ children: [leaf] }), + root = new Node({ children: [node] }), + queueRunner = jasmine.createSpy('queueRunner'), + nodeStart = jasmine.createSpy('nodeStart'), + nodeComplete = jasmine.createSpy('nodeComplete'), + processor = new jasmineUnderTest.TreeProcessor({ + tree: root, + runnableIds: [], + queueRunnerFactory: queueRunner, + nodeStart: nodeStart, + nodeComplete: nodeComplete, + failSpecWithNoExpectations: true + }), + treeComplete = jasmine.createSpy('treeComplete'), + nodeDone = jasmine.createSpy('nodeDone'); + + processor.execute(treeComplete); + var queueableFns = queueRunner.calls.mostRecent().args[0].queueableFns; + queueableFns[0].fn(nodeDone); + + queueableFns = queueRunner.calls.mostRecent().args[0].queueableFns; + expect(queueableFns.length).toBe(2); + + queueableFns[1].fn('foo'); + expect(leaf.execute).toHaveBeenCalledWith('foo', true, true); + }); + it('runs beforeAlls for a node with children', function() { var leaf = new Leaf(), node = new Node({ @@ -600,11 +629,11 @@ describe('TreeProcessor', function() { queueableFns[0].fn(); expect(nonSpecified.execute).not.toHaveBeenCalled(); - expect(specified.execute).toHaveBeenCalledWith(undefined, false); + expect(specified.execute).toHaveBeenCalledWith(undefined, false, false); queueableFns[1].fn(); - expect(nonSpecified.execute).toHaveBeenCalledWith(undefined, true); + expect(nonSpecified.execute).toHaveBeenCalledWith(undefined, true, false); }); it('runs nodes and leaves with a specified order', function() { diff --git a/spec/core/integration/EnvSpec.js b/spec/core/integration/EnvSpec.js index 45b91601..e31f7d9d 100644 --- a/spec/core/integration/EnvSpec.js +++ b/spec/core/integration/EnvSpec.js @@ -2138,6 +2138,41 @@ describe("Env integration", function() { }); }); + describe('when spec has no expectations', function() { + var env, reporter; + + beforeEach(function() { + env = new jasmineUnderTest.Env(); + reporter = jasmine.createSpyObj('reporter', ['jasmineDone', 'suiteDone', 'specDone']); + + env.addReporter(reporter); + env.it('is a spec without any expectations', function() { + // does nothing, just a mock spec without expectations + }); + + }); + + it('should report "failed" status if "failSpecWithNoExpectations" is enabled', function(done) { + reporter.jasmineDone.and.callFake(function(e) { + expect(e.overallStatus).toEqual('failed'); + done(); + }); + + env.configure({ failSpecWithNoExpectations: true }); + env.execute(); + }); + + it('should report "passed" status if "failSpecWithNoExpectations" is disabled', function(done) { + reporter.jasmineDone.and.callFake(function(e) { + expect(e.overallStatus).toEqual('passed'); + done(); + }); + + env.configure({ failSpecWithNoExpectations: false }); + env.execute(); + }); + }); + describe('When a top-level beforeAll fails', function() { it('is "failed"', function(done) { var env = new jasmineUnderTest.Env(), diff --git a/spec/html/HtmlReporterSpec.js b/spec/html/HtmlReporterSpec.js index da18deb3..526fa76c 100644 --- a/spec/html/HtmlReporterSpec.js +++ b/spec/html/HtmlReporterSpec.js @@ -62,19 +62,19 @@ describe('HtmlReporter', function() { }); describe('when a spec is done', function() { - it('logs errors to the console and prints a special symbol if it is an empty spec', function() { - if (typeof console === 'undefined') { - console = { warn: function() {} }; - } + describe('and no expectations ran', function() { + var container, reporter; + beforeEach(function() { + if (typeof console === 'undefined') { + console = { warn: function() {}, error: function() {} }; + } - var env = new jasmineUnderTest.Env(), - container = document.createElement('div'), - getContainer = function() { - return container; - }, + container = document.createElement('div'); reporter = new jasmineUnderTest.HtmlReporter({ - env: env, - getContainer: getContainer, + env: new jasmineUnderTest.Env(), + getContainer: function() { + return container; + }, createElement: function() { return document.createElement.apply(document, arguments); }, @@ -83,21 +83,39 @@ describe('HtmlReporter', function() { } }); - spyOn(console, 'warn'); + spyOn(console, 'warn'); + spyOn(console, 'error'); - reporter.initialize(); - - reporter.specDone({ - status: 'passed', - fullName: 'Some Name', - passedExpectations: [], - failedExpectations: [] + reporter.initialize(); + }); + + it('should log warning to the console and print a special symbol when empty spec status is passed', function() { + reporter.specDone({ + status: 'passed', + fullName: 'Some Name', + passedExpectations: [], + failedExpectations: [] + }); + expect(console.warn).toHaveBeenCalledWith( + "Spec 'Some Name' has no expectations." + ); + var specEl = container.querySelector('.jasmine-symbol-summary li'); + expect(specEl.getAttribute('class')).toEqual('jasmine-empty'); + }); + + it('should log error to the console and print a failure symbol when empty spec status is failed', function() { + reporter.specDone({ + status: 'failed', + fullName: 'Some Name', + passedExpectations: [], + failedExpectations: [] + }); + expect(console.error).toHaveBeenCalledWith( + "Spec 'Some Name' has no expectations." + ); + var specEl = container.querySelector('.jasmine-symbol-summary li'); + expect(specEl.getAttribute('class')).toEqual('jasmine-failed'); }); - expect(console.warn).toHaveBeenCalledWith( - "Spec 'Some Name' has no expectations." - ); - var specEl = container.querySelector('.jasmine-symbol-summary li'); - expect(specEl.getAttribute('class')).toEqual('jasmine-empty'); }); it('reports the status symbol of a excluded spec', function() { diff --git a/src/core/Env.js b/src/core/Env.js index 20221550..7d6a1210 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -65,6 +65,16 @@ getJasmineRequireObj().Env = function(j$) { * @default false */ failFast: false, + /** + * Whether to fail the spec if it ran no expectations. By default + * a spec that ran no expectations is reported as passed. Setting this + * to true will report such spec as a failure. + * @name Configuration#failSpecWithNoExpectations + * @since 3.5.0 + * @type Boolean + * @default false + */ + failSpecWithNoExpectations: false, /** * Whether to cause specs to only have one expectation failure. * @name Configuration#oneFailurePerSpec @@ -167,6 +177,11 @@ getJasmineRequireObj().Env = function(j$) { config.failFast = configuration.failFast; } + if (configuration.hasOwnProperty('failSpecWithNoExpectations')) { + config.failSpecWithNoExpectations = + configuration.failSpecWithNoExpectations; + } + if (configuration.hasOwnProperty('oneFailurePerSpec')) { config.oneFailurePerSpec = configuration.oneFailurePerSpec; } @@ -641,6 +656,7 @@ getJasmineRequireObj().Env = function(j$) { tree: topSuite, runnableIds: runnablesToRun, queueRunnerFactory: queueRunnerFactory, + failSpecWithNoExpectations: config.failSpecWithNoExpectations, nodeStart: function(suite, next) { currentlyExecutingSuites.push(suite); defaultResourcesForRunnable(suite.id, suite.parentSuite.id); diff --git a/src/core/Spec.js b/src/core/Spec.js index 53a4f0ef..b8791890 100644 --- a/src/core/Spec.js +++ b/src/core/Spec.js @@ -82,7 +82,7 @@ getJasmineRequireObj().Spec = function(j$) { return this.asyncExpectationFactory(actual, this); }; - Spec.prototype.execute = function(onComplete, excluded) { + Spec.prototype.execute = function(onComplete, excluded, failSpecWithNoExp) { var self = this; var onStart = { @@ -95,7 +95,7 @@ getJasmineRequireObj().Spec = function(j$) { var complete = { fn: function(done) { self.queueableFn.fn = null; - self.result.status = self.status(excluded); + self.result.status = self.status(excluded, failSpecWithNoExp); self.resultCallback(self.result, done); } }; @@ -166,7 +166,7 @@ getJasmineRequireObj().Spec = function(j$) { return this.result; }; - Spec.prototype.status = function(excluded) { + Spec.prototype.status = function(excluded, failSpecWithNoExpectations) { if (excluded === true) { return 'excluded'; } @@ -175,11 +175,17 @@ getJasmineRequireObj().Spec = function(j$) { return 'pending'; } - if (this.result.failedExpectations.length > 0) { + if ( + this.result.failedExpectations.length > 0 || + (failSpecWithNoExpectations && + this.result.failedExpectations.length + + this.result.passedExpectations.length === + 0) + ) { return 'failed'; - } else { - return 'passed'; } + + return 'passed'; }; Spec.prototype.getFullName = function() { diff --git a/src/core/TreeProcessor.js b/src/core/TreeProcessor.js index 469c1009..14798d77 100644 --- a/src/core/TreeProcessor.js +++ b/src/core/TreeProcessor.js @@ -5,6 +5,7 @@ getJasmineRequireObj().TreeProcessor = function() { queueRunnerFactory = attrs.queueRunnerFactory, nodeStart = attrs.nodeStart || function() {}, nodeComplete = attrs.nodeComplete || function() {}, + failSpecWithNoExpectations = !!attrs.failSpecWithNoExpectations, orderChildren = attrs.orderChildren || function(node) { @@ -222,7 +223,11 @@ getJasmineRequireObj().TreeProcessor = function() { } else { return { fn: function(done) { - node.execute(done, stats[node.id].excluded); + node.execute( + done, + stats[node.id].excluded, + failSpecWithNoExpectations + ); } }; } diff --git a/src/html/HtmlReporter.js b/src/html/HtmlReporter.js index 6bc8d1eb..1c6478cf 100644 --- a/src/html/HtmlReporter.js +++ b/src/html/HtmlReporter.js @@ -112,12 +112,13 @@ jasmineRequire.HtmlReporter = function(j$) { this.specDone = function(result) { stateBuilder.specDone(result); - if ( - noExpectations(result) && - typeof console !== 'undefined' && - typeof console.error !== 'undefined' - ) { - console.warn("Spec '" + result.fullName + "' has no expectations."); + if (noExpectations(result)) { + var noSpecMsg = "Spec '" + result.fullName + "' has no expectations."; + if (result.status === 'failed') { + console.error(noSpecMsg); + } else { + console.warn(noSpecMsg); + } } if (!symbols) { @@ -140,7 +141,7 @@ jasmineRequire.HtmlReporter = function(j$) { }; this.displaySpecInCorrectFormat = function(result) { - return noExpectations(result) + return noExpectations(result) && result.status === 'passed' ? 'jasmine-empty' : this.resultStatus(result.status); }; @@ -363,6 +364,16 @@ jasmineRequire.HtmlReporter = function(j$) { ); } + if (result.failedExpectations.length === 0) { + messages.appendChild( + createDom( + 'div', + { className: 'jasmine-result-message' }, + 'Spec has no expectations' + ) + ); + } + return failure; } @@ -654,9 +665,12 @@ jasmineRequire.HtmlReporter = function(j$) { } function noExpectations(result) { + var allExpectations = + result.failedExpectations.length + result.passedExpectations.length; + return ( - result.failedExpectations.length + result.passedExpectations.length === - 0 && result.status === 'passed' + allExpectations === 0 && + (result.status === 'passed' || result.status === 'failed') ); }