Smart Contract Tests failing

Smart Contracts
, ,
1

Here is my smart contract.

I am trying to test 2 functions and i have 3 test cases in place.

Here is my test file.

I keep getting the below fail/error message.

 1) should be correctly initialized
  2) should fail when called by a non-owner account
  3) should be able to mint a new token

  0 passing (989ms)
  3 failing

  1) should be correctly initialized:

      AssertionError: expected '0x00000000000000000000000000000000000…' to equal '0xa497A44ed66fB60249185Dc875daca9a321…'
      + expected - actual

      -0x0000000000000000000000000000000000000000
      +0xa497A44ed66fB60249185Dc875daca9a32145e60

      at Context.<anonymous> (test/ComNFT.js:19:22)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

  2) should fail when called by a non-owner account:

      AssertionError [ERR_ASSERTION]: Expected "onlyOwner" error message not found
      + expected - actual

      -false
      +true

      at Context.<anonymous> (test/ComNFT.js:36:7)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)

  3) should be able to mint a new token:
     TypeError: comNFT.methods.safeMint is not a function

I have tried several things, but it still keeps failing. Also, the test seems to be creating a different contract address/instance everytime it runs. Anybody have any ideas? Tried almost everything.

const assert = require("assert");
const { accounts } = require("@openzeppelin/test-environment");
const ComNFT = artifacts.require("ComNFT");
const { expect } = require("chai");

// describe('ComNFT', () => {
// let accounts;
let comNFT;

beforeEach(async () => {
  // accounts = await web3.eth.getAccounts();
  comNFT = await ComNFT.at("0xe29693019FDA487c7B6765a1109cC6E4031851E4");
  // console.log(comNFT.address);
});

  it("should be correctly initialized", async () => {
    // console.log(comNFT.address);
    const owner = await comNFT.owner();
    expect(owner).to.equal(accounts[0]);
    const name = await comNFT.name();
    expect(name).to.equal("ComNFT");
    const symbol = await comNFT.symbol();
    expect(symbol).to.equal("CMT");
  });
// });

  it('should fail when called by a non-owner account', async () => {
    try {
      await web3.eth.sendTransaction({
        from: accounts[1], // The non-owner account
        to: comNFT.address, // The contract address
        data: comNFT.methods.safeMint(accounts[1], 'token URI').encodeABI() // The function call and arguments, encoded as ABI
      });
      assert.fail('Expected error not thrown');
    } catch (error) {
      assert(error.message.includes('onlyOwner'), 'Expected "onlyOwner" error message not found');
    }
  });

  it('should be able to mint a new token', async () => {
    await web3.eth.sendTransaction({
      from: accounts[0], // The owner account
      to: comNFT.address, // The contract address
      data: comNFT.methods.safeMint(accounts[1], 'token URI').encodeABI() // The function call and arguments, encoded as ABI
    });

    const tokenURI = await comNFT.tokenURI(1); // Assume the token ID is 1
    assert.equal(tokenURI, 'token URI');
  });

// SPDX-License-Identifier: MIT
//pragma solidity 0.8.9;
pragma solidity 0.8.17;

import "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721URIStorageUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/utils/CountersUpgradeable.sol";

contract ComNFT is Initializable, ERC721Upgradeable, ERC721URIStorageUpgradeable, PausableUpgradeable, OwnableUpgradeable {
    using CountersUpgradeable for CountersUpgradeable.Counter;

    CountersUpgradeable.Counter private _tokenIdCounter;

    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() {
        _disableInitializers();
    }
    // internal onlyInitializing
    // initializer public
    function initialize() initializer public {
        //require(msg.sender == this.owner(), "Only the owner can call this function");
        __ERC721_init("ComNFT", "CMT");
        __ERC721URIStorage_init();
        __Pausable_init();
        __Ownable_init();
    }

    function pause() public onlyOwner {
        _pause();
    }

    function unpause() public onlyOwner {
        _unpause();
    }

    function safeMint(address to, string memory uri) public onlyOwner {
        uint256 tokenId = _tokenIdCounter.current();
        _tokenIdCounter.increment();
        _safeMint(to, tokenId);
        _setTokenURI(tokenId, uri);
    }

    function _beforeTokenTransfer(address from, address to, uint256 tokenId, uint256 batchSize) internal whenNotPaused override {
        super._beforeTokenTransfer(from, to, tokenId, batchSize);
    }


    // The following functions are overrides required by Solidity.

    function _burn(uint256 tokenId) internal override(ERC721Upgradeable, ERC721URIStorageUpgradeable) {
        super._burn(tokenId);
    }

    function tokenURI(uint256 tokenId) public view override(ERC721Upgradeable, ERC721URIStorageUpgradeable) returns (string memory) {
        return super.tokenURI(tokenId);
    }
}
2

Hey @Golanger, thanks for sharing the details.

I'm going test by test

  1. The first one is because the contract is deployed but not initialized. You need to run the initialize function first, it's not executed automatically (and remove the _disableInitializers())
    1.1 If you don't want to remove _disableInitializers(), consider testing your upgradeable contract using deployProxy and interacting with the proxy as stated in our docs.
  2. The safeMint method doesn't exist because the .methods key has them indexed by signature according to the docs.
    2.1. You'll need to find a way to correctly call safeMint, and once that's done correctly
    2.2. The error you should look for is the revert message in the OwnableUpgradeable contract, which is Ownable: caller is not the owner and doesn't contain the onlyOwner in the message.
  3. Is the same as above, however, test #3 is not catching the error that's caught in #2, I'm pretty sure that if you console.log(error.message) in #2 you'll find the same error as here. Therefore, solving #2 might help you solving #3

Also, the test seems to be creating a different contract address/instance everytime it runs.

This should be expected, the beforeEach block runs before every test, so, if you deploy in the beforeEach, you'll get different instances for each tests.

Another recommendation is that your attaching ComNFT to a specific address, but your tests should deploy in each run, since they're expected to be an isolated environment and not depending on previous conditions (here it's depending on ComNFT to be already deployed at 0xe29693019FDA487c7B6765a1109cC6E4031851E4)

Hope this helps.

3

Right, I would prefer to disable the Initializer once it has been called and run, so rather than testing if the contract initializes, i would need to implement a test regarding the proxy?

This should be expected, the beforeEach block runs before every test, so, if you deploy in the beforeEach, you'll get different instances for each tests.

Another recommendation is that your attaching ComNFT to a specific address, but your tests should deploy in each run, since they're expected to be an isolated environment and not depending on previous conditions (here it's depending on ComNFT to be already deployed at 0xe29693019FDA487c7B6765a1109cC6E4031851E4)

So instead of the example in the test i have given you, would i write something like this instead?

beforeEach(async () => {
accounts = await web3.eth.getAccounts();
comNFT = await ComNFT.new({ from: accounts[0] });

Will this help when addressing the below?

  1. The safeMint method doesn't exist because the .methods key has them indexed by signature according to the docs.
    2.1. You'll need to find a way to correctly call safeMint, and once that's done correctly
    2.2. The error you should look for is the revert message in the OwnableUpgradeable contract, which is Ownable: caller is not the owner and doesn't contain the onlyOwner in the message.
  2. Is the same as above, however, test #3 is not catching the error that's caught in #2, I'm pretty sure that if you console.log(error.message) in #2 you'll find the same error as here. Therefore, solving #2 might help you solving #3