Refactor assert revert helper to encapsulate promises (#628)

test/BasicToken.test.js

@@ -1,4 +1,4 @@
-const assertRevert = require('./helpers/assertRevert');
+import assertRevert from './helpers/assertRevert';
 
 var BasicTokenMock = artifacts.require('mocks/BasicTokenMock.sol');
 
@@ -23,21 +23,11 @@ contract('BasicToken', function (accounts) {
 
   it('should throw an error when trying to transfer more than balance', async function () {
     let token = await BasicTokenMock.new(accounts[0], 100);
-    try {
+    await assertRevert(token.transfer(accounts[1], 101));
-      await token.transfer(accounts[1], 101);
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 
   it('should throw an error when trying to transfer to 0x0', async function () {
     let token = await BasicTokenMock.new(accounts[0], 100);
-    try {
+    await assertRevert(token.transfer(0x0, 100));
-      await token.transfer(0x0, 100);
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 });

test/Claimable.test.js

@@ -1,5 +1,5 @@
 
-const assertRevert = require('./helpers/assertRevert');
+import assertRevert from './helpers/assertRevert';
 
 var Claimable = artifacts.require('../contracts/ownership/Claimable.sol');
 
@@ -24,24 +24,15 @@ contract('Claimable', function (accounts) {
   });
 
   it('should prevent to claimOwnership from no pendingOwner', async function () {
-    try {
+    await assertRevert(claimable.claimOwnership({ from: accounts[2] }));
-      await claimable.claimOwnership({ from: accounts[2] });
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 
   it('should prevent non-owners from transfering', async function () {
     const other = accounts[2];
     const owner = await claimable.owner.call();
+
     assert.isTrue(owner !== other);
-    try {
+    await assertRevert(claimable.transferOwnership(other, { from: other }));
-      await claimable.transferOwnership(other, { from: other });
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 
   describe('after initiating a transfer', function () {

test/DayLimit.test.js

@@ -2,7 +2,7 @@
 import latestTime from './helpers/latestTime';
 import { increaseTimeTo, duration } from './helpers/increaseTime';
 
-const assertRevert = require('./helpers/assertRevert');
+import assertRevert from './helpers/assertRevert';
 
 const DayLimitMock = artifacts.require('mocks/DayLimitMock.sol');
 
@@ -34,13 +34,7 @@ contract('DayLimit', function (accounts) {
     await dayLimit.attemptSpend(8);
     let spentToday = await dayLimit.spentToday();
     assert.equal(spentToday, 8);
-
+    await assertRevert(dayLimit.attemptSpend(3));
-    try {
-      await dayLimit.attemptSpend(3);
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 
   it('should allow spending if daily limit is reached and then set higher', async function () {
@@ -48,12 +42,7 @@ contract('DayLimit', function (accounts) {
     let spentToday = await dayLimit.spentToday();
     assert.equal(spentToday, 8);
 
-    try {
+    await assertRevert(dayLimit.attemptSpend(3));
-      await dayLimit.attemptSpend(3);
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
     spentToday = await dayLimit.spentToday();
     assert.equal(spentToday, 8);
 
@@ -68,12 +57,7 @@ contract('DayLimit', function (accounts) {
     let spentToday = await dayLimit.spentToday();
     assert.equal(spentToday, 8);
 
-    try {
+    await assertRevert(dayLimit.attemptSpend(3));
-      await dayLimit.attemptSpend(3);
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
     spentToday = await dayLimit.spentToday();
     assert.equal(spentToday, 8);
 
@@ -91,12 +75,7 @@ contract('DayLimit', function (accounts) {
     let spentToday = await dayLimit.spentToday();
     assert.equal(spentToday, 8);
 
-    try {
+    await assertRevert(dayLimit.attemptSpend(3));
-      await dayLimit.attemptSpend(3);
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
     spentToday = await dayLimit.spentToday();
     assert.equal(spentToday, 8);
 

test/LimitBalance.test.js

@@ -1,6 +1,5 @@
-
+import assertRevert from './helpers/assertRevert';
 var LimitBalanceMock = artifacts.require('mocks/LimitBalanceMock.sol');
-const assertRevert = require('./helpers/assertRevert');
 
 contract('LimitBalance', function (accounts) {
   let lb;
@@ -25,12 +24,7 @@ contract('LimitBalance', function (accounts) {
 
   it('shouldnt allow sending above limit', async function () {
     let amount = 1110;
-    try {
+    await assertRevert(lb.limitedDeposit({ value: amount }));
-      await lb.limitedDeposit({ value: amount });
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 
   it('should allow multiple sends below limit', async function () {
@@ -48,12 +42,6 @@ contract('LimitBalance', function (accounts) {
     await lb.limitedDeposit({ value: amount });
 
     assert.equal(web3.eth.getBalance(lb.address), amount);
-
+    await assertRevert(lb.limitedDeposit({ value: amount + 1 }));
-    try {
-      await lb.limitedDeposit({ value: amount + 1 });
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 });

test/Ownable.test.js

@@ -1,5 +1,5 @@
 
-const assertRevert = require('./helpers/assertRevert');
+import assertRevert from './helpers/assertRevert';
 
 var Ownable = artifacts.require('../contracts/ownership/Ownable.sol');
 
@@ -27,21 +27,11 @@ contract('Ownable', function (accounts) {
     const other = accounts[2];
     const owner = await ownable.owner.call();
     assert.isTrue(owner !== other);
-    try {
+    await assertRevert(ownable.transferOwnership(other, { from: other }));
-      await ownable.transferOwnership(other, { from: other });
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 
   it('should guard ownership against stuck state', async function () {
     let originalOwner = await ownable.owner();
-    try {
+    await assertRevert(ownable.transferOwnership(null, { from: originalOwner }));
-      await ownable.transferOwnership(null, { from: originalOwner });
-      assert.fail();
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 });

test/Pausable.test.js

@@ -1,5 +1,5 @@
 
-const assertRevert = require('./helpers/assertRevert');
+import assertRevert from './helpers/assertRevert';
 const PausableMock = artifacts.require('mocks/PausableMock.sol');
 
 contract('Pausable', function (accounts) {
@@ -19,24 +19,14 @@ contract('Pausable', function (accounts) {
     let count0 = await Pausable.count();
     assert.equal(count0, 0);
 
-    try {
+    await assertRevert(Pausable.normalProcess());
-      await Pausable.normalProcess();
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
     let count1 = await Pausable.count();
     assert.equal(count1, 0);
   });
 
   it('can not take drastic measure in non-pause', async function () {
     let Pausable = await PausableMock.new();
-    try {
+    await assertRevert(Pausable.drasticMeasure());
-      await Pausable.drasticMeasure();
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
     const drasticMeasureTaken = await Pausable.drasticMeasureTaken();
     assert.isFalse(drasticMeasureTaken);
   });
@@ -64,12 +54,8 @@ contract('Pausable', function (accounts) {
     let Pausable = await PausableMock.new();
     await Pausable.pause();
     await Pausable.unpause();
-    try {
+
-      await Pausable.drasticMeasure();
+    await assertRevert(Pausable.drasticMeasure());
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
 
     const drasticMeasureTaken = await Pausable.drasticMeasureTaken();
     assert.isFalse(drasticMeasureTaken);

test/PausableToken.test.js

@@ -1,7 +1,7 @@
 'user strict';
 
-const assertRevert = require('./helpers/assertRevert');
+import assertRevert from './helpers/assertRevert';
-var PausableTokenMock = artifacts.require('mocks/PausableTokenMock.sol');
+var PausableTokenMock = artifacts.require('./mocks/PausableTokenMock.sol');
 
 contract('PausableToken', function (accounts) {
   let token;
@@ -53,21 +53,11 @@ contract('PausableToken', function (accounts) {
 
   it('should throw an error trying to transfer while transactions are paused', async function () {
     await token.pause();
-    try {
+    await assertRevert(token.transfer(accounts[1], 100));
-      await token.transfer(accounts[1], 100);
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 
   it('should throw an error trying to transfer from another account while transactions are paused', async function () {
     await token.pause();
-    try {
+    await assertRevert(token.transferFrom(accounts[0], accounts[1], 100));
-      await token.transferFrom(accounts[0], accounts[1], 100);
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 });

test/SafeMath.test.js

@@ -1,4 +1,4 @@
-const assertRevert = require('./helpers/assertRevert');
+import assertRevert from './helpers/assertRevert';
 const assertJump = require('./helpers/assertJump');
 var SafeMathMock = artifacts.require('mocks/SafeMathMock.sol');
 
@@ -49,22 +49,12 @@ contract('SafeMath', function (accounts) {
   it('should throw an error on addition overflow', async function () {
     let a = 115792089237316195423570985008687907853269984665640564039457584007913129639935;
     let b = 1;
-    try {
+    await assertRevert(safeMath.add(a, b));
-      await safeMath.add(a, b);
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 
   it('should throw an error on multiplication overflow', async function () {
     let a = 115792089237316195423570985008687907853269984665640564039457584007913129639933;
     let b = 2;
-    try {
+    await assertRevert(safeMath.multiply(a, b));
-      await safeMath.multiply(a, b);
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 });

test/StandardToken.test.js

@@ -1,5 +1,5 @@
 
-const assertRevert = require('./helpers/assertRevert');
+import assertRevert from './helpers/assertRevert';
 
 var StandardTokenMock = artifacts.require('mocks/StandardTokenMock.sol');
 
@@ -36,12 +36,7 @@ contract('StandardToken', function (accounts) {
 
   it('should throw an error when trying to transfer more than balance', async function () {
     let token = await StandardTokenMock.new(accounts[0], 100);
-    try {
+    await assertRevert(token.transfer(accounts[1], 101));
-      await token.transfer(accounts[1], 101);
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 
   it('should return correct balances after transfering from another account', async function () {
@@ -61,23 +56,13 @@ contract('StandardToken', function (accounts) {
 
   it('should throw an error when trying to transfer more than allowed', async function () {
     await token.approve(accounts[1], 99);
-    try {
+    await assertRevert(token.transferFrom(accounts[0], accounts[2], 100, { from: accounts[1] }));
-      await token.transferFrom(accounts[0], accounts[2], 100, { from: accounts[1] });
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 
   it('should throw an error when trying to transferFrom more than _from has', async function () {
     let balance0 = await token.balanceOf(accounts[0]);
     await token.approve(accounts[1], 99);
-    try {
+    await assertRevert(token.transferFrom(accounts[0], accounts[2], balance0 + 1, { from: accounts[1] }));
-      await token.transferFrom(accounts[0], accounts[2], balance0 + 1, { from: accounts[1] });
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 
   describe('validating allowance updates to spender', function () {
@@ -107,22 +92,12 @@ contract('StandardToken', function (accounts) {
 
   it('should throw an error when trying to transfer to 0x0', async function () {
     let token = await StandardTokenMock.new(accounts[0], 100);
-    try {
+    await assertRevert(token.transfer(0x0, 100));
-      await token.transfer(0x0, 100);
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 
   it('should throw an error when trying to transferFrom to 0x0', async function () {
     let token = await StandardTokenMock.new(accounts[0], 100);
     await token.approve(accounts[1], 100);
-    try {
+    await assertRevert(token.transferFrom(accounts[0], 0x0, 100, { from: accounts[1] }));
-      await token.transferFrom(accounts[0], 0x0, 100, { from: accounts[1] });
-      assert.fail('should have thrown before');
-    } catch (error) {
-      assertRevert(error);
-    }
   });
 });

test/helpers/assertRevert.js

@@ -1,3 +1,9 @@
-module.exports = function (error) {
+export default async promise => {
-  assert.isAbove(error.message.search('revert'), -1, 'Error containing "revert" must be returned');
+  try {
+    await promise;
+    assert.fail('Expected revert not received');
+  } catch (error) {
+    const revertFound = error.message.search('revert') >= 0;
+    assert(revertFound, `Expected "revert", got ${error} instead`);
+  }
 };