diff --git a/packages/dev-utils/src/chai_setup.ts b/packages/dev-utils/src/chai_setup.ts index 727e4b839e..099869d957 100644 --- a/packages/dev-utils/src/chai_setup.ts +++ b/packages/dev-utils/src/chai_setup.ts @@ -10,31 +10,31 @@ export const chaiSetup = { // Order matters. chai.use(ChaiBigNumber()); chai.use(chaiAsPromised); - chai.use(richRevertExtension); + chai.use(revertErrorHelper); chai.use(dirtyChai); }, }; // tslint:disable: only-arrow-functions prefer-conditional-expression -type ChaiPromiseHandler = (x: any) => Promise; -type ChaiAssertHandler = (x: any) => void; +type ChaiPromiseHandler = (x: any, ...rest: any[]) => Promise; +type ChaiAssertHandler = (x: any, ...rest: any[]) => void; interface Chai { - Assertion: any; + Assertion: ChaiAssertionType; } -interface ChaiUtils { - flag: (assertion: any, name: string, value?: any) => any; - overwriteMethod: (ctx: any, name: string, _super: (expected: any) => any) => void; -} - -interface ChaiExtension { +interface ChaiAssertionInstance { assert: ChaiAssert; _obj: any; __flags: any; } +interface ChaiAssertionType { + overwriteMethod: (name: string, _super: (expected: any) => any) => void; + new (): ChaiAssertionInstance; +} + type ChaiAssert = ( condition: boolean, failMessage?: string, @@ -43,71 +43,77 @@ type ChaiAssert = ( actual?: any, ) => void; -function richRevertExtension(_chai: Chai, utils: ChaiUtils): void { - utils.overwriteMethod(_chai.Assertion.prototype, 'rejectedWith', function( - _super: ChaiPromiseHandler, - ): ChaiPromiseHandler { - return async function(this: ChaiExtension, expected: any): Promise { +function revertErrorHelper(_chai: Chai): void { + const proto = _chai.Assertion; + proto.overwriteMethod('revertWith', function(_super: ChaiPromiseHandler): ChaiPromiseHandler { + return async function(this: ChaiAssertionInstance, expected: any, ...rest: any[]): Promise { const maybePromise = this._obj; // Make sure we're working with a promise. - new _chai.Assertion().assert(maybePromise instanceof Promise, `Expected ${maybePromise} to be a promise`); + chaiAssert(_chai, maybePromise instanceof Promise, `Expected ${maybePromise} to be a promise`); // Wait for the promise to reject. - let err: any; + let resolveValue; + let rejectValue: any; let didReject = false; try { - await maybePromise; - } catch (_err) { - err = _err; + resolveValue = await maybePromise; + } catch (err) { + rejectValue = err; didReject = true; } if (!didReject) { - new _chai.Assertion().assert(false, `Expected promise to reject`); + chaiFail(_chai, `Expected promise to reject but instead resolved with: ${resolveValue}`); + } + if (!compareRevertErrors.call(this, _chai, rejectValue, expected)) { + // Wasn't handled by the comparison function so call the previous handler. + _super.call(this, expected, ...rest); } - return compareRichRevertReasons.call(this, _chai, _super, err, expected); }; }); - utils.overwriteMethod(_chai.Assertion.prototype, 'become', function( - _super: ChaiPromiseHandler, - ): ChaiPromiseHandler { - return async function(this: ChaiExtension, expected: any): Promise { + proto.overwriteMethod('become', function(_super: ChaiPromiseHandler): ChaiPromiseHandler { + return async function(this: ChaiAssertionInstance, expected: any, ...rest: any[]): Promise { const maybePromise = this._obj; // Make sure we're working with a promise. - new _chai.Assertion().assert(maybePromise instanceof Promise, `Expected ${maybePromise} to be a promise`); + chaiAssert(_chai, maybePromise instanceof Promise, `Expected ${maybePromise} to be a promise`); // Wait for the promise to resolve. - return compareRichRevertReasons.call(this, _chai, _super, await maybePromise, expected); + if (!compareRevertErrors.call(this, _chai, await maybePromise, expected)) { + // Wasn't handled by the comparison function so call the previous handler. + _super.call(this, expected, ...rest); + } }; }); - utils.overwriteMethod(_chai.Assertion.prototype, 'equal', function(_super: ChaiAssertHandler): ChaiAssertHandler { - return function(this: ChaiExtension, expected: any): void { - compareRichRevertReasons.call(this, _chai, _super, this._obj, expected); - }; - }); - utils.overwriteMethod(_chai.Assertion.prototype, 'eql', function(_super: ChaiAssertHandler): ChaiAssertHandler { - return function(this: ChaiExtension, expected: any): void { - compareRichRevertReasons.call(this, _chai, _super, this._obj, expected); + proto.overwriteMethod('equal', function(_super: ChaiAssertHandler): ChaiAssertHandler { + return function(this: ChaiAssertionInstance, expected: any, ...rest: any[]): void { + if (!compareRevertErrors.call(this, _chai, this._obj, expected)) { + // Wasn't handled by the comparison function so call the previous handler. + _super.call(this, expected, ...rest); + } }; }); } -function compareRichRevertReasons( - this: ChaiExtension, - _chai: Chai, - _super: ChaiAssertHandler, - _actual: any, - _expected: any, -): void { +/** + * Compare two values as compatible RevertError types. + * @return `true` if the comparison was fully evaluated. `false` indicates that + * it should be deferred to another handler. + */ +function compareRevertErrors(this: ChaiAssertionInstance, _chai: Chai, _actual: any, _expected: any): boolean { let actual = _actual; let expected = _expected; // If either subject is a RevertError, try to coerce the other into the same. + // Some of this is for convenience, some is for backwards-compatibility. + // TODO: Remove coercion of `actual` when all contracts and tests are upgraded + // to explicitly use RevertErrors. if (expected instanceof RevertError || actual instanceof RevertError) { // `actual` can be a RevertError, string, or an Error type. if (!(actual instanceof RevertError)) { if (typeof actual === 'string') { actual = new StringRevertError(actual); } else if (actual instanceof Error) { + // `BaseContract` will throw a plain `Error` type for `StringRevertErrors` + // for backwards compatibility. So coerce it into a StringRevertError. actual = new StringRevertError(actual.message); } else { - new _chai.Assertion().assert(false, `Result is not of type RevertError: ${actual}`); + chaiAssert(_chai, false, `Result is not of type RevertError: ${actual}`); } } // `expected` can be a RevertError or string. @@ -124,7 +130,18 @@ function compareRichRevertReasons( expected, actual, ); - return; + // Return true to signal we handled it. + return true; } - _super.call(this, _expected); + return false; +} + +function chaiAssert(_chai: Chai, condition: boolean, failMessage?: string, expected?: any, actual?: any): void { + const assert = new _chai.Assertion(); + assert.assert(condition, failMessage, undefined, expected, actual); +} + +function chaiFail(_chai: Chai, failMessage?: string, expected?: any, actual?: any): void { + const assert = new _chai.Assertion(); + assert.assert(false, failMessage, undefined, expected, actual); } diff --git a/packages/dev-utils/src/globals.d.ts b/packages/dev-utils/src/globals.d.ts index 94e63a32de..8fefe122e0 100644 --- a/packages/dev-utils/src/globals.d.ts +++ b/packages/dev-utils/src/globals.d.ts @@ -4,3 +4,13 @@ declare module '*.json' { export default json; /* tslint:enable */ } + +declare module 'chai' { + global { + export namespace Chai { + export interface Assertion { + revertWith: (expected: string | RevertError) => Promise; + } + } + } +} diff --git a/packages/dev-utils/test/chai_test.ts b/packages/dev-utils/test/chai_test.ts index 694c48e1c0..3258c27ad0 100644 --- a/packages/dev-utils/test/chai_test.ts +++ b/packages/dev-utils/test/chai_test.ts @@ -67,15 +67,15 @@ describe('Chai tests', () => { expect(error).is.not.equal(revert); }); }); - describe('#rejectedWith', () => { - it('should equate a promise that rejects to an identical RevertErrors', async () => { + describe('#revertWith', () => { + it('should equate a promise that rejects to identical RevertErrors', async () => { const message = 'foo'; const revert1 = new StringRevertError(message); const revert2 = new StringRevertError(message); const promise = (async () => { throw revert1; })(); - return expect(promise).to.be.rejectedWith(revert2); + return expect(promise).to.revertWith(revert2); }); it('should not equate a promise that rejects to a StringRevertError with a different messages', async () => { const revert1 = new StringRevertError('foo1'); @@ -83,7 +83,7 @@ describe('Chai tests', () => { const promise = (async () => { throw revert1; })(); - return expect(promise).to.not.be.rejectedWith(revert2); + return expect(promise).to.not.revertWith(revert2); }); it('should not equate a promise that rejects to different RevertError types', async () => { const message = 'foo'; @@ -92,7 +92,7 @@ describe('Chai tests', () => { const promise = (async () => { throw revert1; })(); - return expect(promise).to.not.be.rejectedWith(revert2); + return expect(promise).to.not.revertWith(revert2); }); }); describe('#become', () => { @@ -117,5 +117,32 @@ describe('Chai tests', () => { return expect(promise).to.not.become(revert2); }); }); + // TODO: Remove these tests when we no longer coerce `Error` types to `StringRevertError` types + // for backwards compatibility. + describe('#rejectedWith (backwards compatibility)', () => { + it('should equate a promise that rejects with an Error to a string of the same message', async () => { + const message = 'foo'; + const revert1 = new Error(message); + const promise = (async () => { + throw revert1; + })(); + return expect(promise).to.rejectedWith(message); + }); + it('should equate a promise that rejects with an StringRevertErrors to a string of the same message', async () => { + const message = 'foo'; + const revert = new StringRevertError(message); + const promise = (async () => { + throw revert; + })(); + return expect(promise).to.rejectedWith(message); + }); + it('should not equate a promise that rejects with an StringRevertErrors to a string with different messages', async () => { + const revert = new StringRevertError('foo1'); + const promise = (async () => { + throw revert; + })(); + return expect(promise).to.not.be.rejectedWith('foo2'); + }); + }); }); });