It's been noted a couple times the 721 suite is lacking in some aspects: though the contracts have full line coverage, adding or modifying tests is somewhat cumbersome, and it's not easy to see which edge cases have been tested and which ones haven't. We should collect ideas on this topic before moving forward with the test files themselves.
Something I'd like to see added is some poking around possible edge cases for the implementation: IERC721Enumerable lends itself to multiple arrays and mappings, which require non-trivial techniques due to gas limitations (e.g. removal via swap-and-pop as opposed to memmove). These can be tricky to get right, and we should add explicit tests for scenarios such as removal of array with single element, removal of last element in array, etc. ArrayUtils may eventually come in handy for this.
Something that is severely lacking is tests for internal functions, the 721 ones are only being tested indirectly, which makes it very easy to introduce regression bugs during a refactor.
Just look at how many of these we have:
function _exists(uint256 tokenId) internal view returns (bool);
function _isApprovedOrOwner(address spender, uint256 tokenId) internal view returns (bool);
function _mint(address to, uint256 tokenId) internal;
function _burn(address owner, uint256 tokenId) internal;
function _burn(uint256 tokenId) internal;
function _transferFrom(address from, address to, uint256 tokenId) internal;
function _checkOnERC721Received(address from, address to, uint256 tokenId, bytes _data) internal returns (bool);
This should be fixed before 2.2 IMO. I’d also like to make a test run against previous versions with the upgraded suite.
I meant we may start using ArrayUtils to implement ERC721.
The missing internal tests are the first thing that come to mind. There's also some misleading stuff, like the presence of a creator account in some tests, where none exists (ERC721Mock allows anyone to mint).
It might be a good idea to write the testing guide (#1077) in parallel with the rewrite of this test suite. Most of the guidelines will come up in the process.
I would like to start writing a guide for testing. I’ve been trying to use the tests but there is a lot I don’t understand, I think it could be a good idea if I could write a guide at the same time I also understand how to do it. I would need some guidance but I really want to do it and learn this.
)