From bd7517cfd489a9789f81c247fb45329881274d15 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Sat, 10 Mar 2018 06:07:55 +0100 Subject: Add support for async calls under coverage --- packages/contracts/test/token_registry.ts | 12 ++++--- packages/sol-cov/package.json | 3 +- packages/sol-cov/src/coverage_subprovider.ts | 52 ++++++++++++++++++++-------- yarn.lock | 10 +++--- 4 files changed, 52 insertions(+), 25 deletions(-) diff --git a/packages/contracts/test/token_registry.ts b/packages/contracts/test/token_registry.ts index 5313ec1b1..9dcc77b82 100644 --- a/packages/contracts/test/token_registry.ts +++ b/packages/contracts/test/token_registry.ts @@ -142,8 +142,10 @@ describe('TokenRegistry', () => { await tokenReg.setTokenName.sendTransactionAsync(token1.address, token2.name, { from: owner, }); - const newData = await tokenRegWrapper.getTokenByNameAsync(token2.name); - const oldData = await tokenRegWrapper.getTokenByNameAsync(token1.name); + const [newData, oldData] = await Promise.all([ + tokenRegWrapper.getTokenByNameAsync(token2.name), + tokenRegWrapper.getTokenByNameAsync(token1.name), + ]); const expectedNewData = _.assign({}, token1, { name: token2.name }); const expectedOldData = nullToken; @@ -177,8 +179,10 @@ describe('TokenRegistry', () => { it('should change the token symbol when called by owner', async () => { await tokenReg.setTokenSymbol.sendTransactionAsync(token1.address, token2.symbol, { from: owner }); - const newData = await tokenRegWrapper.getTokenBySymbolAsync(token2.symbol); - const oldData = await tokenRegWrapper.getTokenBySymbolAsync(token1.symbol); + const [newData, oldData] = await Promise.all([ + tokenRegWrapper.getTokenBySymbolAsync(token2.symbol), + tokenRegWrapper.getTokenBySymbolAsync(token1.symbol), + ]); const expectedNewData = _.assign({}, token1, { symbol: token2.symbol }); const expectedOldData = nullToken; diff --git a/packages/sol-cov/package.json b/packages/sol-cov/package.json index f80e186ca..6a0b61ce9 100644 --- a/packages/sol-cov/package.json +++ b/packages/sol-cov/package.json @@ -22,10 +22,11 @@ "dependencies": { "@0xproject/subproviders": "^0.7.0", "@0xproject/utils": "^0.3.4", - "glob": "^7.1.2", "ethereumjs-util": "^5.1.1", + "glob": "^7.1.2", "istanbul": "^0.4.5", "lodash": "^4.17.4", + "semaphore-async-await": "^1.5.1", "solidity-coverage": "^0.4.10", "solidity-parser-sc": "^0.4.4", "web3": "^0.20.0" diff --git a/packages/sol-cov/src/coverage_subprovider.ts b/packages/sol-cov/src/coverage_subprovider.ts index f91b95c79..c84211d3a 100644 --- a/packages/sol-cov/src/coverage_subprovider.ts +++ b/packages/sol-cov/src/coverage_subprovider.ts @@ -1,21 +1,32 @@ import { Callback, NextCallback, Subprovider } from '@0xproject/subproviders'; import { promisify } from '@0xproject/utils'; import * as _ from 'lodash'; +import { Lock } from 'semaphore-async-await'; import * as Web3 from 'web3'; import { constants } from './constants'; import { CoverageManager } from './coverage_manager'; import { TraceInfoExistingContract, TraceInfoNewContract } from './types'; +interface MaybeFakeTxData extends Web3.TxData { + isFakeTransaction?: boolean; +} + /* * This class implements the web3-provider-engine subprovider interface and collects traces of all transactions that were sent and all calls that were executed. + * Because there is no notion of call trace in the rpc - we collect them in rather non-obvious/hacky way. + * On each call - we create a snapshot, execute the call as a transaction, get the trace, revert the snapshot. + * That allows us to not influence the test behaviour. * Source: https://github.com/MetaMask/provider-engine/blob/master/subproviders/subprovider.js */ export class CoverageSubprovider extends Subprovider { + // Lock is used to not accept normal transactions while doing call/snapshot magic because they'll be reverted later otherwise + private _lock: Lock; private _coverageManager: CoverageManager; private _defaultFromAddress: string; constructor(artifactsPath: string, sourcesPath: string, networkId: number, defaultFromAddress: string) { super(); + this._lock = new Lock(); this._defaultFromAddress = defaultFromAddress; this._coverageManager = new CoverageManager( artifactsPath, @@ -50,11 +61,16 @@ export class CoverageSubprovider extends Subprovider { await this._coverageManager.writeCoverageAsync(); } private async _onTransactionSentAsync( - txData: Web3.TxData, + txData: MaybeFakeTxData, err: Error | null, - txHash?: string, - cb?: Callback, + txHash: string | undefined, + cb: Callback, ): Promise { + if (!txData.isFakeTransaction) { + // This transaction is a usual ttransaction. Not a call executed as one. + // And we don't want it to be executed within a snapshotting period + await this._lock.acquire(); + } if (_.isNull(err)) { await this._recordTxTraceAsync(txData.to || constants.NEW_CONTRACT, txData.data, txHash as string); } else { @@ -72,9 +88,12 @@ export class CoverageSubprovider extends Subprovider { ); } } - if (!_.isUndefined(cb)) { - cb(); + if (!txData.isFakeTransaction) { + // This transaction is a usual ttransaction. Not a call executed as one. + // And we don't want it to be executed within a snapshotting period + await this._lock.release(); } + cb(); } private async _onCallExecutedAsync( callData: Partial, @@ -115,22 +134,25 @@ export class CoverageSubprovider extends Subprovider { } } private async _recordCallTraceAsync(callData: Partial, blockNumber: Web3.BlockParam): Promise { + // We don't want other transactions to be exeucted during snashotting period, that's why we lock the + // transaction execution for all transactions except our fake ones. + await this._lock.acquire(); const snapshotId = Number((await this.emitPayloadAsync({ method: 'evm_snapshot' })).result); - const txData = callData; - if (_.isUndefined(txData.from)) { - txData.from = this._defaultFromAddress; - } - const txDataWithFromAddress = txData as Web3.TxData; + const fakeTxData: MaybeFakeTxData = { + from: this._defaultFromAddress, + isFakeTransaction: true, // This transaction (and only it) is allowed to come through when the lock is locked + ...callData, + }; try { - const txHash = (await this.emitPayloadAsync({ + await this.emitPayloadAsync({ method: 'eth_sendTransaction', - params: [txDataWithFromAddress], - })).result; - await this._onTransactionSentAsync(txDataWithFromAddress, null, txHash); + params: [fakeTxData], + }); } catch (err) { - await this._onTransactionSentAsync(txDataWithFromAddress, err, undefined); + // Even if this transaction failed - we've already recorded it's trace. } const jsonRPCResponse = await this.emitPayloadAsync({ method: 'evm_revert', params: [snapshotId] }); + await this._lock.release(); const didRevert = jsonRPCResponse.result; if (!didRevert) { throw new Error('Failed to revert the snapshot'); diff --git a/yarn.lock b/yarn.lock index e3dcde420..bcb436056 100644 --- a/yarn.lock +++ b/yarn.lock @@ -711,7 +711,7 @@ async-eventemitter@^0.2.2: dependencies: async "^2.4.0" -"async-eventemitter@github:ahultgren/async-eventemitter#fa06e39e56786ba541c180061dbf2c0a5bbf951c": +async-eventemitter@ahultgren/async-eventemitter#fa06e39e56786ba541c180061dbf2c0a5bbf951c: version "0.2.3" resolved "https://codeload.github.com/ahultgren/async-eventemitter/tar.gz/fa06e39e56786ba541c180061dbf2c0a5bbf951c" dependencies: @@ -1493,13 +1493,13 @@ big.js@^3.1.3: version "3.2.0" resolved "https://registry.yarnpkg.com/big.js/-/big.js-3.2.0.tgz#a5fc298b81b9e0dca2e458824784b65c52ba588e" -"bignumber.js@git+https://github.com/debris/bignumber.js.git#94d7146671b9719e00a09c29b01a691bc85048c2": +"bignumber.js@git+https://github.com/debris/bignumber.js#master": version "2.0.7" - resolved "git+https://github.com/debris/bignumber.js.git#94d7146671b9719e00a09c29b01a691bc85048c2" + resolved "git+https://github.com/debris/bignumber.js#c7a38de919ed75e6fb6ba38051986e294b328df9" -"bignumber.js@git+https://github.com/debris/bignumber.js.git#master": +"bignumber.js@git+https://github.com/debris/bignumber.js.git#94d7146671b9719e00a09c29b01a691bc85048c2": version "2.0.7" - resolved "git+https://github.com/debris/bignumber.js.git#c7a38de919ed75e6fb6ba38051986e294b328df9" + resolved "git+https://github.com/debris/bignumber.js.git#94d7146671b9719e00a09c29b01a691bc85048c2" "bignumber.js@git+https://github.com/frozeman/bignumber.js-nolookahead.git": version "2.0.7" -- cgit v1.2.3