From b9874b5728119bad6c9e00897cb7d7ddb8541d36 Mon Sep 17 00:00:00 2001 From: TG Date: Thu, 19 Dec 2024 16:19:44 +0100 Subject: [PATCH 01/12] fix: improve error handling in global fixtures #5208 --- lib/mocha.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/mocha.js b/lib/mocha.js index c6ee248561..5ff39f157a 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -881,7 +881,7 @@ Mocha.prototype.failZero = function (failZero) { * @return {Mocha} this * @chainable */ -Mocha.prototype.passOnFailingTestSuite = function(passOnFailingTestSuite) { +Mocha.prototype.passOnFailingTestSuite = function (passOnFailingTestSuite) { this.options.passOnFailingTestSuite = passOnFailingTestSuite === true; return this; }; @@ -1229,8 +1229,12 @@ Mocha.prototype._runGlobalFixtures = async function _runGlobalFixtures( fixtureFns = [], context = {} ) { - for await (const fixtureFn of fixtureFns) { - await fixtureFn.call(context); + for (const fixtureFn of fixtureFns) { + try { + await fixtureFn.call(context); + } catch (err) { + console.error(err.stack); + } } return context; }; From bc63cb0f3daea7104136aa2f9dcebe438d769192 Mon Sep 17 00:00:00 2001 From: TG Date: Thu, 19 Dec 2024 17:39:41 +0100 Subject: [PATCH 02/12] remove test file --- lib/mocha.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mocha.js b/lib/mocha.js index 5ff39f157a..124c4152f6 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -1219,7 +1219,7 @@ Mocha.prototype.runGlobalTeardown = async function runGlobalTeardown( }; /** - * Run global fixtures sequentially with context `context` + * Run global fixtures sequentially with context `context`. * @private * @param {MochaGlobalFixture[]} [fixtureFns] - Fixtures to run * @param {object} [context] - context object From f0d9524ea470c2f9146aebfe0c33af0c4e2f8c7d Mon Sep 17 00:00:00 2001 From: Kelechi Ebiri <56020538+TG199@users.noreply.github.com> Date: Fri, 3 Jan 2025 13:03:48 +0100 Subject: [PATCH 03/12] Update lib/mocha.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit improve format Co-authored-by: Josh Goldberg ✨ --- lib/mocha.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mocha.js b/lib/mocha.js index 124c4152f6..8732470134 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -881,7 +881,7 @@ Mocha.prototype.failZero = function (failZero) { * @return {Mocha} this * @chainable */ -Mocha.prototype.passOnFailingTestSuite = function (passOnFailingTestSuite) { +Mocha.prototype.passOnFailingTestSuite = function(passOnFailingTestSuite) { this.options.passOnFailingTestSuite = passOnFailingTestSuite === true; return this; }; From ae2c3a3a2b92ef3cdd1fe9e46b0fd90c56ec2de4 Mon Sep 17 00:00:00 2001 From: TG Date: Sat, 4 Jan 2025 00:33:19 +0100 Subject: [PATCH 04/12] mark failed test --- lib/mocha.js | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/mocha.js b/lib/mocha.js index 8732470134..4a3387c13c 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -1225,19 +1225,23 @@ Mocha.prototype.runGlobalTeardown = async function runGlobalTeardown( * @param {object} [context] - context object * @returns {Promise} context object */ -Mocha.prototype._runGlobalFixtures = async function _runGlobalFixtures( - fixtureFns = [], - context = {} -) { - for (const fixtureFn of fixtureFns) { - try { - await fixtureFn.call(context); - } catch (err) { - console.error(err.stack); + Mocha.prototype._runGlobalFixtures = async function _runGlobalFixtures( + fixtureFns = [], + context = {} + ) { + for (const fixtureFn of fixtureFns) { + try { + await fixtureFn.call(context); + } catch (err) { + if (this.runner) { + this.runner.failures += 1; + } + console.error('Global fixture error:', err.stack); + + } } - } - return context; -}; + return context; + }; /** * Toggle execution of any global setup fixture(s) From 23194873aa3c6f672875407907ac61701758e976 Mon Sep 17 00:00:00 2001 From: TG Date: Fri, 31 Jan 2025 23:04:06 +0100 Subject: [PATCH 05/12] create false hook to represent global teardown failure --- lib/mocha.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/mocha.js b/lib/mocha.js index 4a3387c13c..806351a602 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -1233,11 +1233,12 @@ Mocha.prototype.runGlobalTeardown = async function runGlobalTeardown( try { await fixtureFn.call(context); } catch (err) { - if (this.runner) { - this.runner.failures += 1; - } console.error('Global fixture error:', err.stack); + if (this.runner) { + const falseHook = new this._Hook('globalTeardown', 'global teardown'); + this.runner.failHook(falseHook, err); + } } } return context; From 4649a6af7f8575fb1a5309cc20da66fa74db3a4f Mon Sep 17 00:00:00 2001 From: TG Date: Fri, 31 Jan 2025 23:04:29 +0100 Subject: [PATCH 06/12] Add test --- .../fixtures/global-teardown-error.js | 9 ++++++++ .../global-teardown-errors.spec.js | 22 +++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 test/integration/fixtures/global-teardown-error.js create mode 100644 test/integration/global-teardown-errors.spec.js diff --git a/test/integration/fixtures/global-teardown-error.js b/test/integration/fixtures/global-teardown-error.js new file mode 100644 index 0000000000..a2b72aaee4 --- /dev/null +++ b/test/integration/fixtures/global-teardown-error.js @@ -0,0 +1,9 @@ +'use strict' + +const { it } = require('../../../lib/mocha'); + +it('should pass', () => {}); + +exports.mochaTeardown = async function () { + throw new Error('Teardown problem') +} \ No newline at end of file diff --git a/test/integration/global-teardown-errors.spec.js b/test/integration/global-teardown-errors.spec.js new file mode 100644 index 0000000000..40a3e5cacd --- /dev/null +++ b/test/integration/global-teardown-errors.spec.js @@ -0,0 +1,22 @@ +'use strict' + +const { spawnSync } = require('child_process'); +const { resolve} = require('path'); +const { expect } = require('chai'); + +describe('global teardown errors', () => { + it('should fail with non-zero exit code and report the teardown error', () => { + + const testFile = resolve(__dirname, '../fixtures/global-teardown-errors.js'); + + const result = spawnSync(process.execPath, [ + './bin/mocha', + testFile, + '--reporter', 'spec' + ], { encoding: 'utf-8' }); + + expect(result.status).to.equal(1); + expect(result.stdout).to.include('Global Teardown Error:'); + expect(result.stdout).to.include('Teardown problem'); + }); +}); \ No newline at end of file From 041536ddf27efd2d34f7f8e494275a5b27e5ebf9 Mon Sep 17 00:00:00 2001 From: TG Date: Fri, 31 Jan 2025 23:25:07 +0100 Subject: [PATCH 07/12] correct assertion --- test/integration/global-teardown-errors.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/global-teardown-errors.spec.js b/test/integration/global-teardown-errors.spec.js index 40a3e5cacd..d61408cf05 100644 --- a/test/integration/global-teardown-errors.spec.js +++ b/test/integration/global-teardown-errors.spec.js @@ -16,7 +16,7 @@ describe('global teardown errors', () => { ], { encoding: 'utf-8' }); expect(result.status).to.equal(1); - expect(result.stdout).to.include('Global Teardown Error:'); + expect(result.stdout).to.include('Global fixture error'); expect(result.stdout).to.include('Teardown problem'); }); }); \ No newline at end of file From d6ff7ccb5237e4fd4b2dd0983ce18d87506f6886 Mon Sep 17 00:00:00 2001 From: TG Date: Sat, 1 Feb 2025 00:23:55 +0100 Subject: [PATCH 08/12] fix test file path, correct global teardown type --- test/integration/fixtures/global-teardown-error.js | 2 +- test/integration/global-teardown-errors.spec.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/integration/fixtures/global-teardown-error.js b/test/integration/fixtures/global-teardown-error.js index a2b72aaee4..b958030f1b 100644 --- a/test/integration/fixtures/global-teardown-error.js +++ b/test/integration/fixtures/global-teardown-error.js @@ -4,6 +4,6 @@ const { it } = require('../../../lib/mocha'); it('should pass', () => {}); -exports.mochaTeardown = async function () { +exports.mochaGlobalTeardown = async function () { throw new Error('Teardown problem') } \ No newline at end of file diff --git a/test/integration/global-teardown-errors.spec.js b/test/integration/global-teardown-errors.spec.js index d61408cf05..54ea3cc708 100644 --- a/test/integration/global-teardown-errors.spec.js +++ b/test/integration/global-teardown-errors.spec.js @@ -7,7 +7,7 @@ const { expect } = require('chai'); describe('global teardown errors', () => { it('should fail with non-zero exit code and report the teardown error', () => { - const testFile = resolve(__dirname, '../fixtures/global-teardown-errors.js'); + const testFile = resolve(__dirname, './fixtures/global-teardown-error.js'); const result = spawnSync(process.execPath, [ './bin/mocha', @@ -16,7 +16,7 @@ describe('global teardown errors', () => { ], { encoding: 'utf-8' }); expect(result.status).to.equal(1); - expect(result.stdout).to.include('Global fixture error'); - expect(result.stdout).to.include('Teardown problem'); + expect(result.stderr).to.include('Global fixture error'); + expect(result.stderr).to.include('Error: Teardown problem'); }); }); \ No newline at end of file From b00630cbdab157b8d1f8fb1b8ab6d97f8ca6175a Mon Sep 17 00:00:00 2001 From: TG Date: Wed, 5 Feb 2025 23:31:47 +0100 Subject: [PATCH 09/12] Changes: - Add globalTeardownErrored flag to track teardown failures - Override Mocha.run to combine test failures with teardown errors --- lib/mocha.js | 48 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/lib/mocha.js b/lib/mocha.js index 806351a602..fdb7b8dbff 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -184,6 +184,7 @@ function Mocha(options = {}) { options = {...mocharc, ...options}; this.files = []; this.options = options; + this.globalTeardownErrored = false; // root suite this.suite = new exports.Suite('', new exports.Context(), true); this._cleanReferencesAfterRun = true; @@ -987,6 +988,7 @@ Mocha.prototype.run = function (fn) { if (this.files.length && !this._lazyLoadFiles) { this.loadFiles(); } + var suite = this.suite; var options = this.options; options.files = this.files; @@ -1225,24 +1227,37 @@ Mocha.prototype.runGlobalTeardown = async function runGlobalTeardown( * @param {object} [context] - context object * @returns {Promise} context object */ - Mocha.prototype._runGlobalFixtures = async function _runGlobalFixtures( - fixtureFns = [], - context = {} - ) { - for (const fixtureFn of fixtureFns) { - try { - await fixtureFn.call(context); - } catch (err) { - console.error('Global fixture error:', err.stack); - if (this.runner) { - const falseHook = new this._Hook('globalTeardown', 'global teardown'); - this.runner.failHook(falseHook, err); - } - } + Mocha.prototype._runGlobalFixtures = async function _runGlobalFixtures( + fixtureFns = [], + context = {}, +) { + for (const fixtureFn of fixtureFns) { + try { + await fixtureFn.call(context); + } catch (err) { + console.error('Global fixture error:', err.stack); + this.globalTeardownErrored = true; } - return context; - }; + } + return context; +}; + +/** + * Override of the original run method to handle global teardown errors. + * @param {Function} [fn] - Callback function to be invoked when test run is complete + * @returns {Runner} Runner instance + * @public + */ +const originalRun = Mocha.prototype.run; +Mocha.prototype.run = function (fn) { + const self = this; + return originalRun.call(this, (exitCode) => { + // Combine test failures and teardown errors + const finalExitCode = exitCode > 0 || self.globalTeardownErrored ? 1 : 0; + if (fn) fn(finalExitCode); + }); +}; /** * Toggle execution of any global setup fixture(s) @@ -1339,3 +1354,4 @@ Mocha.prototype.hasGlobalTeardownFixtures = * @param {Array<*>} impls - User-supplied implementations * @returns {Promise<*>|*} */ + From 848f6a720fc119834c1f6d9fff30f7bcec613697 Mon Sep 17 00:00:00 2001 From: TG Date: Thu, 6 Feb 2025 01:44:36 +0100 Subject: [PATCH 10/12] Add tests --- lib/mocha.js | 3 +- .../global-teardown-errors.spec.js | 106 +++++++++++++++--- 2 files changed, 94 insertions(+), 15 deletions(-) diff --git a/lib/mocha.js b/lib/mocha.js index fdb7b8dbff..7ee7e58979 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -1227,8 +1227,7 @@ Mocha.prototype.runGlobalTeardown = async function runGlobalTeardown( * @param {object} [context] - context object * @returns {Promise} context object */ - - Mocha.prototype._runGlobalFixtures = async function _runGlobalFixtures( +Mocha.prototype._runGlobalFixtures = async function _runGlobalFixtures( fixtureFns = [], context = {}, ) { diff --git a/test/integration/global-teardown-errors.spec.js b/test/integration/global-teardown-errors.spec.js index 54ea3cc708..981f657e1b 100644 --- a/test/integration/global-teardown-errors.spec.js +++ b/test/integration/global-teardown-errors.spec.js @@ -1,22 +1,102 @@ 'use strict' +const assert = require('assert'); +const path = require('path'); const { spawnSync } = require('child_process'); -const { resolve} = require('path'); -const { expect } = require('chai'); +const fs = require('fs').promises; -describe('global teardown errors', () => { - it('should fail with non-zero exit code and report the teardown error', () => { +describe('Global Teardown Error Handling', function() { + this.timeout(5000); - const testFile = resolve(__dirname, './fixtures/global-teardown-error.js'); + const setupFile = 'test/fixtures/global-teardown/setup.js'; + const testFile = 'test/fixtures/global-teardown/test.js'; - const result = spawnSync(process.execPath, [ - './bin/mocha', - testFile, - '--reporter', 'spec' - ], { encoding: 'utf-8' }); + before(async function() { + await fs.mkdir(path.dirname(setupFile), { recursive: true }); - expect(result.status).to.equal(1); - expect(result.stderr).to.include('Global fixture error'); - expect(result.stderr).to.include('Error: Teardown problem'); + await fs.writeFile(setupFile, ` + exports.mochaGlobalTeardown = async function () { + throw new Error('Teardown failure'); + }; + `); + + await fs.writeFile(testFile, ` + describe('Test Suite', function() { + it('passing test', function() { + // This test passes + }); + }); + `); + }); + + after(async function() { + await fs.rm(path.dirname(setupFile), { recursive: true, force: true }); + }); + + it('should fail with non-zero exit code when global teardown fails', function() { + const result = spawnSync('node', [ + 'bin/mocha', + '--require', setupFile, + testFile + ], { + encoding: 'utf8' + }); + + assert.strictEqual(result.status, 1, 'Process should exit with code 1'); + + assert(result.stderr.includes('Teardown failure') || + result.stdout.includes('Teardown failure'), + 'Should show teardown error message'); + }); + + it('should combine test failures with teardown failures', async function() { + + await fs.writeFile(testFile, ` + describe('Test Suite', function() { + it('failing test', function() { + throw new Error('Test failure'); + }); + }); + `); + + const result = spawnSync('node', [ + 'bin/mocha', + '--require', setupFile, + testFile + ], { + encoding: 'utf8' + }); + + assert.strictEqual(result.status, 1, 'Process should exit with code 1'); + + const output = result.stdout + result.stderr; + assert(output.includes('Test failure'), 'Should show test error'); + assert(output.includes('Teardown failure'), 'Should show teardown error'); + }); + + it('should pass with zero exit code when no errors occur', async function() { + await fs.writeFile(setupFile, ` + exports.mochaGlobalTeardown = async function () { + // Success case + }; + `); + + await fs.writeFile(testFile, ` + describe('Test Suite', function() { + it('passing test', function() { + // This test passes + }); + }); + `); + + const result = spawnSync('node', [ + 'bin/mocha', + '--require', setupFile, + testFile + ], { + encoding: 'utf8' + }); + + assert.strictEqual(result.status, 0, 'Process should exit with code 0'); }); }); \ No newline at end of file From 2731673dfffdcd339170cfd9ea7367dedd1ea0b2 Mon Sep 17 00:00:00 2001 From: TG Date: Fri, 7 Feb 2025 22:28:21 +0100 Subject: [PATCH 11/12] Mark failed test: properly mark failed global fixtuers test when they fail. --- lib/mocha.js | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/lib/mocha.js b/lib/mocha.js index 7ee7e58979..54b82ae8c0 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -1235,28 +1235,24 @@ Mocha.prototype._runGlobalFixtures = async function _runGlobalFixtures( try { await fixtureFn.call(context); } catch (err) { - console.error('Global fixture error:', err.stack); - this.globalTeardownErrored = true; + if(context.stats) { + context.stats.failures++; + context.failures++; + + const isSetup = this.options.globalSetup && this.options.globalSetup.includes(fixtureFn); + const phase = isSetup ? 'Global Setup' : 'Global Teardown'; + + context.emit('fail', { + title: phase, + err: err + }); + console.error(err); + } } } return context; }; -/** - * Override of the original run method to handle global teardown errors. - * @param {Function} [fn] - Callback function to be invoked when test run is complete - * @returns {Runner} Runner instance - * @public - */ -const originalRun = Mocha.prototype.run; -Mocha.prototype.run = function (fn) { - const self = this; - return originalRun.call(this, (exitCode) => { - // Combine test failures and teardown errors - const finalExitCode = exitCode > 0 || self.globalTeardownErrored ? 1 : 0; - if (fn) fn(finalExitCode); - }); -}; /** * Toggle execution of any global setup fixture(s) From 38dfe9ba60cc4cfa5e8650e404922d6d87ac550a Mon Sep 17 00:00:00 2001 From: TG Date: Fri, 7 Feb 2025 22:41:03 +0100 Subject: [PATCH 12/12] exit with code 1 --- lib/mocha.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/mocha.js b/lib/mocha.js index 54b82ae8c0..35923ad14a 100644 --- a/lib/mocha.js +++ b/lib/mocha.js @@ -1247,6 +1247,7 @@ Mocha.prototype._runGlobalFixtures = async function _runGlobalFixtures( err: err }); console.error(err); + process.exit(1); } } }