Fix/protocol test flakyness (#2155)

.prettierignore

@@ -21,7 +21,7 @@ packages/protocol/types/
 !packages/protocol/lib/**/*.ts
 packages/protocol/scripts/**/*.js
 packages/protocol/migrations/**/*.js
-packages/protocol/tests/**/*.js
+packages/protocol/test/**/*.js
 
 packages/verification-pool-api/contracts/*.ts
 

packages/protocol/lib/test-utils.ts

@@ -41,7 +41,9 @@ export async function jsonRpc(web3: Web3, method: string, params: any[] = []): P
         jsonrpc: '2.0',
         method,
         params,
-        id: new Date().getTime(),
+        // salt id generation, milliseconds might not be
+        // enough to generate unique ids
+        id: new Date().getTime() + Math.floor(Math.random() * ( 1 + 100 - 1 )),
       },
       // @ts-ignore
       (err: any, result: any) => {
@@ -240,13 +242,13 @@ export function assertLogMatches(
 }
 
 export function assertEqualBN(
-  value: number | BN | BigNumber,
+  actual: number | BN | BigNumber,
   expected: number | BN | BigNumber,
   msg?: string
 ) {
   assert(
-    web3.utils.toBN(value).eq(web3.utils.toBN(expected)),
-    `expected ${expected.toString()} and got ${value.toString()}. ${msg || ''}`
+    web3.utils.toBN(actual).eq(web3.utils.toBN(expected)),
+    `expected ${expected.toString(10)} and got ${actual.toString(10)}. ${msg || ''}`
   )
 }
 

packages/protocol/runTests.js

@@ -70,7 +70,7 @@ async function test() {
       const testFiles = glob.readdirSync(testGlob)
       if (testFiles.length === 0) {
         // tslint:disable-next-line: no-console
-        console.error(`No tests matched with ${argv._}`)
+        console.error(`No test files matched with ${testGlob}`)
         process.exit(1)
       }
       testArgs = testArgs.concat(testFiles)

packages/protocol/test/baklava/freezable.ts

@@ -1,4 +1,4 @@
-import { assertSameAddress, assertLogMatches2, assertRevert } from '@celo/protocol/lib/test-utils'
+import { assertLogMatches2, assertRevert, assertSameAddress } from '@celo/protocol/lib/test-utils'
 import { FreezableTestInstance } from 'types'
 
 contract('Freezable', (accounts: string[]) => {
@@ -7,7 +7,7 @@ contract('Freezable', (accounts: string[]) => {
 
   beforeEach(async () => {
     freezableTest = await FreezableTest.new()
-    freezableTest.setFreezer(accounts[0])
+    await freezableTest.setFreezer(accounts[0])
   })
 
   describe('_setFreezer', () => {
@@ -55,7 +55,7 @@ contract('Freezable', (accounts: string[]) => {
 
   describe('unfreeze', () => {
     beforeEach(async () => {
-      freezableTest.freeze()
+      await freezableTest.freeze()
     })
 
     it('should allow freezer to unfreeze the contract', async () => {

packages/protocol/test/common/accounts.ts

@@ -1,5 +1,3 @@
-import { parseSolidityStringArray } from '@celo/utils/lib/parsing'
-import { upperFirst } from 'lodash'
 import { CeloContractName } from '@celo/protocol/lib/registry-utils'
 import { getParsedSignatureOfAddress } from '@celo/protocol/lib/signing-utils'
 import {
@@ -8,6 +6,8 @@ import {
   assertRevert,
   NULL_ADDRESS,
 } from '@celo/protocol/lib/test-utils'
+import { parseSolidityStringArray } from '@celo/utils/lib/parsing'
+import { upperFirst } from 'lodash'
 import {
   AccountsContract,
   AccountsInstance,
@@ -243,7 +243,7 @@ contract('Accounts', (accounts: string[]) => {
 
     describe('when the account has been created', () => {
       beforeEach(async () => {
-        accountsInstance.createAccount()
+        await accountsInstance.createAccount()
       })
 
       it('should set the walletAddress', async () => {
@@ -279,7 +279,7 @@ contract('Accounts', (accounts: string[]) => {
 
     describe('when the account has been created', () => {
       beforeEach(async () => {
-        accountsInstance.createAccount()
+        await accountsInstance.createAccount()
       })
 
       it('should set the metadataURL', async () => {
@@ -304,9 +304,9 @@ contract('Accounts', (accounts: string[]) => {
     it('returns multiple metadata URLs', async () => {
       const randomStrings = accounts.map((_) => web3.utils.randomHex(20).slice(2))
       await Promise.all(
-        accounts.map(async (account, i) => {
-          await accountsInstance.createAccount({ from: account })
-          await accountsInstance.setMetadataURL(randomStrings[i], { from: account })
+        accounts.map(async (mappedAccount, i) => {
+          await accountsInstance.createAccount({ from: mappedAccount })
+          await accountsInstance.setMetadataURL(randomStrings[i], { from: mappedAccount })
         })
       )
       const [stringLengths, data] = await accountsInstance.batchGetMetadataURL(accounts)
@@ -329,7 +329,7 @@ contract('Accounts', (accounts: string[]) => {
 
     describe('when the account has been created', () => {
       beforeEach(async () => {
-        accountsInstance.createAccount()
+        await accountsInstance.createAccount()
       })
 
       it('should set the name', async () => {

packages/protocol/test/common/gaspriceminimum.ts

@@ -160,14 +160,14 @@ contract('GasPriceMinimum', (accounts: string[]) => {
       const getUpdatedGasPriceMinimum = (
         previousGasPriceMinimum,
         density,
-        targetDensity,
-        adjustmentSpeed
+        _targetDensity,
+        _adjustmentSpeed
       ) => {
         const one = new BigNumber(1)
         return previousGasPriceMinimum
           .times(
             one.plus(
-              fromFixed(adjustmentSpeed).times(fromFixed(density).minus(fromFixed(targetDensity)))
+              fromFixed(_adjustmentSpeed).times(fromFixed(density).minus(fromFixed(_targetDensity)))
             )
           )
           .plus(one)

packages/protocol/test/common/linkedlist.ts

@@ -45,8 +45,9 @@ contract('LinkedListTest', () => {
     describe('when inserting to a list with more items', () => {
       beforeEach(async () => {
         await linkedListTest.insert(firstKey, NULL_KEY, NULL_KEY)
-        for (let i = 1; i < keys.length; i++)
+        for (let i = 1; i < keys.length; i++) {
           await linkedListTest.insert(keys[i], NULL_KEY, keys[i - 1])
+        }
       })
 
       it('should revert if next is equal to key (beginning)', async () => {

packages/protocol/test/governance/blockchainparams.ts

@@ -1,6 +1,6 @@
-import { assertContainSubset, assertRevert, assertEqualBN } from '@celo/protocol/lib/test-utils'
-import { BlockchainParametersContract, BlockchainParametersInstance } from 'types'
+import { assertContainSubset, assertEqualBN, assertRevert } from '@celo/protocol/lib/test-utils'
 import { BigNumber } from 'bignumber.js'
+import { BlockchainParametersContract, BlockchainParametersInstance } from 'types'
 
 const BlockchainParameters: BlockchainParametersContract = artifacts.require('BlockchainParameters')
 

packages/protocol/test/governance/election.ts

@@ -375,10 +375,10 @@ contract('Election', (accounts: string[]) => {
               })
 
               describe('when the voter has already voted for this group', () => {
-                let resp: any
+                let response: any
                 beforeEach(async () => {
                   await mockLockedGold.incrementNonvotingAccountBalance(voter, value)
-                  resp = await election.vote(group, value, NULL_ADDRESS, NULL_ADDRESS)
+                  response = await election.vote(group, value, NULL_ADDRESS, NULL_ADDRESS)
                 })
 
                 it('should not change the list of groups the account has voted for', async () => {
@@ -416,8 +416,8 @@ contract('Election', (accounts: string[]) => {
                 })
 
                 it('should emit the ValidatorGroupVoteCast event', async () => {
-                  assert.equal(resp.logs.length, 1)
-                  const log = resp.logs[0]
+                  assert.equal(response.logs.length, 1)
+                  const log = response.logs[0]
                   assertContainSubset(log, {
                     event: 'ValidatorGroupVoteCast',
                     args: {

packages/protocol/test/governance/epochrewards.ts

@@ -4,22 +4,23 @@ import {
   assertEqualBN,
   assertEqualDpBN,
   assertRevert,
+  jsonRpc,
   timeTravel,
 } from '@celo/protocol/lib/test-utils'
+import { fromFixed, toFixed } from '@celo/utils/lib/fixidity'
 import BigNumber from 'bignumber.js'
 import {
+  EpochRewardsTestContract,
+  EpochRewardsTestInstance,
   MockElectionContract,
   MockElectionInstance,
   MockGoldTokenContract,
   MockGoldTokenInstance,
   MockSortedOraclesContract,
   MockSortedOraclesInstance,
-  EpochRewardsTestContract,
-  EpochRewardsTestInstance,
   RegistryContract,
   RegistryInstance,
 } from 'types'
-import { fromFixed, toFixed } from '@celo/utils/lib/fixidity'
 
 const EpochRewards: EpochRewardsTestContract = artifacts.require('EpochRewardsTest')
 const MockElection: MockElectionContract = artifacts.require('MockElection')
@@ -34,7 +35,7 @@ EpochRewards.numberFormat = 'BigNumber'
 const YEAR = new BigNumber(365 * 24 * 60 * 60)
 const SUPPLY_CAP = new BigNumber(web3.utils.toWei('1000000000'))
 
-const getExpectedTargetTotalSupply = (timeDelta: BigNumber) => {
+const getExpectedTargetTotalSupply = (timeDelta: BigNumber): BigNumber => {
   const genesisSupply = new BigNumber(web3.utils.toWei('600000000'))
   const linearRewards = new BigNumber(web3.utils.toWei('200000000'))
   return genesisSupply
@@ -68,10 +69,13 @@ contract('EpochRewards', (accounts: string[]) => {
   const mockStableTokenAddress = web3.utils.randomHex(20)
   const sortedOraclesDenominator = new BigNumber('0x10000000000000000')
   const timeTravelToDelta = async (timeDelta: BigNumber) => {
-    const currentTime = new BigNumber((await web3.eth.getBlock('latest')).timestamp)
-    const startTime = await epochRewards.startTime()
-    const desiredTime = startTime.plus(timeDelta)
-    await timeTravel(desiredTime.minus(currentTime).toNumber(), web3)
+    // mine beforehand, just in case
+    await jsonRpc(web3, 'evm_mine', [])
+    const currentTime: BigNumber = new BigNumber((await web3.eth.getBlock('latest')).timestamp)
+    const startTime: BigNumber = await epochRewards.startTime()
+    const desiredTime: BigNumber = startTime.plus(timeDelta)
+    const delta: number = desiredTime.minus(currentTime).toNumber()
+    await timeTravel(delta, web3)
   }
 
   beforeEach(async () => {
@@ -354,7 +358,7 @@ contract('EpochRewards', (accounts: string[]) => {
 
   describe('#getTargetGoldTotalSupply()', () => {
     describe('when it has been fewer than 15 years since genesis', () => {
-      const timeDelta = YEAR.times(10)
+      const timeDelta: BigNumber = YEAR.times(10)
       beforeEach(async () => {
         await timeTravelToDelta(timeDelta)
       })

packages/protocol/test/governance/governance.ts

@@ -642,7 +642,7 @@ contract('Governance', (accounts: string[]) => {
           args: {
             destination,
             functionId: web3.utils.padRight(functionId, 64),
-            threshold: threshold,
+            threshold,
           },
         })
       })
@@ -675,7 +675,7 @@ contract('Governance', (accounts: string[]) => {
           args: {
             destination,
             functionId: web3.utils.padRight(functionId, 64),
-            threshold: threshold,
+            threshold,
           },
         })
       })
@@ -777,7 +777,7 @@ contract('Governance', (accounts: string[]) => {
             proposalId: new BigNumber(1),
             proposer: accounts[0],
             deposit: new BigNumber(minDeposit),
-            timestamp: timestamp,
+            timestamp,
             transactionCount: 0,
           },
         })
@@ -842,7 +842,7 @@ contract('Governance', (accounts: string[]) => {
             proposalId: new BigNumber(1),
             proposer: accounts[0],
             deposit: new BigNumber(minDeposit),
-            timestamp: timestamp,
+            timestamp,
             transactionCount: 1,
           },
         })
@@ -915,7 +915,7 @@ contract('Governance', (accounts: string[]) => {
             proposalId: new BigNumber(1),
             proposer: accounts[0],
             deposit: new BigNumber(minDeposit),
-            timestamp: timestamp,
+            timestamp,
             transactionCount: 2,
           },
         })
@@ -1039,10 +1039,10 @@ contract('Governance', (accounts: string[]) => {
           // @ts-ignore: TODO(mcortesi) fix typings for TransactionDetails
           { value: minDeposit }
         )
-        const otherAccount = accounts[1]
-        await accountsInstance.createAccount({ from: otherAccount })
-        await mockLockedGold.setAccountTotalLockedGold(otherAccount, weight)
-        await governance.upvote(otherProposalId, proposalId, 0, { from: otherAccount })
+        const otherAccount1 = accounts[1]
+        await accountsInstance.createAccount({ from: otherAccount1 })
+        await mockLockedGold.setAccountTotalLockedGold(otherAccount1, weight)
+        await governance.upvote(otherProposalId, proposalId, 0, { from: otherAccount1 })
         await timeTravel(queueExpiry, web3)
       })
 
@@ -1105,11 +1105,11 @@ contract('Governance', (accounts: string[]) => {
     describe('when the previously upvoted proposal is in the queue and expired', () => {
       const upvotedProposalId = 2
       // Expire the upvoted proposal without dequeueing it.
-      const queueExpiry = 60
+      const queueExpiry1 = 60
       beforeEach(async () => {
         await governance.setQueueExpiry(60)
         await governance.upvote(proposalId, 0, 0)
-        await timeTravel(queueExpiry, web3)
+        await timeTravel(queueExpiry1, web3)
         await governance.propose(
           [transactionSuccess1.value],
           [transactionSuccess1.destination],

packages/protocol/test/governance/lockedgold.ts

@@ -40,7 +40,7 @@ const HOUR = 60 * 60
 const DAY = 24 * HOUR
 
 contract('LockedGold', (accounts: string[]) => {
-  let account = accounts[0]
+  const account = accounts[0]
   const nonOwner = accounts[1]
   const unlockingPeriod = 3 * DAY
   let accountsInstance: AccountsInstance