From d4fd2a6439814aa7b4ed137a51de6931f64b207c Mon Sep 17 00:00:00 2001 From: Arvid Ottenberg Date: Tue, 12 May 2020 20:03:47 +0200 Subject: [PATCH] --forbid-only doesn't recognize `it.only` when `before` crashes (#4256); closes #3840 * add fixtures that result in it.only combined with --forbid-only bug * adapt forbidonly test cases to cover it.only bug * check if forbid only option is set prior to marking a test only and throw error * change name of only test back to previous * use custom assertion for expecting error in forbidOnly tests * remove empty line * use createUnsupportedError instead of throw new Error * use createUnsupportedError instead of throw new Error * remove check if suite hasOnly and forbidOnly option is set as this is done before runtime * implement markOnly instance method in suite class * add unit test for suites markOnly method * throw exception if --forbid-only option is set even if suite is not selected by grep * adapt forbidOnly integration tests to check for failure if only suite is not selected by grep * fix jsdocs of suite markonly * Revert "fix jsdocs of suite markonly" This reverts commit cffc71a5c55114805a05a08c131439b993cf7d1c. * Revert "adapt forbidOnly integration tests to check for failure if only suite is not selected by grep" This reverts commit 336425ad89d9fbc7b728040ae02c9ec013cf727e. * Revert "throw exception if --forbid-only option is set even if suite is not selected by grep" This reverts commit f87178241fc74a54739497a5f1faedbf1cab5e76. * Revert "add unit test for suites markOnly method" This reverts commit c2c8dc8b2fc2fdaf73c2edaf5afee78edbf15576. * Revert "implement markOnly instance method in suite class" This reverts commit 4b37e3c1f6ecee6a90ef8c12dc8e3e7baabe98b1. --- lib/interfaces/common.js | 7 +- lib/runner.js | 5 -- .../forbid-only/only-before-each.fixture.js | 8 +++ .../forbid-only/only-before.fixture.js | 8 +++ test/integration/options/forbidOnly.spec.js | 70 +++++++++++++------ 5 files changed, 71 insertions(+), 27 deletions(-) create mode 100644 test/integration/fixtures/options/forbid-only/only-before-each.fixture.js create mode 100644 test/integration/fixtures/options/forbid-only/only-before.fixture.js diff --git a/lib/interfaces/common.js b/lib/interfaces/common.js index 7991e113f7..e7e84b2511 100644 --- a/lib/interfaces/common.js +++ b/lib/interfaces/common.js @@ -3,6 +3,7 @@ var Suite = require('../suite'); var errors = require('../errors'); var createMissingArgumentError = errors.createMissingArgumentError; +var createUnsupportedError = errors.createUnsupportedError; /** * Functions common to more than one interface. @@ -126,14 +127,14 @@ module.exports = function(suites, context, mocha) { suites.unshift(suite); if (opts.isOnly) { if (mocha.options.forbidOnly && shouldBeTested(suite)) { - throw new Error('`.only` forbidden'); + throw createUnsupportedError('`.only` forbidden'); } suite.parent.appendOnlySuite(suite); } if (suite.pending) { if (mocha.options.forbidPending && shouldBeTested(suite)) { - throw new Error('Pending test forbidden'); + throw createUnsupportedError('Pending test forbidden'); } } if (typeof opts.fn === 'function') { @@ -165,6 +166,8 @@ module.exports = function(suites, context, mocha) { * @returns {*} */ only: function(mocha, test) { + if (mocha.options.forbidOnly) + throw createUnsupportedError('`.only` forbidden'); test.markOnly(); return test; }, diff --git a/lib/runner.js b/lib/runner.js index 8fcf9d968e..936192f55d 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -577,11 +577,6 @@ Runner.prototype.runTest = function(fn) { return; } - var suite = this.parents().reverse()[0] || this.suite; - if (this.forbidOnly && suite.hasOnly()) { - fn(new Error('`.only` forbidden')); - return; - } if (this.asyncOnly) { test.asyncOnly = true; } diff --git a/test/integration/fixtures/options/forbid-only/only-before-each.fixture.js b/test/integration/fixtures/options/forbid-only/only-before-each.fixture.js new file mode 100644 index 0000000000..19bfb86409 --- /dev/null +++ b/test/integration/fixtures/options/forbid-only/only-before-each.fixture.js @@ -0,0 +1,8 @@ +'use strict'; + +describe('test marked with only and beforeEach has skip', function() { + beforeEach(function() { + this.skip(); + }); + it.only('only test', function() {}); +}); diff --git a/test/integration/fixtures/options/forbid-only/only-before.fixture.js b/test/integration/fixtures/options/forbid-only/only-before.fixture.js new file mode 100644 index 0000000000..3924ac4082 --- /dev/null +++ b/test/integration/fixtures/options/forbid-only/only-before.fixture.js @@ -0,0 +1,8 @@ +'use strict'; + +describe('test marked with only and before has skip', function() { + before(function() { + this.skip(); + }); + it.only('only test', function() {}); +}); diff --git a/test/integration/options/forbidOnly.spec.js b/test/integration/options/forbidOnly.spec.js index 592f0f25df..1886becb7d 100644 --- a/test/integration/options/forbidOnly.spec.js +++ b/test/integration/options/forbidOnly.spec.js @@ -26,13 +26,19 @@ describe('--forbid-only', function() { it('should fail if there are tests marked only', function(done) { var fixture = path.join('options', 'forbid-only', 'only'); - runMochaJSON(fixture, args, function(err, res) { - if (err) { - return done(err); - } - expect(res, 'to have failed with error', onlyErrorMessage); - done(); - }); + var spawnOpts = {stdio: 'pipe'}; + runMocha( + fixture, + args, + function(err, res) { + if (err) { + return done(err); + } + expect(res, 'to have failed with output', new RegExp(onlyErrorMessage)); + done(); + }, + spawnOpts + ); }); it('should fail if there are tests in suites marked only', function(done) { @@ -45,11 +51,7 @@ describe('--forbid-only', function() { if (err) { return done(err); } - - expect(res, 'to satisfy', { - code: 1, - output: new RegExp(onlyErrorMessage) - }); + expect(res, 'to have failed with output', new RegExp(onlyErrorMessage)); done(); }, spawnOpts @@ -66,10 +68,7 @@ describe('--forbid-only', function() { if (err) { return done(err); } - expect(res, 'to satisfy', { - code: 1, - output: new RegExp(onlyErrorMessage) - }); + expect(res, 'to have failed with output', new RegExp(onlyErrorMessage)); done(); }, spawnOpts @@ -86,10 +85,7 @@ describe('--forbid-only', function() { if (err) { return done(err); } - expect(res, 'to satisfy', { - code: 1, - output: new RegExp(onlyErrorMessage) - }); + expect(res, 'to have failed with output', new RegExp(onlyErrorMessage)); done(); }, spawnOpts @@ -124,4 +120,38 @@ describe('--forbid-only', function() { } ); }); + + it('should fail even if before has "skip"', function(done) { + var fixture = path.join('options', 'forbid-only', 'only-before'); + var spawnOpts = {stdio: 'pipe'}; + runMocha( + fixture, + args, + function(err, res) { + if (err) { + return done(err); + } + expect(res, 'to have failed with output', new RegExp(onlyErrorMessage)); + done(); + }, + spawnOpts + ); + }); + + it('should fail even if beforeEach has "skip"', function(done) { + var fixture = path.join('options', 'forbid-only', 'only-before-each'); + var spawnOpts = {stdio: 'pipe'}; + runMocha( + fixture, + args, + function(err, res) { + if (err) { + return done(err); + } + expect(res, 'to have failed with output', new RegExp(onlyErrorMessage)); + done(); + }, + spawnOpts + ); + }); });