Treat multiple calls to done callbacks as errors

This commit is contained in:
Steve Gravrock
2021-09-10 18:07:35 -07:00
parent 15f7013f85
commit d092a59bd1
8 changed files with 144 additions and 160 deletions

View File

@@ -684,7 +684,7 @@ getJasmineRequireObj().Spec = function(j$) {
};
this.expectationResultFactory =
attrs.expectationResultFactory || function() {};
this.deprecated = attrs.deprecated || function() {};
this.onLateError = attrs.onLateError || function() {};
this.queueRunnerFactory = attrs.queueRunnerFactory || function() {};
this.catchingExceptions =
attrs.catchingExceptions ||
@@ -783,15 +783,13 @@ getJasmineRequireObj().Spec = function(j$) {
// Issue a deprecation. Include the context ourselves and pass
// ignoreRunnable: true, since getting here always means that we've already
// moved on and the current runnable isn't the one that caused the problem.
self.deprecated(
"An asynchronous function called its 'done' " +
'callback more than once. This is a bug in the spec, beforeAll, ' +
'beforeEach, afterAll, or afterEach function in question. This will ' +
'be treated as an error in a future version.\n' +
'(in spec: ' +
self.getFullName() +
')',
{ ignoreRunnable: true }
self.onLateError(
new Error(
'An asynchronous spec, beforeEach, or afterEach function called its ' +
"'done' callback more than once.\n(in spec: " +
self.getFullName() +
')'
)
);
},
onComplete: function() {
@@ -1332,6 +1330,18 @@ getJasmineRequireObj().Env = function(j$) {
}
};
function recordLateError(error) {
topSuite.result.failedExpectations.push(
expectationResultFactory({
error,
passed: false,
matcherName: '',
expected: '',
actual: ''
})
);
}
function recordLateExpectation(runable, runableType, result) {
var delayedExpectationResult = {};
Object.keys(result).forEach(function(k) {
@@ -1509,12 +1519,12 @@ getJasmineRequireObj().Env = function(j$) {
};
var topSuite = new j$.Suite({
env: this,
id: getNextSuiteId(),
description: 'Jasmine__TopLevel__Suite',
expectationFactory: expectationFactory,
asyncExpectationFactory: suiteAsyncExpectationFactory,
expectationResultFactory: expectationResultFactory
expectationResultFactory: expectationResultFactory,
onLateError: recordLateError
});
var deprecator = new j$.Deprecator(topSuite);
defaultResourcesForRunnable(topSuite.id);
@@ -1604,7 +1614,7 @@ getJasmineRequireObj().Env = function(j$) {
'specDone'
],
queueRunnerFactory,
self.deprecated
recordLateError
);
/**
@@ -1898,7 +1908,6 @@ getJasmineRequireObj().Env = function(j$) {
var suiteFactory = function(description) {
var suite = new j$.Suite({
env: self,
id: getNextSuiteId(),
description: description,
parentSuite: currentDeclarationSuite,
@@ -1906,7 +1915,8 @@ getJasmineRequireObj().Env = function(j$) {
expectationFactory: expectationFactory,
asyncExpectationFactory: suiteAsyncExpectationFactory,
expectationResultFactory: expectationResultFactory,
throwOnExpectationFailure: config.stopSpecOnExpectationFailure
throwOnExpectationFailure: config.stopSpecOnExpectationFailure,
onLateError: recordLateError
});
return suite;
@@ -2002,7 +2012,7 @@ getJasmineRequireObj().Env = function(j$) {
beforeAndAfterFns: beforeAndAfterFns(suite),
expectationFactory: expectationFactory,
asyncExpectationFactory: specAsyncExpectationFactory,
deprecated: self.deprecated,
onLateError: recordLateError,
resultCallback: specResultCallback,
getSpecName: function(spec) {
return getSpecName(spec, suite);
@@ -7723,7 +7733,7 @@ getJasmineRequireObj().QueueRunner = function(j$) {
};
getJasmineRequireObj().ReportDispatcher = function(j$) {
function ReportDispatcher(methods, queueRunnerFactory, deprecated) {
function ReportDispatcher(methods, queueRunnerFactory, onLateError) {
var dispatchedMethods = methods || [];
for (var i = 0; i < dispatchedMethods.length; i++) {
@@ -7769,11 +7779,11 @@ getJasmineRequireObj().ReportDispatcher = function(j$) {
onComplete: onComplete,
isReporter: true,
onMultipleDone: function() {
deprecated(
"An asynchronous reporter callback called its 'done' callback " +
'more than once. This is a bug in the reporter callback in ' +
'question. This will be treated as an error in a future version.',
{ ignoreRunnable: true }
onLateError(
new Error(
"An asynchronous reporter callback called its 'done' callback " +
'more than once.'
)
);
}
});
@@ -9091,7 +9101,6 @@ getJasmineRequireObj().StackTrace = function(j$) {
getJasmineRequireObj().Suite = function(j$) {
function Suite(attrs) {
this.env = attrs.env;
this.id = attrs.id;
this.parentSuite = attrs.parentSuite;
this.description = attrs.description;
@@ -9099,6 +9108,7 @@ getJasmineRequireObj().Suite = function(j$) {
this.asyncExpectationFactory = attrs.asyncExpectationFactory;
this.expectationResultFactory = attrs.expectationResultFactory;
this.throwOnExpectationFailure = !!attrs.throwOnExpectationFailure;
this.onLateError = attrs.onLateError;
this.beforeFns = [];
this.afterFns = [];
@@ -9258,29 +9268,25 @@ getJasmineRequireObj().Suite = function(j$) {
};
Suite.prototype.onMultipleDone = function() {
var msg;
let msg;
// Issue a deprecation. Include the context ourselves and pass
// ignoreRunnable: true, since getting here always means that we've already
// moved on and the current runnable isn't the one that caused the problem.
if (this.parentSuite) {
msg =
"An asynchronous function called its 'done' callback more than " +
'once. This is a bug in the spec, beforeAll, beforeEach, afterAll, ' +
'or afterEach function in question. This will be treated as an error ' +
'in a future version.\n' +
"An asynchronous beforeAll or afterAll function called its 'done' " +
'callback more than once.\n' +
'(in suite: ' +
this.getFullName() +
')';
} else {
msg =
'A top-level beforeAll or afterAll function called its ' +
"'done' callback more than once. This is a bug in the beforeAll " +
'or afterAll function in question. This will be treated as an ' +
'error in a future version.';
"'done' callback more than once.";
}
this.env.deprecated(msg, { ignoreRunnable: true });
this.onLateError(new Error(msg));
};
Suite.prototype.addExpectationResult = function() {

View File

@@ -513,11 +513,11 @@ describe('Spec', function() {
expect(resultCallback.calls.first().args[0].failedExpectations).toEqual([]);
});
it('passes an onMultipleDone that logs a deprecation', function() {
it('treats multiple done calls as late errors', function() {
var queueRunnerFactory = jasmine.createSpy('queueRunnerFactory'),
deprecated = jasmine.createSpy('depredated'),
onLateError = jasmine.createSpy('onLateError'),
spec = new jasmineUnderTest.Spec({
deprecated: deprecated,
onLateError: onLateError,
queueableFn: { fn: function() {} },
queueRunnerFactory: queueRunnerFactory,
getSpecName: function() {
@@ -530,13 +530,11 @@ describe('Spec', function() {
expect(queueRunnerFactory).toHaveBeenCalled();
queueRunnerFactory.calls.argsFor(0)[0].onMultipleDone();
expect(deprecated).toHaveBeenCalledWith(
"An asynchronous function called its 'done' " +
'callback more than once. This is a bug in the spec, beforeAll, ' +
'beforeEach, afterAll, or afterEach function in question. This will ' +
'be treated as an error in a future version.\n' +
'(in spec: a spec)',
{ ignoreRunnable: true }
expect(onLateError).toHaveBeenCalledTimes(1);
expect(onLateError.calls.argsFor(0)[0]).toBeInstanceOf(Error);
expect(onLateError.calls.argsFor(0)[0].message).toEqual(
'An asynchronous spec, beforeEach, or afterEach function called its ' +
"'done' callback more than once.\n(in spec: a spec)"
);
});
});

View File

@@ -144,25 +144,27 @@ describe('Suite', function() {
});
describe('#onMultipleDone', function() {
it('logs a special deprecation when it is the top suite', function() {
var env = jasmine.createSpyObj('env', ['deprecated']);
var suite = new jasmineUnderTest.Suite({ env: env, parentSuite: null });
it('reports a special error when it is the top suite', function() {
const onLateError = jasmine.createSpy('onLateError');
const suite = new jasmineUnderTest.Suite({
onLateError,
parentSuite: null
});
suite.onMultipleDone();
expect(env.deprecated).toHaveBeenCalledWith(
expect(onLateError).toHaveBeenCalledTimes(1);
expect(onLateError.calls.argsFor(0)[0]).toBeInstanceOf(Error);
expect(onLateError.calls.argsFor(0)[0].message).toEqual(
'A top-level beforeAll or afterAll function called its ' +
"'done' callback more than once. This is a bug in the beforeAll " +
'or afterAll function in question. This will be treated as an ' +
'error in a future version.',
{ ignoreRunnable: true }
"'done' callback more than once."
);
});
it('logs a deprecation including the suite name when it is a normal suite', function() {
var env = jasmine.createSpyObj('env', ['deprecated']);
it('reports an error including the suite name when it is a normal suite', function() {
const onLateError = jasmine.createSpy('onLateError');
var suite = new jasmineUnderTest.Suite({
env: env,
onLateError,
description: 'the suite',
parentSuite: {
description: 'the parent suite',
@@ -172,13 +174,11 @@ describe('Suite', function() {
suite.onMultipleDone();
expect(env.deprecated).toHaveBeenCalledWith(
"An asynchronous function called its 'done' callback more than " +
'once. This is a bug in the spec, beforeAll, beforeEach, afterAll, ' +
'or afterEach function in question. This will be treated as an error ' +
'in a future version.\n' +
'(in suite: the parent suite the suite)',
{ ignoreRunnable: true }
expect(onLateError).toHaveBeenCalledTimes(1);
expect(onLateError.calls.argsFor(0)[0]).toBeInstanceOf(Error);
expect(onLateError.calls.argsFor(0)[0].message).toEqual(
"An asynchronous beforeAll or afterAll function called its 'done' " +
'callback more than once.\n(in suite: the parent suite the suite)'
);
});
});

View File

@@ -509,17 +509,13 @@ describe('Env integration', function() {
env.execute(null, assertions);
});
it('deprecates multiple calls to done in the top suite', function(done) {
it('reports multiple calls to done in the top suite as errors', function(done) {
var reporter = jasmine.createSpyObj('fakeReporter', ['jasmineDone']);
var message =
'A top-level beforeAll or afterAll function called its ' +
"'done' callback more than once. This is a bug in the beforeAll " +
'or afterAll function in question. This will be treated as an ' +
'error in a future version.';
"'done' callback more than once.";
spyOn(console, 'error');
env.addReporter(reporter);
env.configure({ verboseDeprecations: true });
env.beforeAll(function(innerDone) {
innerDone();
innerDone();
@@ -531,31 +527,27 @@ describe('Env integration', function() {
});
env.execute(null, function() {
var warnings;
expect(reporter.jasmineDone).toHaveBeenCalled();
warnings = reporter.jasmineDone.calls.argsFor(0)[0].deprecationWarnings;
expect(warnings.length).toEqual(2);
expect(warnings[0])
const errors = reporter.jasmineDone.calls.argsFor(0)[0]
.failedExpectations;
expect(errors.length).toEqual(2);
expect(errors[0].message)
.withContext('top beforeAll')
.toEqual(jasmine.objectContaining({ message: message }));
expect(warnings[1])
.toContain(message);
expect(errors[1].message)
.withContext('top afterAll')
.toEqual(jasmine.objectContaining({ message: message }));
.toContain(message);
done();
});
});
it('deprecates multiple calls to done in a non-top suite', function(done) {
it('reports multiple calls to done in a non-top suite as errors', function(done) {
var reporter = jasmine.createSpyObj('fakeReporter', ['jasmineDone']);
var message =
"An asynchronous function called its 'done' " +
'callback more than once. This is a bug in the spec, beforeAll, ' +
'beforeEach, afterAll, or afterEach function in question. This will ' +
'be treated as an error in a future version.';
"An asynchronous beforeAll or afterAll function called its 'done' " +
'callback more than once.\n(in suite: a suite)';
spyOn(console, 'error');
env.addReporter(reporter);
env.configure({ verboseDeprecations: true });
env.describe('a suite', function() {
env.beforeAll(function(innerDone) {
innerDone();
@@ -569,40 +561,27 @@ describe('Env integration', function() {
});
env.execute(null, function() {
var warnings;
expect(reporter.jasmineDone).toHaveBeenCalled();
warnings = reporter.jasmineDone.calls.argsFor(0)[0].deprecationWarnings;
expect(warnings.length).toEqual(2);
expect(warnings[0])
const errors = reporter.jasmineDone.calls.argsFor(0)[0]
.failedExpectations;
expect(errors.length).toEqual(2);
expect(errors[0].message)
.withContext('suite beforeAll')
.toEqual(
jasmine.objectContaining({
message: message + '\n(in suite: a suite)'
})
);
expect(warnings[1])
.toContain(message);
expect(errors[1].message)
.withContext('suite afterAll')
.toEqual(
jasmine.objectContaining({
message: message + '\n(in suite: a suite)'
})
);
.toContain(message);
done();
});
});
it('deprecates multiple calls to done in a spec', function(done) {
it('reports multiple calls to done in a spec as errors', function(done) {
var reporter = jasmine.createSpyObj('fakeReporter', ['jasmineDone']);
var message =
"An asynchronous function called its 'done' " +
'callback more than once. This is a bug in the spec, beforeAll, ' +
'beforeEach, afterAll, or afterEach function in question. This will ' +
'be treated as an error in a future version.\n' +
'(in spec: a suite a spec)';
'An asynchronous spec, beforeEach, or afterEach function called its ' +
"'done' callback more than once.\n(in spec: a suite a spec)";
spyOn(console, 'error');
env.addReporter(reporter);
env.configure({ verboseDeprecations: true });
env.describe('a suite', function() {
env.beforeEach(function(innerDone) {
innerDone();
@@ -619,30 +598,27 @@ describe('Env integration', function() {
});
env.execute(null, function() {
var warnings;
expect(reporter.jasmineDone).toHaveBeenCalled();
warnings = reporter.jasmineDone.calls.argsFor(0)[0].deprecationWarnings;
expect(warnings.length).toEqual(3);
expect(warnings[0])
.withContext('warning caused by beforeEach')
.toEqual(jasmine.objectContaining({ message: message }));
expect(warnings[1])
.withContext('warning caused by it')
.toEqual(jasmine.objectContaining({ message: message }));
expect(warnings[2])
.withContext('warning caused by afterEach')
.toEqual(jasmine.objectContaining({ message: message }));
const errors = reporter.jasmineDone.calls.argsFor(0)[0]
.failedExpectations;
expect(errors.length).toEqual(3);
expect(errors[0].message)
.withContext('error caused by beforeEach')
.toContain(message);
expect(errors[1].message)
.withContext('error caused by it')
.toContain(message);
expect(errors[2].message)
.withContext('error caused by afterEach')
.toContain(message);
done();
});
});
it('deprecates multiple calls to done in reporters', function(done) {
it('reports multiple calls to done in reporters as errors', function(done) {
var message =
"An asynchronous reporter callback called its 'done' callback more " +
'than once. This is a bug in the reporter callback in question. This ' +
'will be treated as an error in a future version.\nNote: This message ' +
'will be shown only once. Set config.verboseDeprecations to true to ' +
'see every occurrence.';
'than once.';
var reporter = jasmine.createSpyObj('fakeReport', ['jasmineDone']);
reporter.specDone = function(result, done) {
done();
@@ -652,19 +628,17 @@ describe('Env integration', function() {
env.it('a spec', function() {});
spyOn(console, 'error');
env.execute(null, function() {
expect(reporter.jasmineDone).toHaveBeenCalled();
warnings = reporter.jasmineDone.calls.argsFor(0)[0].deprecationWarnings;
expect(warnings.length).toEqual(1);
expect(warnings[0]).toEqual(
jasmine.objectContaining({ message: message })
);
const errors = reporter.jasmineDone.calls.argsFor(0)[0]
.failedExpectations;
expect(errors.length).toEqual(1);
expect(errors[0].message).toContain(message);
done();
});
});
it('does not deprecate a call to done that comes after a timeout', function(done) {
it('does not report an error for a call to done that comes after a timeout', function(done) {
var reporter = jasmine.createSpyObj('fakeReporter', ['jasmineDone']),
firstSpecDone;
@@ -687,7 +661,7 @@ describe('Env integration', function() {
env.execute(null, function() {
expect(reporter.jasmineDone).toHaveBeenCalledWith(
jasmine.objectContaining({
deprecationWarnings: []
failedExpectations: []
})
);
done();

View File

@@ -338,6 +338,18 @@ getJasmineRequireObj().Env = function(j$) {
}
};
function recordLateError(error) {
topSuite.result.failedExpectations.push(
expectationResultFactory({
error,
passed: false,
matcherName: '',
expected: '',
actual: ''
})
);
}
function recordLateExpectation(runable, runableType, result) {
var delayedExpectationResult = {};
Object.keys(result).forEach(function(k) {
@@ -515,12 +527,12 @@ getJasmineRequireObj().Env = function(j$) {
};
var topSuite = new j$.Suite({
env: this,
id: getNextSuiteId(),
description: 'Jasmine__TopLevel__Suite',
expectationFactory: expectationFactory,
asyncExpectationFactory: suiteAsyncExpectationFactory,
expectationResultFactory: expectationResultFactory
expectationResultFactory: expectationResultFactory,
onLateError: recordLateError
});
var deprecator = new j$.Deprecator(topSuite);
defaultResourcesForRunnable(topSuite.id);
@@ -610,7 +622,7 @@ getJasmineRequireObj().Env = function(j$) {
'specDone'
],
queueRunnerFactory,
self.deprecated
recordLateError
);
/**
@@ -904,7 +916,6 @@ getJasmineRequireObj().Env = function(j$) {
var suiteFactory = function(description) {
var suite = new j$.Suite({
env: self,
id: getNextSuiteId(),
description: description,
parentSuite: currentDeclarationSuite,
@@ -912,7 +923,8 @@ getJasmineRequireObj().Env = function(j$) {
expectationFactory: expectationFactory,
asyncExpectationFactory: suiteAsyncExpectationFactory,
expectationResultFactory: expectationResultFactory,
throwOnExpectationFailure: config.stopSpecOnExpectationFailure
throwOnExpectationFailure: config.stopSpecOnExpectationFailure,
onLateError: recordLateError
});
return suite;
@@ -1008,7 +1020,7 @@ getJasmineRequireObj().Env = function(j$) {
beforeAndAfterFns: beforeAndAfterFns(suite),
expectationFactory: expectationFactory,
asyncExpectationFactory: specAsyncExpectationFactory,
deprecated: self.deprecated,
onLateError: recordLateError,
resultCallback: specResultCallback,
getSpecName: function(spec) {
return getSpecName(spec, suite);

View File

@@ -1,5 +1,5 @@
getJasmineRequireObj().ReportDispatcher = function(j$) {
function ReportDispatcher(methods, queueRunnerFactory, deprecated) {
function ReportDispatcher(methods, queueRunnerFactory, onLateError) {
var dispatchedMethods = methods || [];
for (var i = 0; i < dispatchedMethods.length; i++) {
@@ -45,11 +45,11 @@ getJasmineRequireObj().ReportDispatcher = function(j$) {
onComplete: onComplete,
isReporter: true,
onMultipleDone: function() {
deprecated(
"An asynchronous reporter callback called its 'done' callback " +
'more than once. This is a bug in the reporter callback in ' +
'question. This will be treated as an error in a future version.',
{ ignoreRunnable: true }
onLateError(
new Error(
"An asynchronous reporter callback called its 'done' callback " +
'more than once.'
)
);
}
});

View File

@@ -24,7 +24,7 @@ getJasmineRequireObj().Spec = function(j$) {
};
this.expectationResultFactory =
attrs.expectationResultFactory || function() {};
this.deprecated = attrs.deprecated || function() {};
this.onLateError = attrs.onLateError || function() {};
this.queueRunnerFactory = attrs.queueRunnerFactory || function() {};
this.catchingExceptions =
attrs.catchingExceptions ||
@@ -123,15 +123,13 @@ getJasmineRequireObj().Spec = function(j$) {
// Issue a deprecation. Include the context ourselves and pass
// ignoreRunnable: true, since getting here always means that we've already
// moved on and the current runnable isn't the one that caused the problem.
self.deprecated(
"An asynchronous function called its 'done' " +
'callback more than once. This is a bug in the spec, beforeAll, ' +
'beforeEach, afterAll, or afterEach function in question. This will ' +
'be treated as an error in a future version.\n' +
'(in spec: ' +
self.getFullName() +
')',
{ ignoreRunnable: true }
self.onLateError(
new Error(
'An asynchronous spec, beforeEach, or afterEach function called its ' +
"'done' callback more than once.\n(in spec: " +
self.getFullName() +
')'
)
);
},
onComplete: function() {

View File

@@ -1,6 +1,5 @@
getJasmineRequireObj().Suite = function(j$) {
function Suite(attrs) {
this.env = attrs.env;
this.id = attrs.id;
this.parentSuite = attrs.parentSuite;
this.description = attrs.description;
@@ -8,6 +7,7 @@ getJasmineRequireObj().Suite = function(j$) {
this.asyncExpectationFactory = attrs.asyncExpectationFactory;
this.expectationResultFactory = attrs.expectationResultFactory;
this.throwOnExpectationFailure = !!attrs.throwOnExpectationFailure;
this.onLateError = attrs.onLateError;
this.beforeFns = [];
this.afterFns = [];
@@ -167,29 +167,25 @@ getJasmineRequireObj().Suite = function(j$) {
};
Suite.prototype.onMultipleDone = function() {
var msg;
let msg;
// Issue a deprecation. Include the context ourselves and pass
// ignoreRunnable: true, since getting here always means that we've already
// moved on and the current runnable isn't the one that caused the problem.
if (this.parentSuite) {
msg =
"An asynchronous function called its 'done' callback more than " +
'once. This is a bug in the spec, beforeAll, beforeEach, afterAll, ' +
'or afterEach function in question. This will be treated as an error ' +
'in a future version.\n' +
"An asynchronous beforeAll or afterAll function called its 'done' " +
'callback more than once.\n' +
'(in suite: ' +
this.getFullName() +
')';
} else {
msg =
'A top-level beforeAll or afterAll function called its ' +
"'done' callback more than once. This is a bug in the beforeAll " +
'or afterAll function in question. This will be treated as an ' +
'error in a future version.';
"'done' callback more than once.";
}
this.env.deprecated(msg, { ignoreRunnable: true });
this.onLateError(new Error(msg));
};
Suite.prototype.addExpectationResult = function() {