Add tests to extend code coverage (#2586)

contracts/mocks/ERC721Mock.sol

@@ -9,18 +9,8 @@ import "../token/ERC721/ERC721.sol";
  * This mock just provides a public safeMint, mint, and burn functions for testing purposes
  */
 contract ERC721Mock is ERC721 {
-    string private _baseTokenURI;
-
     constructor (string memory name, string memory symbol) ERC721(name, symbol) { }
 
-    function _baseURI() internal view virtual override returns (string memory) {
-        return _baseTokenURI;
-    }
-
-    function setBaseURI(string calldata newBaseTokenURI) public {
-        _baseTokenURI = newBaseTokenURI;
-    }
-
     function baseURI() public view returns (string memory) {
         return _baseURI();
     }

test/governance/TimelockController.test.js

@@ -368,6 +368,36 @@ contract('TimelockController', function (accounts) {
           );
         });
 
+        it('length of batch parameter must match #1', async function () {
+          await expectRevert(
+            this.timelock.scheduleBatch(
+              this.operation.targets,
+              [],
+              this.operation.datas,
+              this.operation.predecessor,
+              this.operation.salt,
+              MINDELAY,
+              { from: proposer },
+            ),
+            'TimelockController: length mismatch',
+          );
+        });
+
+        it('length of batch parameter must match #1', async function () {
+          await expectRevert(
+            this.timelock.scheduleBatch(
+              this.operation.targets,
+              this.operation.values,
+              [],
+              this.operation.predecessor,
+              this.operation.salt,
+              MINDELAY,
+              { from: proposer },
+            ),
+            'TimelockController: length mismatch',
+          );
+        });
+
         it('prevent non-proposer from commiting', async function () {
           await expectRevert(
             this.timelock.scheduleBatch(
@@ -623,6 +653,13 @@ contract('TimelockController', function (accounts) {
         expectEvent(receipt, 'Cancelled', { id: this.operation.id });
       });
 
+      it('cannot cancel invalid operation', async function () {
+        await expectRevert(
+          this.timelock.cancel(constants.ZERO_BYTES32, { from: proposer }),
+          'TimelockController: operation cannot be cancelled',
+        );
+      });
+
       it('prevent non-proposer from canceling', async function () {
         await expectRevert(
           this.timelock.cancel(this.operation.id, { from: other }),

test/proxy/transparent/ProxyAdmin.test.js

@@ -35,6 +35,10 @@ contract('ProxyAdmin', function (accounts) {
       const admin = await this.proxyAdmin.getProxyAdmin(this.proxy.address);
       expect(admin).to.be.equal(this.proxyAdmin.address);
     });
+
+    it('call to invalid proxy', async function () {
+      await expectRevert.unspecified(this.proxyAdmin.getProxyAdmin(this.implementationV1.address));
+    });
   });
 
   describe('#changeProxyAdmin', function () {
@@ -56,6 +60,10 @@ contract('ProxyAdmin', function (accounts) {
       const implementationAddress = await this.proxyAdmin.getProxyImplementation(this.proxy.address);
       expect(implementationAddress).to.be.equal(this.implementationV1.address);
     });
+
+    it('call to invalid proxy', async function () {
+      await expectRevert.unspecified(this.proxyAdmin.getProxyImplementation(this.implementationV1.address));
+    });
   });
 
   describe('#upgrade', function () {

test/security/ReentrancyGuard.test.js

@@ -11,6 +11,12 @@ contract('ReentrancyGuard', function (accounts) {
     expect(await this.reentrancyMock.counter()).to.be.bignumber.equal('0');
   });
 
+  it('nonReentrant function can be called', async function () {
+    expect(await this.reentrancyMock.counter()).to.be.bignumber.equal('0');
+    await this.reentrancyMock.callback();
+    expect(await this.reentrancyMock.counter()).to.be.bignumber.equal('1');
+  });
+
   it('does not allow remote callback', async function () {
     const attacker = await ReentrancyAttack.new();
     await expectRevert(

test/token/ERC1155/presets/ERC1155PresetMinterPauser.test.js

@@ -120,6 +120,15 @@ contract('ERC1155PresetMinterPauser', function (accounts) {
         'ERC1155PresetMinterPauser: must have pauser role to pause',
       );
     });
+
+    it('other accounts cannot unpause', async function () {
+      await this.token.pause({ from: deployer });
+
+      await expectRevert(
+        this.token.unpause({ from: other }),
+        'ERC1155PresetMinterPauser: must have pauser role to unpause',
+      );
+    });
   });
 
   describe('burning', function () {

test/token/ERC20/presets/ERC20PresetMinterPauser.test.js

@@ -84,7 +84,19 @@ contract('ERC20PresetMinterPauser', function (accounts) {
     });
 
     it('other accounts cannot pause', async function () {
-      await expectRevert(this.token.pause({ from: other }), 'ERC20PresetMinterPauser: must have pauser role to pause');
+      await expectRevert(
+        this.token.pause({ from: other }),
+        'ERC20PresetMinterPauser: must have pauser role to pause',
+      );
+    });
+
+    it('other accounts cannot unpause', async function () {
+      await this.token.pause({ from: deployer });
+
+      await expectRevert(
+        this.token.unpause({ from: other }),
+        'ERC20PresetMinterPauser: must have pauser role to unpause',
+      );
     });
   });
 

test/token/ERC721/extensions/ERC721URIStorage.test.js

@@ -73,6 +73,15 @@ contract('ERC721URIStorage', function (accounts) {
       expect(await this.token.tokenURI(firstTokenId)).to.be.equal(baseURI + firstTokenId);
     });
 
+    it('tokens without URI can be burnt ', async function () {
+      await this.token.burn(firstTokenId, { from: owner });
+
+      expect(await this.token.exists(firstTokenId)).to.equal(false);
+      await expectRevert(
+        this.token.tokenURI(firstTokenId), 'ERC721URIStorage: URI query for nonexistent token',
+      );
+    });
+
     it('tokens with URI can be burnt ', async function () {
       await this.token.setTokenURI(firstTokenId, sampleUri);
 

test/token/ERC721/presets/ERC721PresetMinterPauserAutoId.test.js

@@ -97,6 +97,15 @@ contract('ERC721PresetMinterPauserAutoId', function (accounts) {
         'ERC721PresetMinterPauserAutoId: must have pauser role to pause',
       );
     });
+
+    it('other accounts cannot unpause', async function () {
+      await this.token.pause({ from: deployer });
+
+      await expectRevert(
+        this.token.unpause({ from: other }),
+        'ERC721PresetMinterPauserAutoId: must have pauser role to unpause',
+      );
+    });
   });
 
   describe('burning', function () {