From 347f5021603fcfa3df2bf70aad5512360e496161 Mon Sep 17 00:00:00 2001 From: Fabian Zeiher Date: Sat, 16 Dec 2023 21:45:01 +0100 Subject: [PATCH] 100 percent testcoverage for tests Co-authored-by: IosifKoen --- .github/workflows/codecov.yml | 13 ------ contract/src/ItemBlocks.sol | 5 +-- contract/test/ItemBlocksTest.t.sol | 18 ++++++--- contract/test/TestCreatePassport.t.sol | 56 +++++++++++++++++++++++++- contract/test/TestGetters.t.sol | 17 +++++++- 5 files changed, 86 insertions(+), 23 deletions(-) delete mode 100644 .github/workflows/codecov.yml diff --git a/.github/workflows/codecov.yml b/.github/workflows/codecov.yml deleted file mode 100644 index 7d8fc6a..0000000 --- a/.github/workflows/codecov.yml +++ /dev/null @@ -1,13 +0,0 @@ -name: codecov -"on": - push: - branches: - - main -jobs: - uploader: - runs-on: ubuntu-latest - steps: - - name: Upload coverage reports to Codecov - uses: codecov/codecov-action@v3 - env: - CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/contract/src/ItemBlocks.sol b/contract/src/ItemBlocks.sol index e6ff1b2..a220cbb 100644 --- a/contract/src/ItemBlocks.sol +++ b/contract/src/ItemBlocks.sol @@ -40,7 +40,7 @@ contract ItemBlocks is ERC721, Ownable { } function updatePassport(uint tokenId, string calldata name, string calldata desc, string calldata family, string calldata url, string calldata img) public returns(uint256) { - require( isEligible(tokenId, msg.sender) == true, "Must be the owner of the item or the creator of it" ); + require( isEligible(tokenId, msg.sender), "Must be the owner of the item or the creator of it" ); itemPassports[tokenId] = Passport ({ name: name, @@ -67,8 +67,7 @@ contract ItemBlocks is ERC721, Ownable { // isEligible is a funtion that it takes the tokenId and userAddress and checks // if the user is eligible user for this item (tokenId) function isEligible(uint256 tokenId, address userAddress) public view returns(bool){ - if (userAddress == ownerOf(tokenId) || userAddress == allItemOwners[tokenId][0]) return true; - return false; + return (userAddress == ownerOf(tokenId) || userAddress == allItemOwners[tokenId][0]); } function setCreator(address creatorAddress, uint256 tokenId) internal { diff --git a/contract/test/ItemBlocksTest.t.sol b/contract/test/ItemBlocksTest.t.sol index 131c44c..95beb04 100644 --- a/contract/test/ItemBlocksTest.t.sol +++ b/contract/test/ItemBlocksTest.t.sol @@ -57,11 +57,6 @@ contract ItemBlocksTest is Test{ assertTrue(itemBlocks.isEligible(12, address(420))); } - // TESTS FOR updateOwnership - function testOwnershipShouldChange() public { - - } - // TESTS FOR setCreator // should place owner into the allItemOwners array function testSetCreator() public { @@ -74,6 +69,12 @@ contract ItemBlocksTest is Test{ itemBlocks.exposed_setCreator(address(0), 12); } + // should fail because zero address should not be allowed + function testUpdateSetCreatorZeroAddress() public { + vm.expectRevert(); + itemBlocks.exposed_setCreator(address(0), 12); + } + // should fail because token 13 should not be in the mapping function testFailUpdateSetCreatorNoToken() public { itemBlocks.exposed_setCreator(address(42), 12); @@ -86,6 +87,13 @@ contract ItemBlocksTest is Test{ itemBlocks.exposed_setCreator(address(44), 12); } + // should fail because token is already owned + function testSetCreatorAlreadyOwner() public { + itemBlocks.exposed_setCreator(address(42), 12); + vm.expectRevert(); + itemBlocks.exposed_setCreator(address(44), 12); + } + //TESTS for updateOwnership // should not allow transfer from zero address function testFailUpdateOwnershipFromZeroAddress() public { diff --git a/contract/test/TestCreatePassport.t.sol b/contract/test/TestCreatePassport.t.sol index 705fb44..33f43e2 100644 --- a/contract/test/TestCreatePassport.t.sol +++ b/contract/test/TestCreatePassport.t.sol @@ -51,7 +51,7 @@ contract ItemBlocksTest is Test{ itemBlocks.createPassport(7, testPassport.name, testPassport.desc, testPassport.family, testPassport.url, testPassport.img); } - function testUpdatePassportEligibility() public { + function testUpdatePassport() public { ItemBlocks.Passport memory testPassport = ItemBlocks.Passport({ name: "TestItemName", desc: "TestItemDesc", @@ -108,4 +108,58 @@ contract ItemBlocksTest is Test{ itemBlocks.updatePassport(tokenId, updatedPassport.name, updatedPassport.desc, updatedPassport.family, updatedPassport.url, updatedPassport.img); } + function testUpdatePassportNotEligibility() public { + ItemBlocks.Passport memory testPassport = ItemBlocks.Passport({ + name: "TestItemName", + desc: "TestItemDesc", + family: "TestItemFamily", + url: "TestItemUrl", + img: "TestItemImg" + }); + + vm.prank(address(42)); + uint256 tokenId = itemBlocks.createPassport(7, testPassport.name, testPassport.desc, testPassport.family, testPassport.url, testPassport.img); + + ItemBlocks.Passport memory updatedPassport = ItemBlocks.Passport({ + name: "UpdatedItemName", + desc: "UpdatedItemDesc", + family: "UpdatedItemFamily", + url: "UpdatedItemUrl", + img: "UpdatedItemImg" + }); + + //Not eligible user for passport updates. + vm.prank(address(40)); + vm.expectRevert(); + itemBlocks.updatePassport(tokenId, updatedPassport.name, updatedPassport.desc, updatedPassport.family, updatedPassport.url, updatedPassport.img); + } + + function testUpdatePassportEligibileCreator() public { + ItemBlocks.Passport memory testPassport = ItemBlocks.Passport({ + name: "TestItemName", + desc: "TestItemDesc", + family: "TestItemFamily", + url: "TestItemUrl", + img: "TestItemImg" + }); + + vm.prank(address(42)); + uint256 tokenId = itemBlocks.createPassport(7, testPassport.name, testPassport.desc, testPassport.family, testPassport.url, testPassport.img); + + vm.prank(address(42)); + itemBlocks.updateOwnership(address(42), address(43), 7); + + ItemBlocks.Passport memory updatedPassport = ItemBlocks.Passport({ + name: "UpdatedItemName", + desc: "UpdatedItemDesc", + family: "UpdatedItemFamily", + url: "UpdatedItemUrl", + img: "UpdatedItemImg" + }); + + //Eligible as the creator + vm.prank(address(42)); + itemBlocks.updatePassport(tokenId, updatedPassport.name, updatedPassport.desc, updatedPassport.family, updatedPassport.url, updatedPassport.img); + } + } \ No newline at end of file diff --git a/contract/test/TestGetters.t.sol b/contract/test/TestGetters.t.sol index 9de2e1a..c50cee9 100644 --- a/contract/test/TestGetters.t.sol +++ b/contract/test/TestGetters.t.sol @@ -40,7 +40,12 @@ contract ItemBlocksTest is Test { function testFailGetCreatedItemTokensZeroAddress() public view{ itemBlocks.getCreatedItemTokens(address(0)); } - + + // should fail for null address + function testGetCreatedItemTokensZeroAddress() public { + vm.expectRevert(); + itemBlocks.getCreatedItemTokens(address(0)); + } // should return list of users who owned this token function testGetUserHistory() public { assertEq(itemBlocks.getUserHistory(7)[0], address(42)); @@ -51,6 +56,11 @@ contract ItemBlocksTest is Test { itemBlocks.getUserHistory(1234); } + // should fail if token doesn't exits + function testUserHistoryZeroAddress() public { + vm.expectRevert(); + itemBlocks.getUserHistory(1234); + } // should return passport function testGetPassport() public { ItemBlocks.Passport memory p = itemBlocks.getPassport(7); @@ -66,4 +76,9 @@ contract ItemBlocksTest is Test { itemBlocks.getPassport(1234); } + // should fail if token doesn't exits + function testGetPassportNotToken() public { + vm.expectRevert(); + itemBlocks.getPassport(1234); + } }