diff options
author | Fabio Berger <me@fabioberger.com> | 2018-08-28 23:04:02 +0800 |
---|---|---|
committer | Fabio Berger <me@fabioberger.com> | 2018-08-28 23:04:02 +0800 |
commit | 4a524a5f275b80d2e5122ee8a055cf4cc737e32c (patch) | |
tree | 6df0c8aab5f8c28145628a98b50909b9cbab0395 /packages | |
parent | ee1e50a7225882bb0c97408100c85a199611fadb (diff) | |
parent | 9fd46c790097a64c29df816abf83a05cb6acccee (diff) | |
download | dexon-sol-tools-4a524a5f275b80d2e5122ee8a055cf4cc737e32c.tar dexon-sol-tools-4a524a5f275b80d2e5122ee8a055cf4cc737e32c.tar.gz dexon-sol-tools-4a524a5f275b80d2e5122ee8a055cf4cc737e32c.tar.bz2 dexon-sol-tools-4a524a5f275b80d2e5122ee8a055cf4cc737e32c.tar.lz dexon-sol-tools-4a524a5f275b80d2e5122ee8a055cf4cc737e32c.tar.xz dexon-sol-tools-4a524a5f275b80d2e5122ee8a055cf4cc737e32c.tar.zst dexon-sol-tools-4a524a5f275b80d2e5122ee8a055cf4cc737e32c.zip |
Merge branch 'dev-dropdown' into doc-overview-page
* dev-dropdown:
Update to latest react-shared
Use translation helper
Use generatic ObjectMap type
fix(contracts): Catch cases where the actual error differs from the expected error (#1032)
Add clarifying comments
Remove redundant mstores from fillOrderNoThrow
fix(contracts): Use correct error message for division by zero
Diffstat (limited to 'packages')
7 files changed, 121 insertions, 49 deletions
diff --git a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol index a7ff400b9..9e816716c 100644 --- a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol +++ b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol @@ -69,20 +69,14 @@ contract MixinExchangeWrapper is fillOrderCalldata, // write output over input 128 // output size is 128 bytes ) - switch success - case 0 { - mstore(fillResults, 0) - mstore(add(fillResults, 32), 0) - mstore(add(fillResults, 64), 0) - mstore(add(fillResults, 96), 0) - } - case 1 { + if success { mstore(fillResults, mload(fillOrderCalldata)) mstore(add(fillResults, 32), mload(add(fillOrderCalldata, 32))) mstore(add(fillResults, 64), mload(add(fillOrderCalldata, 64))) mstore(add(fillResults, 96), mload(add(fillOrderCalldata, 96))) } } + // fillResults values will be 0 by default if call was unsuccessful return fillResults; } diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol index 39fa724cc..a5459a21e 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol @@ -96,6 +96,7 @@ contract MixinWrapperFunctions is mstore(add(fillResults, 96), mload(add(fillOrderCalldata, 96))) } } + // fillResults values will be 0 by default if call was unsuccessful return fillResults; } diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts index 2521665c2..de381fca3 100644 --- a/packages/contracts/test/exchange/internal.ts +++ b/packages/contracts/test/exchange/internal.ts @@ -67,9 +67,7 @@ describe('Exchange core internal functions', () => { overflowErrorForSendTransaction = new Error( await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.Uint256Overflow), ); - divisionByZeroErrorForCall = new Error( - await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.DivisionByZero), - ); + divisionByZeroErrorForCall = new Error(RevertReason.DivisionByZero); invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync()); }); // Note(albrow): Don't forget to add beforeEach and afterEach calls to reset diff --git a/packages/contracts/test/utils/test_with_reference.ts b/packages/contracts/test/utils/test_with_reference.ts index 599b1eed4..b80be4a6c 100644 --- a/packages/contracts/test/utils/test_with_reference.ts +++ b/packages/contracts/test/utils/test_with_reference.ts @@ -6,6 +6,34 @@ import { chaiSetup } from './chai_setup'; chaiSetup.configure(); const expect = chai.expect; +class Value<T> { + public value: T; + constructor(value: T) { + this.value = value; + } +} + +// tslint:disable-next-line: max-classes-per-file +class ErrorMessage { + public error: string; + constructor(message: string) { + this.error = message; + } +} + +type PromiseResult<T> = Value<T> | ErrorMessage; + +// TODO(albrow): This seems like a generic utility function that could exist in +// lodash. We should replace it by a library implementation, or move it to our +// own. +async function evaluatePromise<T>(promise: Promise<T>): Promise<PromiseResult<T>> { + try { + return new Value<T>(await promise); + } catch (e) { + return new ErrorMessage(e.message); + } +} + export async function testWithReferenceFuncAsync<P0, R>( referenceFunc: (p0: P0) => Promise<R>, testFunc: (p0: P0) => Promise<R>, @@ -64,39 +92,31 @@ export async function testWithReferenceFuncAsync( testFuncAsync: (...args: any[]) => Promise<any>, values: any[], ): Promise<void> { - let expectedResult: any; - let expectedErr: string | undefined; - try { - expectedResult = await referenceFuncAsync(...values); - } catch (e) { - expectedErr = e.message; - } - let actualResult: any | undefined; - try { - actualResult = await testFuncAsync(...values); - if (!_.isUndefined(expectedErr)) { + // Measure correct behaviour + const expected = await evaluatePromise(referenceFuncAsync(...values)); + + // Measure actual behaviour + const actual = await evaluatePromise(testFuncAsync(...values)); + + // Compare behaviour + if (expected instanceof ErrorMessage) { + // If we expected an error, check if the actual error message contains the + // expected error message. + if (!(actual instanceof ErrorMessage)) { throw new Error( - `Expected error containing ${expectedErr} but got no error\n\tTest case: ${_getTestCaseString( + `Expected error containing ${expected.error} but got no error\n\tTest case: ${_getTestCaseString( referenceFuncAsync, values, )}`, ); } - } catch (e) { - if (_.isUndefined(expectedErr)) { - throw new Error(`${e.message}\n\tTest case: ${_getTestCaseString(referenceFuncAsync, values)}`); - } else { - expect(e.message).to.contain( - expectedErr, - `${e.message}\n\tTest case: ${_getTestCaseString(referenceFuncAsync, values)}`, - ); - } - } - if (!_.isUndefined(actualResult) && !_.isUndefined(expectedResult)) { - expect(actualResult).to.deep.equal( - expectedResult, - `Test case: ${_getTestCaseString(referenceFuncAsync, values)}`, + expect(actual.error).to.contain( + expected.error, + `${actual.error}\n\tTest case: ${_getTestCaseString(referenceFuncAsync, values)}`, ); + } else { + // If we do not expect an error, compare actual and expected directly. + expect(actual).to.deep.equal(expected, `Test case ${_getTestCaseString(referenceFuncAsync, values)}`); } } diff --git a/packages/contracts/test/utils_test/test_with_reference.ts b/packages/contracts/test/utils_test/test_with_reference.ts new file mode 100644 index 000000000..8d633cd1f --- /dev/null +++ b/packages/contracts/test/utils_test/test_with_reference.ts @@ -0,0 +1,63 @@ +import * as chai from 'chai'; + +import { chaiSetup } from '../utils/chai_setup'; +import { testWithReferenceFuncAsync } from '../utils/test_with_reference'; + +chaiSetup.configure(); +const expect = chai.expect; + +async function divAsync(x: number, y: number): Promise<number> { + if (y === 0) { + throw new Error('MathError: divide by zero'); + } + return x / y; +} + +// returns an async function that always returns the given value. +function alwaysValueFunc(value: number): (x: number, y: number) => Promise<number> { + return async (x: number, y: number) => value; +} + +// returns an async function which always throws/rejects with the given error +// message. +function alwaysFailFunc(errMessage: string): (x: number, y: number) => Promise<number> { + return async (x: number, y: number) => { + throw new Error(errMessage); + }; +} + +describe('testWithReferenceFuncAsync', () => { + it('passes when both succeed and actual === expected', async () => { + await testWithReferenceFuncAsync(alwaysValueFunc(0.5), divAsync, [1, 2]); + }); + + it('passes when both fail and actual error contains expected error', async () => { + await testWithReferenceFuncAsync(alwaysFailFunc('divide by zero'), divAsync, [1, 0]); + }); + + it('fails when both succeed and actual !== expected', async () => { + expect(testWithReferenceFuncAsync(alwaysValueFunc(3), divAsync, [1, 2])).to.be.rejectedWith( + 'Test case {"x":1,"y":2}: expected { value: 0.5 } to deeply equal { value: 3 }', + ); + }); + + it('fails when both fail and actual error does not contain expected error', async () => { + expect( + testWithReferenceFuncAsync(alwaysFailFunc('Unexpected math error'), divAsync, [1, 0]), + ).to.be.rejectedWith( + 'MathError: divide by zero\n\tTest case: {"x":1,"y":0}: expected \'MathError: divide by zero\' to include \'Unexpected math error\'', + ); + }); + + it('fails when referenceFunc succeeds and testFunc fails', async () => { + expect(testWithReferenceFuncAsync(alwaysValueFunc(0), divAsync, [1, 0])).to.be.rejectedWith( + 'Test case {"x":1,"y":0}: expected { error: \'MathError: divide by zero\' } to deeply equal { value: 0 }', + ); + }); + + it('fails when referenceFunc fails and testFunc succeeds', async () => { + expect(testWithReferenceFuncAsync(alwaysFailFunc('divide by zero'), divAsync, [1, 2])).to.be.rejectedWith( + 'Expected error containing divide by zero but got no error\n\tTest case: {"x":1,"y":2}', + ); + }); +}); diff --git a/packages/website/package.json b/packages/website/package.json index 9b6a0ba36..0a223bcb2 100644 --- a/packages/website/package.json +++ b/packages/website/package.json @@ -21,7 +21,7 @@ "@0xproject/contract-wrappers": "^0.0.5", "@0xproject/order-utils": "^0.0.9", "@0xproject/react-docs": "^1.0.7", - "@0xproject/react-shared": "^0.2.3", + "@0xproject/react-shared": "^1.0.8", "@0xproject/subproviders": "^2.0.1", "@0xproject/types": "^0.8.1", "@0xproject/typescript-typings": "^0.4.3", diff --git a/packages/website/ts/components/dropdowns/developers_drop_down.tsx b/packages/website/ts/components/dropdowns/developers_drop_down.tsx index bf89775f7..7a0aea182 100644 --- a/packages/website/ts/components/dropdowns/developers_drop_down.tsx +++ b/packages/website/ts/components/dropdowns/developers_drop_down.tsx @@ -3,20 +3,16 @@ import * as _ from 'lodash'; import * as React from 'react'; import { Link } from 'react-router-dom'; import { DropDown } from 'ts/components/ui/drop_down'; -import { Deco, Key, WebsitePaths } from 'ts/types'; +import { Deco, Key, ObjectMap, WebsitePaths } from 'ts/types'; import { constants } from 'ts/utils/constants'; import { Translate } from 'ts/utils/translate'; -interface KeyToLinkInfo { - [key: string]: LinkInfo; -} - interface LinkInfo { link: string; shouldOpenNewTab: boolean; } -const gettingStartedKeyToLinkInfo1: KeyToLinkInfo = { +const gettingStartedKeyToLinkInfo1: ObjectMap<LinkInfo> = { [Key.BuildARelayer]: { link: `${WebsitePaths.Wiki}#Build-A-Relayer`, shouldOpenNewTab: false, @@ -26,7 +22,7 @@ const gettingStartedKeyToLinkInfo1: KeyToLinkInfo = { shouldOpenNewTab: false, }, }; -const gettingStartedKeyToLinkInfo2: KeyToLinkInfo = { +const gettingStartedKeyToLinkInfo2: ObjectMap<LinkInfo> = { [Key.TradingTutorial]: { link: `${WebsitePaths.Wiki}#Find,-Submit,-Fill-Order-From-Relayer`, shouldOpenNewTab: false, @@ -36,7 +32,7 @@ const gettingStartedKeyToLinkInfo2: KeyToLinkInfo = { shouldOpenNewTab: false, }, }; -const popularDocsToLinkInfos: KeyToLinkInfo = { +const popularDocsToLinkInfos: ObjectMap<LinkInfo> = { [Key.ZeroExJs]: { link: WebsitePaths.ZeroExJs, shouldOpenNewTab: false, @@ -50,7 +46,7 @@ const popularDocsToLinkInfos: KeyToLinkInfo = { shouldOpenNewTab: false, }, }; -const usefulLinksToLinkInfo: KeyToLinkInfo = { +const usefulLinksToLinkInfo: ObjectMap<LinkInfo> = { [Key.Github]: { link: constants.URL_GITHUB_ORG, shouldOpenNewTab: true, @@ -136,7 +132,7 @@ export class DevelopersDropDown extends React.Component<DevelopersDropDownProps, fontSize: 14, }} > - VIEW ALL DOCUMENTATION + {this.props.translate.get(Key.ViewAllDocumentation, Deco.Upper)} </Link> </div> </div> @@ -158,7 +154,7 @@ export class DevelopersDropDown extends React.Component<DevelopersDropDownProps, </div> ); } - private _renderLinkSection(keyToLinkInfo: KeyToLinkInfo): React.ReactNode { + private _renderLinkSection(keyToLinkInfo: ObjectMap<LinkInfo>): React.ReactNode { const linkStyle: React.CSSProperties = { color: colors.lightBlueA700, fontFamily: 'Roboto, Roboto Mono', |