From 0b489f4d79544ab9b870abe121077043feb971b8 Mon Sep 17 00:00:00 2001 From: dibi91 <34476725+dibi91@users.noreply.github.com> Date: Tue, 25 Aug 2020 19:58:45 +0200 Subject: [PATCH] Improve test descriptions #1157 (#2334) Co-authored-by: Paolo Dibitonto Co-authored-by: Francisco Giordano --- test/access/Ownable.test.js | 62 ++++---- test/cryptography/MerkleProof.test.js | 6 +- .../SupportsInterface.behavior.js | 6 +- test/payment/PaymentSplitter.test.js | 45 +++--- test/payment/PullPayment.test.js | 54 +++---- test/token/ERC1155/ERC1155.behavior.js | 10 +- .../ERC20/behaviors/ERC20Capped.behavior.js | 8 +- test/token/ERC721/ERC721.test.js | 16 +-- test/token/ERC721/ERC721Pausable.test.js | 2 +- test/utils/Address.test.js | 4 +- test/utils/Arrays.test.js | 136 +++++++++--------- test/utils/Counters.test.js | 56 ++++---- test/utils/Create2.test.js | 107 +++++++------- test/utils/EnumerableMap.test.js | 134 ++++++++--------- test/utils/EnumerableSet.behavior.js | 124 ++++++++-------- test/utils/ReentrancyGuard.test.js | 7 +- 16 files changed, 402 insertions(+), 375 deletions(-) diff --git a/test/access/Ownable.test.js b/test/access/Ownable.test.js index dce0756e7..b36cd4998 100644 --- a/test/access/Ownable.test.js +++ b/test/access/Ownable.test.js @@ -13,42 +13,46 @@ describe('Ownable', function () { this.ownable = await Ownable.new({ from: owner }); }); - it('should have an owner', async function () { + it('has an owner', async function () { expect(await this.ownable.owner()).to.equal(owner); }); - it('changes owner after transfer', async function () { - const receipt = await this.ownable.transferOwnership(other, { from: owner }); - expectEvent(receipt, 'OwnershipTransferred'); + describe('transfer ownership', function () { + it('changes owner after transfer', async function () { + const receipt = await this.ownable.transferOwnership(other, { from: owner }); + expectEvent(receipt, 'OwnershipTransferred'); - expect(await this.ownable.owner()).to.equal(other); + expect(await this.ownable.owner()).to.equal(other); + }); + + it('prevents non-owners from transferring', async function () { + await expectRevert( + this.ownable.transferOwnership(other, { from: other }), + 'Ownable: caller is not the owner' + ); + }); + + it('guards ownership against stuck state', async function () { + await expectRevert( + this.ownable.transferOwnership(ZERO_ADDRESS, { from: owner }), + 'Ownable: new owner is the zero address' + ); + }); }); - it('should prevent non-owners from transferring', async function () { - await expectRevert( - this.ownable.transferOwnership(other, { from: other }), - 'Ownable: caller is not the owner' - ); - }); + describe('renounce ownership', function () { + it('loses owner after renouncement', async function () { + const receipt = await this.ownable.renounceOwnership({ from: owner }); + expectEvent(receipt, 'OwnershipTransferred'); - it('should guard ownership against stuck state', async function () { - await expectRevert( - this.ownable.transferOwnership(ZERO_ADDRESS, { from: owner }), - 'Ownable: new owner is the zero address' - ); - }); + expect(await this.ownable.owner()).to.equal(ZERO_ADDRESS); + }); - it('loses owner after renouncement', async function () { - const receipt = await this.ownable.renounceOwnership({ from: owner }); - expectEvent(receipt, 'OwnershipTransferred'); - - expect(await this.ownable.owner()).to.equal(ZERO_ADDRESS); - }); - - it('should prevent non-owners from renouncement', async function () { - await expectRevert( - this.ownable.renounceOwnership({ from: other }), - 'Ownable: caller is not the owner' - ); + it('prevents non-owners from renouncement', async function () { + await expectRevert( + this.ownable.renounceOwnership({ from: other }), + 'Ownable: caller is not the owner' + ); + }); }); }); diff --git a/test/cryptography/MerkleProof.test.js b/test/cryptography/MerkleProof.test.js index cd1595a71..724864672 100644 --- a/test/cryptography/MerkleProof.test.js +++ b/test/cryptography/MerkleProof.test.js @@ -15,7 +15,7 @@ describe('MerkleProof', function () { }); describe('verify', function () { - it('should return true for a valid Merkle proof', async function () { + it('returns true for a valid Merkle proof', async function () { const elements = ['a', 'b', 'c', 'd']; const merkleTree = new MerkleTree(elements); @@ -28,7 +28,7 @@ describe('MerkleProof', function () { expect(await this.merkleProof.verify(proof, root, leaf)).to.equal(true); }); - it('should return false for an invalid Merkle proof', async function () { + it('returns false for an invalid Merkle proof', async function () { const correctElements = ['a', 'b', 'c']; const correctMerkleTree = new MerkleTree(correctElements); @@ -44,7 +44,7 @@ describe('MerkleProof', function () { expect(await this.merkleProof.verify(badProof, correctRoot, correctLeaf)).to.equal(false); }); - it('should return false for a Merkle proof of invalid length', async function () { + it('returns false for a Merkle proof of invalid length', async function () { const elements = ['a', 'b', 'c']; const merkleTree = new MerkleTree(elements); diff --git a/test/introspection/SupportsInterface.behavior.js b/test/introspection/SupportsInterface.behavior.js index 83b1511b0..a43ff2280 100644 --- a/test/introspection/SupportsInterface.behavior.js +++ b/test/introspection/SupportsInterface.behavior.js @@ -57,11 +57,11 @@ function shouldSupportInterfaces (interfaces = []) { const interfaceId = INTERFACE_IDS[k]; describe(k, function () { describe('ERC165\'s supportsInterface(bytes4)', function () { - it('should use less than 30k gas', async function () { + it('uses less than 30k gas', async function () { expect(await this.contractUnderTest.supportsInterface.estimateGas(interfaceId)).to.be.lte(30000); }); - it('should claim support', async function () { + it('claims support', async function () { expect(await this.contractUnderTest.supportsInterface(interfaceId)).to.equal(true); }); }); @@ -69,7 +69,7 @@ function shouldSupportInterfaces (interfaces = []) { for (const fnName of INTERFACES[k]) { const fnSig = FN_SIGNATURES[fnName]; describe(fnName, function () { - it('should be implemented', function () { + it('has to be implemented', function () { expect(this.contractUnderTest.abi.filter(fn => fn.signature === fnSig).length).to.equal(1); }); }); diff --git a/test/payment/PaymentSplitter.test.js b/test/payment/PaymentSplitter.test.js index da5111133..fca2a5393 100644 --- a/test/payment/PaymentSplitter.test.js +++ b/test/payment/PaymentSplitter.test.js @@ -54,45 +54,48 @@ describe('PaymentSplitter', function () { this.contract = await PaymentSplitter.new(this.payees, this.shares); }); - it('should have total shares', async function () { + it('has total shares', async function () { expect(await this.contract.totalShares()).to.be.bignumber.equal('100'); }); - it('should have payees', async function () { + it('has payees', async function () { await Promise.all(this.payees.map(async (payee, index) => { expect(await this.contract.payee(index)).to.equal(payee); expect(await this.contract.released(payee)).to.be.bignumber.equal('0'); })); }); - it('should accept payments', async function () { + it('accepts payments', async function () { await send.ether(owner, this.contract.address, amount); expect(await balance.current(this.contract.address)).to.be.bignumber.equal(amount); }); - it('should store shares if address is payee', async function () { - expect(await this.contract.shares(payee1)).to.be.bignumber.not.equal('0'); + describe('shares', async function () { + it('stores shares if address is payee', async function () { + expect(await this.contract.shares(payee1)).to.be.bignumber.not.equal('0'); + }); + + it('does not store shares if address is not payee', async function () { + expect(await this.contract.shares(nonpayee1)).to.be.bignumber.equal('0'); + }); }); - it('should not store shares if address is not payee', async function () { - expect(await this.contract.shares(nonpayee1)).to.be.bignumber.equal('0'); + describe('release', async function () { + it('reverts if no funds to claim', async function () { + await expectRevert(this.contract.release(payee1), + 'PaymentSplitter: account is not due payment' + ); + }); + it('reverts if non-payee want to claim', async function () { + await send.ether(payer1, this.contract.address, amount); + await expectRevert(this.contract.release(nonpayee1), + 'PaymentSplitter: account has no shares' + ); + }); }); - it('should throw if no funds to claim', async function () { - await expectRevert(this.contract.release(payee1), - 'PaymentSplitter: account is not due payment' - ); - }); - - it('should throw if non-payee want to claim', async function () { - await send.ether(payer1, this.contract.address, amount); - await expectRevert(this.contract.release(nonpayee1), - 'PaymentSplitter: account has no shares' - ); - }); - - it('should distribute funds to payees', async function () { + it('distributes funds to payees', async function () { await send.ether(payer1, this.contract.address, amount); // receive funds diff --git a/test/payment/PullPayment.test.js b/test/payment/PullPayment.test.js index 3c2634b18..f312d290d 100644 --- a/test/payment/PullPayment.test.js +++ b/test/payment/PullPayment.test.js @@ -15,35 +15,39 @@ describe('PullPayment', function () { this.contract = await PullPaymentMock.new({ value: amount }); }); - it('can record an async payment correctly', async function () { - await this.contract.callTransfer(payee1, 100, { from: payer }); - expect(await this.contract.payments(payee1)).to.be.bignumber.equal('100'); + describe('payments', function () { + it('can record an async payment correctly', async function () { + await this.contract.callTransfer(payee1, 100, { from: payer }); + expect(await this.contract.payments(payee1)).to.be.bignumber.equal('100'); + }); + + it('can add multiple balances on one account', async function () { + await this.contract.callTransfer(payee1, 200, { from: payer }); + await this.contract.callTransfer(payee1, 300, { from: payer }); + expect(await this.contract.payments(payee1)).to.be.bignumber.equal('500'); + }); + + it('can add balances on multiple accounts', async function () { + await this.contract.callTransfer(payee1, 200, { from: payer }); + await this.contract.callTransfer(payee2, 300, { from: payer }); + + expect(await this.contract.payments(payee1)).to.be.bignumber.equal('200'); + + expect(await this.contract.payments(payee2)).to.be.bignumber.equal('300'); + }); }); - it('can add multiple balances on one account', async function () { - await this.contract.callTransfer(payee1, 200, { from: payer }); - await this.contract.callTransfer(payee1, 300, { from: payer }); - expect(await this.contract.payments(payee1)).to.be.bignumber.equal('500'); - }); + describe('withdrawPayments', function () { + it('can withdraw payment', async function () { + const balanceTracker = await balance.tracker(payee1); - it('can add balances on multiple accounts', async function () { - await this.contract.callTransfer(payee1, 200, { from: payer }); - await this.contract.callTransfer(payee2, 300, { from: payer }); + await this.contract.callTransfer(payee1, amount, { from: payer }); + expect(await this.contract.payments(payee1)).to.be.bignumber.equal(amount); - expect(await this.contract.payments(payee1)).to.be.bignumber.equal('200'); + await this.contract.withdrawPayments(payee1); - expect(await this.contract.payments(payee2)).to.be.bignumber.equal('300'); - }); - - it('can withdraw payment', async function () { - const balanceTracker = await balance.tracker(payee1); - - await this.contract.callTransfer(payee1, amount, { from: payer }); - expect(await this.contract.payments(payee1)).to.be.bignumber.equal(amount); - - await this.contract.withdrawPayments(payee1); - - expect(await balanceTracker.delta()).to.be.bignumber.equal(amount); - expect(await this.contract.payments(payee1)).to.be.bignumber.equal('0'); + expect(await balanceTracker.delta()).to.be.bignumber.equal(amount); + expect(await this.contract.payments(payee1)).to.be.bignumber.equal('0'); + }); }); }); diff --git a/test/token/ERC1155/ERC1155.behavior.js b/test/token/ERC1155/ERC1155.behavior.js index 9401cdfa9..fd855bf27 100644 --- a/test/token/ERC1155/ERC1155.behavior.js +++ b/test/token/ERC1155/ERC1155.behavior.js @@ -356,7 +356,7 @@ function shouldBehaveLikeERC1155 ([minter, firstTokenHolder, secondTokenHolder, value: firstAmount, }); - it('should call onERC1155Received', async function () { + it('calls onERC1155Received', async function () { await expectEvent.inTransaction(this.transferReceipt.tx, ERC1155ReceiverMock, 'Received', { operator: multiTokenHolder, from: multiTokenHolder, @@ -389,7 +389,7 @@ function shouldBehaveLikeERC1155 ([minter, firstTokenHolder, secondTokenHolder, value: firstAmount, }); - it('should call onERC1155Received', async function () { + it('calls onERC1155Received', async function () { await expectEvent.inTransaction(this.transferReceipt.tx, ERC1155ReceiverMock, 'Received', { operator: multiTokenHolder, from: multiTokenHolder, @@ -632,7 +632,7 @@ function shouldBehaveLikeERC1155 ([minter, firstTokenHolder, secondTokenHolder, values: [firstAmount, secondAmount], }); - it('should call onERC1155BatchReceived', async function () { + it('calls onERC1155BatchReceived', async function () { await expectEvent.inTransaction(this.transferReceipt.tx, ERC1155ReceiverMock, 'BatchReceived', { operator: multiTokenHolder, from: multiTokenHolder, @@ -663,7 +663,7 @@ function shouldBehaveLikeERC1155 ([minter, firstTokenHolder, secondTokenHolder, values: [firstAmount, secondAmount], }); - it('should call onERC1155Received', async function () { + it('calls onERC1155Received', async function () { await expectEvent.inTransaction(this.transferReceipt.tx, ERC1155ReceiverMock, 'BatchReceived', { operator: multiTokenHolder, from: multiTokenHolder, @@ -741,7 +741,7 @@ function shouldBehaveLikeERC1155 ([minter, firstTokenHolder, secondTokenHolder, values: [firstAmount, secondAmount], }); - it('should call onERC1155BatchReceived', async function () { + it('calls onERC1155BatchReceived', async function () { await expectEvent.inTransaction(this.transferReceipt.tx, ERC1155ReceiverMock, 'BatchReceived', { operator: multiTokenHolder, from: multiTokenHolder, diff --git a/test/token/ERC20/behaviors/ERC20Capped.behavior.js b/test/token/ERC20/behaviors/ERC20Capped.behavior.js index 29a0a390f..4692f997f 100644 --- a/test/token/ERC20/behaviors/ERC20Capped.behavior.js +++ b/test/token/ERC20/behaviors/ERC20Capped.behavior.js @@ -6,21 +6,21 @@ function shouldBehaveLikeERC20Capped (minter, [other], cap) { describe('capped token', function () { const from = minter; - it('should start with the correct cap', async function () { + it('starts with the correct cap', async function () { expect(await this.token.cap()).to.be.bignumber.equal(cap); }); - it('should mint when amount is less than cap', async function () { + it('mints when amount is less than cap', async function () { await this.token.mint(other, cap.subn(1), { from }); expect(await this.token.totalSupply()).to.be.bignumber.equal(cap.subn(1)); }); - it('should fail to mint if the amount exceeds the cap', async function () { + it('fails to mint if the amount exceeds the cap', async function () { await this.token.mint(other, cap.subn(1), { from }); await expectRevert(this.token.mint(other, 2, { from }), 'ERC20Capped: cap exceeded'); }); - it('should fail to mint after cap is reached', async function () { + it('fails to mint after cap is reached', async function () { await this.token.mint(other, cap, { from }); await expectRevert(this.token.mint(other, 1, { from }), 'ERC20Capped: cap exceeded'); }); diff --git a/test/token/ERC721/ERC721.test.js b/test/token/ERC721/ERC721.test.js index 102853226..90827264b 100644 --- a/test/token/ERC721/ERC721.test.js +++ b/test/token/ERC721/ERC721.test.js @@ -332,7 +332,7 @@ describe('ERC721', function () { shouldTransferTokensByUsers(transferFun); - it('should call onERC721Received', async function () { + it('calls onERC721Received', async function () { const receipt = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: owner }); await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', { @@ -343,7 +343,7 @@ describe('ERC721', function () { }); }); - it('should call onERC721Received from approved', async function () { + it('calls onERC721Received from approved', async function () { const receipt = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: approved }); await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', { @@ -417,7 +417,7 @@ describe('ERC721', function () { const data = '0x42'; describe('via safeMint', function () { // regular minting is tested in ERC721Mintable.test.js and others - it('should call onERC721Received — with data', async function () { + it('calls onERC721Received — with data', async function () { this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, false); const receipt = await this.token.safeMint(this.receiver.address, tokenId, data); @@ -428,7 +428,7 @@ describe('ERC721', function () { }); }); - it('should call onERC721Received — without data', async function () { + it('calls onERC721Received — without data', async function () { this.receiver = await ERC721ReceiverMock.new(RECEIVER_MAGIC_VALUE, false); const receipt = await this.token.safeMint(this.receiver.address, tokenId); @@ -691,7 +691,7 @@ describe('ERC721', function () { await this.token.approve(approved, firstTokenId, { from: owner }); }); - it('should return approved account', async function () { + it('returns approved account', async function () { expect(await this.token.getApproved(firstTokenId)).to.be.equal(approved); }); }); @@ -752,7 +752,7 @@ describe('ERC721', function () { }); describe('tokenByIndex', function () { - it('should return all tokens', async function () { + it('returns all tokens', async function () { const tokensListed = await Promise.all( [0, 1].map(i => this.token.tokenByIndex(i)) ); @@ -760,14 +760,14 @@ describe('ERC721', function () { secondTokenId.toNumber()]); }); - it('should revert if index is greater than supply', async function () { + it('reverts if index is greater than supply', async function () { await expectRevert( this.token.tokenByIndex(2), 'EnumerableMap: index out of bounds' ); }); [firstTokenId, secondTokenId].forEach(function (tokenId) { - it(`should return all tokens after burning token ${tokenId} and minting new tokens`, async function () { + it(`returns all tokens after burning token ${tokenId} and minting new tokens`, async function () { const newTokenId = new BN(300); const anotherNewTokenId = new BN(400); diff --git a/test/token/ERC721/ERC721Pausable.test.js b/test/token/ERC721/ERC721Pausable.test.js index 659757ee7..b544ca37b 100644 --- a/test/token/ERC721/ERC721Pausable.test.js +++ b/test/token/ERC721/ERC721Pausable.test.js @@ -86,7 +86,7 @@ describe('ERC721Pausable', function () { }); describe('exists', function () { - it('should return token existence', async function () { + it('returns token existence', async function () { expect(await this.token.exists(firstTokenId)).to.equal(true); }); }); diff --git a/test/utils/Address.test.js b/test/utils/Address.test.js index 8c0844287..60e83c74d 100644 --- a/test/utils/Address.test.js +++ b/test/utils/Address.test.js @@ -15,11 +15,11 @@ describe('Address', function () { }); describe('isContract', function () { - it('should return false for account address', async function () { + it('returns false for account address', async function () { expect(await this.mock.isContract(other)).to.equal(false); }); - it('should return true for contract address', async function () { + it('returns true for contract address', async function () { const contract = await AddressImpl.new(); expect(await this.mock.isContract(contract.address)).to.equal(true); }); diff --git a/test/utils/Arrays.test.js b/test/utils/Arrays.test.js index 92c970d8c..42693aeb8 100644 --- a/test/utils/Arrays.test.js +++ b/test/utils/Arrays.test.js @@ -6,81 +6,83 @@ const { expect } = require('chai'); const ArraysImpl = contract.fromArtifact('ArraysImpl'); describe('Arrays', function () { - context('Even number of elements', function () { - const EVEN_ELEMENTS_ARRAY = [11, 12, 13, 14, 15, 16, 17, 18, 19, 20]; + describe('findUpperBound', function () { + context('Even number of elements', function () { + const EVEN_ELEMENTS_ARRAY = [11, 12, 13, 14, 15, 16, 17, 18, 19, 20]; - beforeEach(async function () { - this.arrays = await ArraysImpl.new(EVEN_ELEMENTS_ARRAY); + beforeEach(async function () { + this.arrays = await ArraysImpl.new(EVEN_ELEMENTS_ARRAY); + }); + + it('returns correct index for the basic case', async function () { + expect(await this.arrays.findUpperBound(16)).to.be.bignumber.equal('5'); + }); + + it('returns 0 for the first element', async function () { + expect(await this.arrays.findUpperBound(11)).to.be.bignumber.equal('0'); + }); + + it('returns index of the last element', async function () { + expect(await this.arrays.findUpperBound(20)).to.be.bignumber.equal('9'); + }); + + it('returns first index after last element if searched value is over the upper boundary', async function () { + expect(await this.arrays.findUpperBound(32)).to.be.bignumber.equal('10'); + }); + + it('returns 0 for the element under the lower boundary', async function () { + expect(await this.arrays.findUpperBound(2)).to.be.bignumber.equal('0'); + }); }); - it('should return correct index for the basic case', async function () { - expect(await this.arrays.findUpperBound(16)).to.be.bignumber.equal('5'); + context('Odd number of elements', function () { + const ODD_ELEMENTS_ARRAY = [11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21]; + + beforeEach(async function () { + this.arrays = await ArraysImpl.new(ODD_ELEMENTS_ARRAY); + }); + + it('returns correct index for the basic case', async function () { + expect(await this.arrays.findUpperBound(16)).to.be.bignumber.equal('5'); + }); + + it('returns 0 for the first element', async function () { + expect(await this.arrays.findUpperBound(11)).to.be.bignumber.equal('0'); + }); + + it('returns index of the last element', async function () { + expect(await this.arrays.findUpperBound(21)).to.be.bignumber.equal('10'); + }); + + it('returns first index after last element if searched value is over the upper boundary', async function () { + expect(await this.arrays.findUpperBound(32)).to.be.bignumber.equal('11'); + }); + + it('returns 0 for the element under the lower boundary', async function () { + expect(await this.arrays.findUpperBound(2)).to.be.bignumber.equal('0'); + }); }); - it('should return 0 for the first element', async function () { - expect(await this.arrays.findUpperBound(11)).to.be.bignumber.equal('0'); + context('Array with gap', function () { + const WITH_GAP_ARRAY = [11, 12, 13, 14, 15, 20, 21, 22, 23, 24]; + + beforeEach(async function () { + this.arrays = await ArraysImpl.new(WITH_GAP_ARRAY); + }); + + it('returns index of first element in next filled range', async function () { + expect(await this.arrays.findUpperBound(17)).to.be.bignumber.equal('5'); + }); }); - it('should return index of the last element', async function () { - expect(await this.arrays.findUpperBound(20)).to.be.bignumber.equal('9'); - }); + context('Empty array', function () { + beforeEach(async function () { + this.arrays = await ArraysImpl.new([]); + }); - it('should return first index after last element if searched value is over the upper boundary', async function () { - expect(await this.arrays.findUpperBound(32)).to.be.bignumber.equal('10'); - }); - - it('should return 0 for the element under the lower boundary', async function () { - expect(await this.arrays.findUpperBound(2)).to.be.bignumber.equal('0'); - }); - }); - - context('Odd number of elements', function () { - const ODD_ELEMENTS_ARRAY = [11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21]; - - beforeEach(async function () { - this.arrays = await ArraysImpl.new(ODD_ELEMENTS_ARRAY); - }); - - it('should return correct index for the basic case', async function () { - expect(await this.arrays.findUpperBound(16)).to.be.bignumber.equal('5'); - }); - - it('should return 0 for the first element', async function () { - expect(await this.arrays.findUpperBound(11)).to.be.bignumber.equal('0'); - }); - - it('should return index of the last element', async function () { - expect(await this.arrays.findUpperBound(21)).to.be.bignumber.equal('10'); - }); - - it('should return first index after last element if searched value is over the upper boundary', async function () { - expect(await this.arrays.findUpperBound(32)).to.be.bignumber.equal('11'); - }); - - it('should return 0 for the element under the lower boundary', async function () { - expect(await this.arrays.findUpperBound(2)).to.be.bignumber.equal('0'); - }); - }); - - context('Array with gap', function () { - const WITH_GAP_ARRAY = [11, 12, 13, 14, 15, 20, 21, 22, 23, 24]; - - beforeEach(async function () { - this.arrays = await ArraysImpl.new(WITH_GAP_ARRAY); - }); - - it('should return index of first element in next filled range', async function () { - expect(await this.arrays.findUpperBound(17)).to.be.bignumber.equal('5'); - }); - }); - - context('Empty array', function () { - beforeEach(async function () { - this.arrays = await ArraysImpl.new([]); - }); - - it('should always return 0 for empty array', async function () { - expect(await this.arrays.findUpperBound(10)).to.be.bignumber.equal('0'); + it('always returns 0 for empty array', async function () { + expect(await this.arrays.findUpperBound(10)).to.be.bignumber.equal('0'); + }); }); }); }); diff --git a/test/utils/Counters.test.js b/test/utils/Counters.test.js index cbfadc2d7..5650bab87 100644 --- a/test/utils/Counters.test.js +++ b/test/utils/Counters.test.js @@ -15,17 +15,19 @@ describe('Counters', function () { }); describe('increment', function () { - it('increments the current value by one', async function () { - await this.counter.increment(); - expect(await this.counter.current()).to.be.bignumber.equal('1'); - }); + context('starting from 0', function () { + it('increments the current value by one', async function () { + await this.counter.increment(); + expect(await this.counter.current()).to.be.bignumber.equal('1'); + }); - it('can be called multiple times', async function () { - await this.counter.increment(); - await this.counter.increment(); - await this.counter.increment(); + it('can be called multiple times', async function () { + await this.counter.increment(); + await this.counter.increment(); + await this.counter.increment(); - expect(await this.counter.current()).to.be.bignumber.equal('3'); + expect(await this.counter.current()).to.be.bignumber.equal('3'); + }); }); }); @@ -34,28 +36,30 @@ describe('Counters', function () { await this.counter.increment(); expect(await this.counter.current()).to.be.bignumber.equal('1'); }); + context('starting from 1', function () { + it('decrements the current value by one', async function () { + await this.counter.decrement(); + expect(await this.counter.current()).to.be.bignumber.equal('0'); + }); - it('decrements the current value by one', async function () { - await this.counter.decrement(); - expect(await this.counter.current()).to.be.bignumber.equal('0'); + it('reverts if the current value is 0', async function () { + await this.counter.decrement(); + await expectRevert(this.counter.decrement(), 'SafeMath: subtraction overflow'); + }); }); + context('after incremented to 3', function () { + it('can be called multiple times', async function () { + await this.counter.increment(); + await this.counter.increment(); - it('reverts if the current value is 0', async function () { - await this.counter.decrement(); - await expectRevert(this.counter.decrement(), 'SafeMath: subtraction overflow'); - }); + expect(await this.counter.current()).to.be.bignumber.equal('3'); - it('can be called multiple times', async function () { - await this.counter.increment(); - await this.counter.increment(); + await this.counter.decrement(); + await this.counter.decrement(); + await this.counter.decrement(); - expect(await this.counter.current()).to.be.bignumber.equal('3'); - - await this.counter.decrement(); - await this.counter.decrement(); - await this.counter.decrement(); - - expect(await this.counter.current()).to.be.bignumber.equal('0'); + expect(await this.counter.current()).to.be.bignumber.equal('0'); + }); }); }); }); diff --git a/test/utils/Create2.test.js b/test/utils/Create2.test.js index 347ad5e54..54b9b9938 100644 --- a/test/utils/Create2.test.js +++ b/test/utils/Create2.test.js @@ -23,71 +23,72 @@ describe('Create2', function () { beforeEach(async function () { this.factory = await Create2Impl.new(); }); + describe('computeAddress', function () { + it('computes the correct contract address', async function () { + const onChainComputed = await this.factory + .computeAddress(saltHex, web3.utils.keccak256(constructorByteCode)); + const offChainComputed = + computeCreate2Address(saltHex, constructorByteCode, this.factory.address); + expect(onChainComputed).to.equal(offChainComputed); + }); - it('should compute the correct contract address', async function () { - const onChainComputed = await this.factory - .computeAddress(saltHex, web3.utils.keccak256(constructorByteCode)); - const offChainComputed = - computeCreate2Address(saltHex, constructorByteCode, this.factory.address); - expect(onChainComputed).to.equal(offChainComputed); + it('computes the correct contract address with deployer', async function () { + const onChainComputed = await this.factory + .computeAddressWithDeployer(saltHex, web3.utils.keccak256(constructorByteCode), deployerAccount); + const offChainComputed = + computeCreate2Address(saltHex, constructorByteCode, deployerAccount); + expect(onChainComputed).to.equal(offChainComputed); + }); }); - it('should compute the correct contract address with deployer', async function () { - const onChainComputed = await this.factory - .computeAddressWithDeployer(saltHex, web3.utils.keccak256(constructorByteCode), deployerAccount); - const offChainComputed = - computeCreate2Address(saltHex, constructorByteCode, deployerAccount); - expect(onChainComputed).to.equal(offChainComputed); - }); + describe('deploy', function () { + it('deploys a ERC1820Implementer from inline assembly code', async function () { + const offChainComputed = + computeCreate2Address(saltHex, ERC1820Implementer.bytecode, this.factory.address); + await this.factory.deployERC1820Implementer(0, saltHex); + expect(ERC1820Implementer.bytecode).to.include((await web3.eth.getCode(offChainComputed)).slice(2)); + }); - it('should deploy a ERC1820Implementer from inline assembly code', async function () { - const offChainComputed = - computeCreate2Address(saltHex, ERC1820Implementer.bytecode, this.factory.address); + it('deploys a ERC20Mock with correct balances', async function () { + const offChainComputed = computeCreate2Address(saltHex, constructorByteCode, this.factory.address); - await this.factory.deployERC1820Implementer(0, saltHex); + await this.factory.deploy(0, saltHex, constructorByteCode); - expect(ERC1820Implementer.bytecode).to.include((await web3.eth.getCode(offChainComputed)).slice(2)); - }); + const erc20 = await ERC20Mock.at(offChainComputed); + expect(await erc20.balanceOf(deployerAccount)).to.be.bignumber.equal(new BN(100)); + }); - it('should deploy a ERC20Mock with correct balances', async function () { - const offChainComputed = computeCreate2Address(saltHex, constructorByteCode, this.factory.address); + it('deploys a contract with funds deposited in the factory', async function () { + const deposit = ether('2'); + await send.ether(deployerAccount, this.factory.address, deposit); + expect(await balance.current(this.factory.address)).to.be.bignumber.equal(deposit); - await this.factory.deploy(0, saltHex, constructorByteCode); + const onChainComputed = await this.factory + .computeAddressWithDeployer(saltHex, web3.utils.keccak256(constructorByteCode), this.factory.address); - const erc20 = await ERC20Mock.at(offChainComputed); - expect(await erc20.balanceOf(deployerAccount)).to.be.bignumber.equal(new BN(100)); - }); + await this.factory.deploy(deposit, saltHex, constructorByteCode); + expect(await balance.current(onChainComputed)).to.be.bignumber.equal(deposit); + }); - it('should deploy a contract with funds deposited in the factory', async function () { - const deposit = ether('2'); - await send.ether(deployerAccount, this.factory.address, deposit); - expect(await balance.current(this.factory.address)).to.be.bignumber.equal(deposit); + it('fails deploying a contract in an existent address', async function () { + await this.factory.deploy(0, saltHex, constructorByteCode, { from: deployerAccount }); + await expectRevert( + this.factory.deploy(0, saltHex, constructorByteCode, { from: deployerAccount }), 'Create2: Failed on deploy' + ); + }); - const onChainComputed = await this.factory - .computeAddressWithDeployer(saltHex, web3.utils.keccak256(constructorByteCode), this.factory.address); + it('fails deploying a contract if the bytecode length is zero', async function () { + await expectRevert( + this.factory.deploy(0, saltHex, '0x', { from: deployerAccount }), 'Create2: bytecode length is zero' + ); + }); - await this.factory.deploy(deposit, saltHex, constructorByteCode); - expect(await balance.current(onChainComputed)).to.be.bignumber.equal(deposit); - }); - - it('should failed deploying a contract in an existent address', async function () { - await this.factory.deploy(0, saltHex, constructorByteCode, { from: deployerAccount }); - await expectRevert( - this.factory.deploy(0, saltHex, constructorByteCode, { from: deployerAccount }), 'Create2: Failed on deploy' - ); - }); - - it('should fail deploying a contract if the bytecode length is zero', async function () { - await expectRevert( - this.factory.deploy(0, saltHex, '0x', { from: deployerAccount }), 'Create2: bytecode length is zero' - ); - }); - - it('should fail deploying a contract if factory contract does not have sufficient balance', async function () { - await expectRevert( - this.factory.deploy(1, saltHex, constructorByteCode, { from: deployerAccount }), - 'Create2: insufficient balance' - ); + it('fails deploying a contract if factory contract does not have sufficient balance', async function () { + await expectRevert( + this.factory.deploy(1, saltHex, constructorByteCode, { from: deployerAccount }), + 'Create2: insufficient balance' + ); + }); }); }); diff --git a/test/utils/EnumerableMap.test.js b/test/utils/EnumerableMap.test.js index 19e24c887..219fc6d5f 100644 --- a/test/utils/EnumerableMap.test.js +++ b/test/utils/EnumerableMap.test.js @@ -46,94 +46,98 @@ describe('EnumerableMap', function () { await expectMembersMatch(this.map, [], []); }); - it('adds a key', async function () { - const receipt = await this.map.set(keyA, accountA); - expectEvent(receipt, 'OperationResult', { result: true }); + describe('set', function () { + it('adds a key', async function () { + const receipt = await this.map.set(keyA, accountA); + expectEvent(receipt, 'OperationResult', { result: true }); - await expectMembersMatch(this.map, [keyA], [accountA]); + await expectMembersMatch(this.map, [keyA], [accountA]); + }); + + it('adds several keys', async function () { + await this.map.set(keyA, accountA); + await this.map.set(keyB, accountB); + + await expectMembersMatch(this.map, [keyA, keyB], [accountA, accountB]); + expect(await this.map.contains(keyC)).to.equal(false); + }); + + it('returns false when adding keys already in the set', async function () { + await this.map.set(keyA, accountA); + + const receipt = (await this.map.set(keyA, accountA)); + expectEvent(receipt, 'OperationResult', { result: false }); + + await expectMembersMatch(this.map, [keyA], [accountA]); + }); + + it('updates values for keys already in the set', async function () { + await this.map.set(keyA, accountA); + + await this.map.set(keyA, accountB); + + await expectMembersMatch(this.map, [keyA], [accountB]); + }); }); - it('adds several keys', async function () { - await this.map.set(keyA, accountA); - await this.map.set(keyB, accountB); + describe('remove', function () { + it('removes added keys', async function () { + await this.map.set(keyA, accountA); - await expectMembersMatch(this.map, [keyA, keyB], [accountA, accountB]); - expect(await this.map.contains(keyC)).to.equal(false); - }); + const receipt = await this.map.remove(keyA); + expectEvent(receipt, 'OperationResult', { result: true }); - it('returns false when adding keys already in the set', async function () { - await this.map.set(keyA, accountA); + expect(await this.map.contains(keyA)).to.equal(false); + await expectMembersMatch(this.map, [], []); + }); - const receipt = (await this.map.set(keyA, accountA)); - expectEvent(receipt, 'OperationResult', { result: false }); + it('returns false when removing keys not in the set', async function () { + const receipt = await this.map.remove(keyA); + expectEvent(receipt, 'OperationResult', { result: false }); - await expectMembersMatch(this.map, [keyA], [accountA]); - }); + expect(await this.map.contains(keyA)).to.equal(false); + }); - it('updates values for keys already in the set', async function () { - await this.map.set(keyA, accountA); + it('adds and removes multiple keys', async function () { + // [] - await this.map.set(keyA, accountB); + await this.map.set(keyA, accountA); + await this.map.set(keyC, accountC); - await expectMembersMatch(this.map, [keyA], [accountB]); - }); + // [A, C] - it('removes added keys', async function () { - await this.map.set(keyA, accountA); + await this.map.remove(keyA); + await this.map.remove(keyB); - const receipt = await this.map.remove(keyA); - expectEvent(receipt, 'OperationResult', { result: true }); + // [C] - expect(await this.map.contains(keyA)).to.equal(false); - await expectMembersMatch(this.map, [], []); - }); + await this.map.set(keyB, accountB); - it('returns false when removing keys not in the set', async function () { - const receipt = await this.map.remove(keyA); - expectEvent(receipt, 'OperationResult', { result: false }); + // [C, B] - expect(await this.map.contains(keyA)).to.equal(false); - }); + await this.map.set(keyA, accountA); + await this.map.remove(keyC); - it('adds and removes multiple keys', async function () { - // [] + // [A, B] - await this.map.set(keyA, accountA); - await this.map.set(keyC, accountC); + await this.map.set(keyA, accountA); + await this.map.set(keyB, accountB); - // [A, C] + // [A, B] - await this.map.remove(keyA); - await this.map.remove(keyB); + await this.map.set(keyC, accountC); + await this.map.remove(keyA); - // [C] + // [B, C] - await this.map.set(keyB, accountB); + await this.map.set(keyA, accountA); + await this.map.remove(keyB); - // [C, B] + // [A, C] - await this.map.set(keyA, accountA); - await this.map.remove(keyC); + await expectMembersMatch(this.map, [keyA, keyC], [accountA, accountC]); - // [A, B] - - await this.map.set(keyA, accountA); - await this.map.set(keyB, accountB); - - // [A, B] - - await this.map.set(keyC, accountC); - await this.map.remove(keyA); - - // [B, C] - - await this.map.set(keyA, accountA); - await this.map.remove(keyB); - - // [A, C] - - await expectMembersMatch(this.map, [keyA, keyC], [accountA, accountC]); - - expect(await this.map.contains(keyB)).to.equal(false); + expect(await this.map.contains(keyB)).to.equal(false); + }); }); }); diff --git a/test/utils/EnumerableSet.behavior.js b/test/utils/EnumerableSet.behavior.js index 7c4711c4e..747344046 100644 --- a/test/utils/EnumerableSet.behavior.js +++ b/test/utils/EnumerableSet.behavior.js @@ -23,91 +23,97 @@ function shouldBehaveLikeSet (valueA, valueB, valueC) { await expectMembersMatch(this.set, []); }); - it('adds a value', async function () { - const receipt = await this.set.add(valueA); - expectEvent(receipt, 'OperationResult', { result: true }); + describe('add', function () { + it('adds a value', async function () { + const receipt = await this.set.add(valueA); + expectEvent(receipt, 'OperationResult', { result: true }); - await expectMembersMatch(this.set, [valueA]); + await expectMembersMatch(this.set, [valueA]); + }); + + it('adds several values', async function () { + await this.set.add(valueA); + await this.set.add(valueB); + + await expectMembersMatch(this.set, [valueA, valueB]); + expect(await this.set.contains(valueC)).to.equal(false); + }); + + it('returns false when adding values already in the set', async function () { + await this.set.add(valueA); + + const receipt = (await this.set.add(valueA)); + expectEvent(receipt, 'OperationResult', { result: false }); + + await expectMembersMatch(this.set, [valueA]); + }); }); - it('adds several values', async function () { - await this.set.add(valueA); - await this.set.add(valueB); - - await expectMembersMatch(this.set, [valueA, valueB]); - expect(await this.set.contains(valueC)).to.equal(false); + describe('at', function () { + it('reverts when retrieving non-existent elements', async function () { + await expectRevert(this.set.at(0), 'EnumerableSet: index out of bounds'); + }); }); - it('returns false when adding values already in the set', async function () { - await this.set.add(valueA); + describe('remove', function () { + it('removes added values', async function () { + await this.set.add(valueA); - const receipt = (await this.set.add(valueA)); - expectEvent(receipt, 'OperationResult', { result: false }); + const receipt = await this.set.remove(valueA); + expectEvent(receipt, 'OperationResult', { result: true }); - await expectMembersMatch(this.set, [valueA]); - }); + expect(await this.set.contains(valueA)).to.equal(false); + await expectMembersMatch(this.set, []); + }); - it('reverts when retrieving non-existent elements', async function () { - await expectRevert(this.set.at(0), 'EnumerableSet: index out of bounds'); - }); + it('returns false when removing values not in the set', async function () { + const receipt = await this.set.remove(valueA); + expectEvent(receipt, 'OperationResult', { result: false }); - it('removes added values', async function () { - await this.set.add(valueA); + expect(await this.set.contains(valueA)).to.equal(false); + }); - const receipt = await this.set.remove(valueA); - expectEvent(receipt, 'OperationResult', { result: true }); + it('adds and removes multiple values', async function () { + // [] - expect(await this.set.contains(valueA)).to.equal(false); - await expectMembersMatch(this.set, []); - }); + await this.set.add(valueA); + await this.set.add(valueC); - it('returns false when removing values not in the set', async function () { - const receipt = await this.set.remove(valueA); - expectEvent(receipt, 'OperationResult', { result: false }); + // [A, C] - expect(await this.set.contains(valueA)).to.equal(false); - }); + await this.set.remove(valueA); + await this.set.remove(valueB); - it('adds and removes multiple values', async function () { - // [] + // [C] - await this.set.add(valueA); - await this.set.add(valueC); + await this.set.add(valueB); - // [A, C] + // [C, B] - await this.set.remove(valueA); - await this.set.remove(valueB); + await this.set.add(valueA); + await this.set.remove(valueC); - // [C] + // [A, B] - await this.set.add(valueB); + await this.set.add(valueA); + await this.set.add(valueB); - // [C, B] + // [A, B] - await this.set.add(valueA); - await this.set.remove(valueC); + await this.set.add(valueC); + await this.set.remove(valueA); - // [A, B] + // [B, C] - await this.set.add(valueA); - await this.set.add(valueB); + await this.set.add(valueA); + await this.set.remove(valueB); - // [A, B] + // [A, C] - await this.set.add(valueC); - await this.set.remove(valueA); + await expectMembersMatch(this.set, [valueA, valueC]); - // [B, C] - - await this.set.add(valueA); - await this.set.remove(valueB); - - // [A, C] - - await expectMembersMatch(this.set, [valueA, valueC]); - - expect(await this.set.contains(valueB)).to.equal(false); + expect(await this.set.contains(valueB)).to.equal(false); + }); }); } diff --git a/test/utils/ReentrancyGuard.test.js b/test/utils/ReentrancyGuard.test.js index b40d29a2c..7ba5e8663 100644 --- a/test/utils/ReentrancyGuard.test.js +++ b/test/utils/ReentrancyGuard.test.js @@ -12,7 +12,7 @@ describe('ReentrancyGuard', function () { expect(await this.reentrancyMock.counter()).to.be.bignumber.equal('0'); }); - it('should not allow remote callback', async function () { + it('does not allow remote callback', async function () { const attacker = await ReentrancyAttack.new(); await expectRevert( this.reentrancyMock.countAndCall(attacker.address), 'ReentrancyAttack: failed call'); @@ -21,14 +21,13 @@ describe('ReentrancyGuard', function () { // The following are more side-effects than intended behavior: // I put them here as documentation, and to monitor any changes // in the side-effects. - - it('should not allow local recursion', async function () { + it('does not allow local recursion', async function () { await expectRevert( this.reentrancyMock.countLocalRecursive(10), 'ReentrancyGuard: reentrant call' ); }); - it('should not allow indirect local recursion', async function () { + it('does not allow indirect local recursion', async function () { await expectRevert( this.reentrancyMock.countThisRecursive(10), 'ReentrancyMock: failed call' );