Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve error handling in global fixtures #5208 #5275

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
29 changes: 25 additions & 4 deletions lib/mocha.js
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Testing] This change is missing test coverage. It's great that existing tests pass, but there should be a test added that shows Mocha acting appropriately when an a global fixture does reject.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TG199 this is still an open piece of feedback, can you add tests please?

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1219,22 +1221,40 @@ 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
* @returns {Promise<object>} context object
*/
Mocha.prototype._runGlobalFixtures = async function _runGlobalFixtures(
fixtureFns = [],
context = {}
context = {},
) {
for await (const fixtureFn of fixtureFns) {
await fixtureFn.call(context);
for (const fixtureFn of fixtureFns) {
try {
await fixtureFn.call(context);
} catch (err) {
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);
process.exit(1);
}
}
}
return context;
};


/**
* Toggle execution of any global setup fixture(s)
*
Expand Down Expand Up @@ -1330,3 +1350,4 @@ Mocha.prototype.hasGlobalTeardownFixtures =
* @param {Array<*>} impls - User-supplied implementations
* @returns {Promise<*>|*}
*/

9 changes: 9 additions & 0 deletions test/integration/fixtures/global-teardown-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use strict'

const { it } = require('../../../lib/mocha');

it('should pass', () => {});

exports.mochaGlobalTeardown = async function () {
throw new Error('Teardown problem')
}
102 changes: 102 additions & 0 deletions test/integration/global-teardown-errors.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
'use strict'

const assert = require('assert');
const path = require('path');
const { spawnSync } = require('child_process');
const fs = require('fs').promises;

describe('Global Teardown Error Handling', function() {
this.timeout(5000);

const setupFile = 'test/fixtures/global-teardown/setup.js';
const testFile = 'test/fixtures/global-teardown/test.js';

before(async function() {
await fs.mkdir(path.dirname(setupFile), { recursive: true });

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');
});
});