From f80c65ff67957404c964af864ffb230e4cbb1cf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Mon, 21 Jan 2019 17:51:35 -0300 Subject: [PATCH] Improve SafeMath test coverage. (#1611) * Improve SafeMath test coverage. * Fix linter error. * Split testCommutative into something more sane. --- test/drafts/SignedSafeMath.test.js | 54 +++++++++++++++++++----------- test/math/SafeMath.test.js | 31 +++++++++-------- 2 files changed, 51 insertions(+), 34 deletions(-) diff --git a/test/drafts/SignedSafeMath.test.js b/test/drafts/SignedSafeMath.test.js index 04d42fdee..59157a1e3 100644 --- a/test/drafts/SignedSafeMath.test.js +++ b/test/drafts/SignedSafeMath.test.js @@ -8,34 +8,43 @@ contract('SignedSafeMath', function () { this.safeMath = await SignedSafeMathMock.new(); }); + async function testCommutative (fn, lhs, rhs, expected) { + (await fn(lhs, rhs)).should.be.bignumber.equal(expected); + (await fn(rhs, lhs)).should.be.bignumber.equal(expected); + } + + async function testFailsCommutative (fn, lhs, rhs) { + await shouldFail.reverting(fn(lhs, rhs)); + await shouldFail.reverting(fn(rhs, lhs)); + } + describe('add', function () { it('adds correctly if it does not overflow and the result is positve', async function () { const a = new BN('1234'); const b = new BN('5678'); - (await this.safeMath.add(a, b)).should.be.bignumber.equal(a.add(b)); + await testCommutative(this.safeMath.add, a, b, a.add(b)); }); it('adds correctly if it does not overflow and the result is negative', async function () { const a = MAX_INT256; const b = MIN_INT256; - const result = await this.safeMath.add(a, b); - result.should.be.bignumber.equal(a.add(b)); + await testCommutative(this.safeMath.add, a, b, a.add(b)); }); it('reverts on positive addition overflow', async function () { const a = MAX_INT256; const b = new BN('1'); - await shouldFail.reverting(this.safeMath.add(a, b)); + await testFailsCommutative(this.safeMath.add, a, b); }); it('reverts on negative addition overflow', async function () { const a = MIN_INT256; const b = new BN('-1'); - await shouldFail.reverting(this.safeMath.add(a, b)); + await testFailsCommutative(this.safeMath.add, a, b); }); }); @@ -76,37 +85,28 @@ contract('SignedSafeMath', function () { const a = new BN('5678'); const b = new BN('-1234'); - const result = await this.safeMath.mul(a, b); - result.should.be.bignumber.equal(a.mul(b)); + await testCommutative(this.safeMath.mul, a, b, a.mul(b)); }); - it('handles a zero product correctly', async function () { + it('multiplies by zero correctly', async function () { const a = new BN('0'); const b = new BN('5678'); - const result = await this.safeMath.mul(a, b); - result.should.be.bignumber.equal(a.mul(b)); + await testCommutative(this.safeMath.mul, a, b, '0'); }); it('reverts on multiplication overflow, positive operands', async function () { const a = MAX_INT256; const b = new BN('2'); - await shouldFail.reverting(this.safeMath.mul(a, b)); + await testFailsCommutative(this.safeMath.mul, a, b); }); it('reverts when minimum integer is multiplied by -1', async function () { const a = MIN_INT256; const b = new BN('-1'); - await shouldFail.reverting(this.safeMath.mul(a, b)); - }); - - it('reverts when -1 is multiplied by minimum integer', async function () { - const a = new BN('-1'); - const b = MIN_INT256; - - await shouldFail.reverting(this.safeMath.mul(a, b)); + await testFailsCommutative(this.safeMath.mul, a, b); }); }); @@ -119,7 +119,21 @@ contract('SignedSafeMath', function () { result.should.be.bignumber.equal(a.div(b)); }); - it('reverts on zero division', async function () { + it('divides zero correctly', async function () { + const a = new BN('0'); + const b = new BN('5678'); + + (await this.safeMath.div(a, b)).should.be.bignumber.equal('0'); + }); + + it('returns complete number result on non-even division', async function () { + const a = new BN('7000'); + const b = new BN('5678'); + + (await this.safeMath.div(a, b)).should.be.bignumber.equal('1'); + }); + + it('reverts on division by zero', async function () { const a = new BN('-5678'); const b = new BN('0'); diff --git a/test/math/SafeMath.test.js b/test/math/SafeMath.test.js index 3869d5c1e..78654465a 100644 --- a/test/math/SafeMath.test.js +++ b/test/math/SafeMath.test.js @@ -8,19 +8,29 @@ contract('SafeMath', function () { this.safeMath = await SafeMathMock.new(); }); + async function testCommutative (fn, lhs, rhs, expected) { + (await fn(lhs, rhs)).should.be.bignumber.equal(expected); + (await fn(rhs, lhs)).should.be.bignumber.equal(expected); + } + + async function testFailsCommutative (fn, lhs, rhs) { + await shouldFail.reverting(fn(lhs, rhs)); + await shouldFail.reverting(fn(rhs, lhs)); + } + describe('add', function () { it('adds correctly', async function () { const a = new BN('5678'); const b = new BN('1234'); - (await this.safeMath.add(a, b)).should.be.bignumber.equal(a.add(b)); + await testCommutative(this.safeMath.add, a, b, a.add(b)); }); it('reverts on addition overflow', async function () { const a = MAX_UINT256; const b = new BN('1'); - await shouldFail.reverting(this.safeMath.add(a, b)); + await testFailsCommutative(this.safeMath.add, a, b); }); }); @@ -45,28 +55,21 @@ contract('SafeMath', function () { const a = new BN('1234'); const b = new BN('5678'); - (await this.safeMath.mul(a, b)).should.be.bignumber.equal(a.mul(b)); + await testCommutative(this.safeMath.mul, a, b, a.mul(b)); }); - it('handles a zero product correctly (first number as zero)', async function () { + it('multiplies by zero correctly', async function () { const a = new BN('0'); const b = new BN('5678'); - (await this.safeMath.mul(a, b)).should.be.bignumber.equal(a.mul(b)); - }); - - it('handles a zero product correctly (second number as zero)', async function () { - const a = new BN('5678'); - const b = new BN('0'); - - (await this.safeMath.mul(a, b)).should.be.bignumber.equal(a.mul(b)); + await testCommutative(this.safeMath.mul, a, b, '0'); }); it('reverts on multiplication overflow', async function () { const a = MAX_UINT256; const b = new BN('2'); - await shouldFail.reverting(this.safeMath.mul(a, b)); + await testFailsCommutative(this.safeMath.mul, a, b); }); }); @@ -92,7 +95,7 @@ contract('SafeMath', function () { (await this.safeMath.div(a, b)).should.be.bignumber.equal('1'); }); - it('reverts on zero division', async function () { + it('reverts on divison by zero', async function () { const a = new BN('5678'); const b = new BN('0');