Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor assert revert helper to encapsulate promises #628

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 3 additions & 13 deletions test/BasicToken.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const assertRevert = require('./helpers/assertRevert');
import assertRevert from './helpers/assertRevert';

var BasicTokenMock = artifacts.require('mocks/BasicTokenMock.sol');

Expand All @@ -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 token.transfer(accounts[1], 101);
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(token.transfer(accounts[1], 101));
});

it('should throw an error when trying to transfer to 0x0', async function () {
let token = await BasicTokenMock.new(accounts[0], 100);
try {
await token.transfer(0x0, 100);
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(token.transfer(0x0, 100));
});
});
17 changes: 4 additions & 13 deletions test/Claimable.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

const assertRevert = require('./helpers/assertRevert');
import assertRevert from './helpers/assertRevert';

var Claimable = artifacts.require('../contracts/ownership/Claimable.sol');

Expand All @@ -24,24 +24,15 @@ contract('Claimable', function (accounts) {
});

it('should prevent to claimOwnership from no pendingOwner', async function () {
try {
await claimable.claimOwnership({ from: accounts[2] });
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(claimable.claimOwnership({ from: accounts[2] }));
});

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 claimable.transferOwnership(other, { from: other });
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(claimable.transferOwnership(other, { from: other }));
});

describe('after initiating a transfer', function () {
Expand Down
31 changes: 5 additions & 26 deletions test/DayLimit.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -34,26 +34,15 @@ contract('DayLimit', function (accounts) {
await dayLimit.attemptSpend(8);
let spentToday = await dayLimit.spentToday();
assert.equal(spentToday, 8);

try {
await dayLimit.attemptSpend(3);
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(dayLimit.attemptSpend(3));
});

it('should allow spending if daily limit is reached and then set higher', async function () {
await dayLimit.attemptSpend(8);
let spentToday = await dayLimit.spentToday();
assert.equal(spentToday, 8);

try {
await dayLimit.attemptSpend(3);
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(dayLimit.attemptSpend(3));
spentToday = await dayLimit.spentToday();
assert.equal(spentToday, 8);

Expand All @@ -68,12 +57,7 @@ contract('DayLimit', function (accounts) {
let spentToday = await dayLimit.spentToday();
assert.equal(spentToday, 8);

try {
await dayLimit.attemptSpend(3);
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(dayLimit.attemptSpend(3));
spentToday = await dayLimit.spentToday();
assert.equal(spentToday, 8);

Expand All @@ -91,12 +75,7 @@ contract('DayLimit', function (accounts) {
let spentToday = await dayLimit.spentToday();
assert.equal(spentToday, 8);

try {
await dayLimit.attemptSpend(3);
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(dayLimit.attemptSpend(3));
spentToday = await dayLimit.spentToday();
assert.equal(spentToday, 8);

Expand Down
18 changes: 3 additions & 15 deletions test/LimitBalance.test.js
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -25,12 +24,7 @@ contract('LimitBalance', function (accounts) {

it('shouldnt allow sending above limit', async function () {
let amount = 1110;
try {
await lb.limitedDeposit({ value: amount });
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(lb.limitedDeposit({ value: amount }));
});

it('should allow multiple sends below limit', async function () {
Expand All @@ -48,12 +42,6 @@ contract('LimitBalance', function (accounts) {
await lb.limitedDeposit({ value: amount });

assert.equal(web3.eth.getBalance(lb.address), amount);

try {
await lb.limitedDeposit({ value: amount + 1 });
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(lb.limitedDeposit({ value: amount + 1 }));
});
});
16 changes: 3 additions & 13 deletions test/Ownable.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

const assertRevert = require('./helpers/assertRevert');
import assertRevert from './helpers/assertRevert';

var Ownable = artifacts.require('../contracts/ownership/Ownable.sol');

Expand Down Expand Up @@ -27,21 +27,11 @@ contract('Ownable', function (accounts) {
const other = accounts[2];
const owner = await ownable.owner.call();
assert.isTrue(owner !== other);
try {
await ownable.transferOwnership(other, { from: other });
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(ownable.transferOwnership(other, { from: other }));
});

it('should guard ownership against stuck state', async function () {
let originalOwner = await ownable.owner();
try {
await ownable.transferOwnership(null, { from: originalOwner });
assert.fail();
} catch (error) {
assertRevert(error);
}
await assertRevert(ownable.transferOwnership(null, { from: originalOwner }));
});
});
24 changes: 5 additions & 19 deletions test/Pausable.test.js
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -19,24 +19,14 @@ contract('Pausable', function (accounts) {
let count0 = await Pausable.count();
assert.equal(count0, 0);

try {
await Pausable.normalProcess();
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(Pausable.normalProcess());
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 Pausable.drasticMeasure();
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(Pausable.drasticMeasure());
const drasticMeasureTaken = await Pausable.drasticMeasureTaken();
assert.isFalse(drasticMeasureTaken);
});
Expand Down Expand Up @@ -64,12 +54,8 @@ contract('Pausable', function (accounts) {
let Pausable = await PausableMock.new();
await Pausable.pause();
await Pausable.unpause();
try {
await Pausable.drasticMeasure();
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}

await assertRevert(Pausable.drasticMeasure());

const drasticMeasureTaken = await Pausable.drasticMeasureTaken();
assert.isFalse(drasticMeasureTaken);
Expand Down
18 changes: 4 additions & 14 deletions test/PausableToken.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'user strict';

const assertRevert = require('./helpers/assertRevert');
var PausableTokenMock = artifacts.require('mocks/PausableTokenMock.sol');
import assertRevert from './helpers/assertRevert';
var PausableTokenMock = artifacts.require('./mocks/PausableTokenMock.sol');

contract('PausableToken', function (accounts) {
let token;
Expand Down Expand Up @@ -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 token.transfer(accounts[1], 100);
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(token.transfer(accounts[1], 100));
});

it('should throw an error trying to transfer from another account while transactions are paused', async function () {
await token.pause();
try {
await token.transferFrom(accounts[0], accounts[1], 100);
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(token.transferFrom(accounts[0], accounts[1], 100));
});
});
16 changes: 3 additions & 13 deletions test/SafeMath.test.js
Original file line number Diff line number Diff line change
@@ -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');

Expand Down Expand Up @@ -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 safeMath.add(a, b);
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(safeMath.add(a, b));
});

it('should throw an error on multiplication overflow', async function () {
let a = 115792089237316195423570985008687907853269984665640564039457584007913129639933;
let b = 2;
try {
await safeMath.multiply(a, b);
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(safeMath.multiply(a, b));
});
});
37 changes: 6 additions & 31 deletions test/StandardToken.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

const assertRevert = require('./helpers/assertRevert');
import assertRevert from './helpers/assertRevert';

var StandardTokenMock = artifacts.require('mocks/StandardTokenMock.sol');

Expand Down Expand Up @@ -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 token.transfer(accounts[1], 101);
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(token.transfer(accounts[1], 101));
});

it('should return correct balances after transfering from another account', async function () {
Expand All @@ -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 token.transferFrom(accounts[0], accounts[2], 100, { from: accounts[1] });
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(token.transferFrom(accounts[0], accounts[2], 100, { from: accounts[1] }));
});

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 token.transferFrom(accounts[0], accounts[2], balance0 + 1, { from: accounts[1] });
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(token.transferFrom(accounts[0], accounts[2], balance0 + 1, { from: accounts[1] }));
});

describe('validating allowance updates to spender', function () {
Expand Down Expand Up @@ -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 token.transfer(0x0, 100);
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(token.transfer(0x0, 100));
});

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 token.transferFrom(accounts[0], 0x0, 100, { from: accounts[1] });
assert.fail('should have thrown before');
} catch (error) {
assertRevert(error);
}
await assertRevert(token.transferFrom(accounts[0], 0x0, 100, { from: accounts[1] }));
});
});
10 changes: 8 additions & 2 deletions test/helpers/assertRevert.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
module.exports = function (error) {
assert.isAbove(error.message.search('revert'), -1, 'Error containing "revert" must be returned');
export default async promise => {
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`);
}
};