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

WIP: Better errors from t.fail, t.notThrows, t.notThrowsAsync #701

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 47 additions & 10 deletions packages/ses-ava/src/ses-ava-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
import 'ses';
import './types.js';

const { apply } = Reflect;

/**
* Just forwards to global `console.error`.
*
Expand All @@ -29,7 +27,7 @@ const isPromise = maybePromise =>
const logErrorFirst = (func, args, name, logger = defaultLogger) => {
let result;
try {
result = apply(func, undefined, args);
result = func(...args);
Copy link
Member

Choose a reason for hiding this comment

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

What’s the motivation for this simplification, or what was the motive for the complication? (I like the simplification)

} catch (err) {
logger(`THROWN from ${name}:`, err);
throw err;
Expand All @@ -47,6 +45,45 @@ const logErrorFirst = (func, args, name, logger = defaultLogger) => {
}
};

/**
* @param {Assertions} originalT
* @param {Logger} logger
* @returns {Assertions}
*/
const wrapAssertions = (originalT, logger) => {
/**
* Inherits all methods from the originalT that it does not override.
* Those that it does override it wraps behavior around a `super` call,
* and so uses concise method syntax rather than arrow functions.
*
* See TODO comment on cast below to remove the `{unknown}` type declaration.
*
* @type {unknown}
*/
const newT = harden({
__proto__: originalT,
fail(message) {
const err = new Error(message);
logger('FAILED by t.fail', err);
return super.fail(message);
Copy link
Member

Choose a reason for hiding this comment

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

Today I learned that super works without the enclosing class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This actually makes it more E-like, even though E did not provoke the idea. But still, E did help corroborate that it is a good idea.

},
notThrows(originalFn, message) {
const newFn = (...args) =>
logErrorFirst(originalFn, args, 'notThrows', logger);
return super.notThrows(newFn, message);
},
notThrowsAsync(originalFn, message) {
const newFn = (...args) =>
logErrorFirst(originalFn, args, 'notThrowsAsync', logger);
return super.notThrowsAsync(newFn, message);
},
});
// TODO The `{unknown}` above and the cast here seem to be needed
// because TypeScript doesn't understand `__proto__:` in an object
// literal implies inheritance, and thus usually subtyping.
return /** @type {Assertions} */ (newT);
};

const testerMethodsWhitelist = [
'after',
'afterEach',
Expand All @@ -61,16 +98,16 @@ const testerMethodsWhitelist = [

/**
* @param {TesterFunc} testerFunc
* @param {Logger} [logger]
* @param {Logger} logger
* @returns {TesterFunc} Not yet frozen!
*/
const wrapTester = (testerFunc, logger = defaultLogger) => {
const wrapTester = (testerFunc, logger) => {
/** @type {TesterFunc} */
const testerWrapper = (title, implFunc, ...otherArgs) => {
/** @type {ImplFunc} */
const testFuncWrapper = t => {
harden(t);
return logErrorFirst(implFunc, [t, ...otherArgs], 'ava test', logger);
const testFuncWrapper = originalT => {
erights marked this conversation as resolved.
Show resolved Hide resolved
const newT = wrapAssertions(originalT, logger);
return logErrorFirst(implFunc, [newT, ...otherArgs], 'ava test', logger);
};
if (implFunc && implFunc.title) {
testFuncWrapper.title = implFunc.title;
Expand Down Expand Up @@ -106,7 +143,7 @@ const wrapTester = (testerFunc, logger = defaultLogger) => {
* propagating into `rawTest`.
*
* @param {TesterInterface} avaTest
* @param {Logger} [logger]
* @param {Logger=} logger
* @returns {TesterInterface}
*/
const wrapTest = (avaTest, logger = defaultLogger) => {
Expand All @@ -117,7 +154,7 @@ const wrapTest = (avaTest, logger = defaultLogger) => {
/** @type {TesterFunc} */
const testerMethod = (title, implFunc, ...otherArgs) =>
avaTest[methodName](title, implFunc, ...otherArgs);
testerWrapper[methodName] = wrapTester(testerMethod);
testerWrapper[methodName] = wrapTester(testerMethod, logger);
}
}
harden(testerWrapper);
Expand Down
17 changes: 12 additions & 5 deletions packages/ses-ava/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,19 @@
/**
* @callback LogCallError
*
* Calls `thunk()` passing back approximately its outcome, but first
* Calls `func(...args)` passing back approximately its outcome, but first
* logging any erroneous outcome to the `logger`, which defaults to
* `console.log`.
*
* If thunk returns a non-promise, silently return it.
* If thunk throws, log what was thrown and then rethrow it.
* If thunk returns a promise, immediately return a new unresolved promise.
* If func returns a non-promise, silently return it.
* If func throws, log what was thrown and then rethrow it.
* If func returns a promise, immediately return a new unresolved promise.
* If the first promise fulfills, silently fulfill the returned promise even if
* the fulfillment was an error.
* If the first promise rejects, log the rejection reason and then reject the
* returned promise with the same reason.
* The delayed rejection of the returned promise is an observable difference
* from directly calling `thunk()` but will be equivalent enough for most
* from directly calling `func(...args)` but will be equivalent enough for most
* purposes.
*
* TODO This function is useful independent of ava, so consider moving it
Expand All @@ -34,10 +34,17 @@
/**
* Simplified form of ava's types.
* TODO perhaps just import ava's type declarations instead
* See https://github.com/avajs/ava/blob/main/index.d.ts
* TODO reconcile also with types and API defined in avaAssertXS.js
*
* @typedef {Object} Assertions
* @property {(actual: unknown, message?: string) => void} assert
* @property {(message?: string) => void} fail
* @property {(fn: () => any, message?: string) => void} notThrows
* @property {(
* fn: () => PromiseLike<any>,
* message?: string
* ) => Promise<void>} notThrowsAsync
* // TODO is, deepEqual, truthy, falsy, etc...
*/

Expand Down