OpenZeppelin / openzeppelin-test-helpers

Assertion library for Ethereum smart contract testing
https://docs.openzeppelin.com/test-helpers
MIT License
416 stars 132 forks source link

Cannot test with expectRevert #134

Open herman-rogers opened 3 years ago

herman-rogers commented 3 years ago

So I have some contract testing code that basically has:

        it('does something', async () => {
            await expectRevert(
                contract.doSomething(
                    mockToken.address,
                    100,
                    { from: holder }),
                'Expected Error'
            );
        });

Which works fine. However, when I add tests below that:

        it('does something', async () => {
            await expectRevert(
                contract.doSomething(
                    mockToken.address,
                    100,
                    { from: holder }),
                'Expected Error'
            );
        });

        it('can get name', async () => {
            const name = await contract.name(owner);
            expect(name).toEqual('Mock Name');
        });

All tests below the expectRevert fail. If I re-order the test such as:

        it('can get name', async () => {
            const name = await contract.name(owner);
            expect(name).toEqual('Mock Name');
        });

        it('does something', async () => {
            await expectRevert(
                contract.doSomething(
                    mockToken.address,
                    100,
                    { from: holder }),
                'Expected Error'
            );
        });

Both tests pass fine. So it appears the order matters. If I order it "incorrectly" I get Error: Error: read ECONNRESET which seems irrelevant to my contracts and something to do with the underlying code.

frangio commented 3 years ago

Hi @herman-rogers. This is a weird issue, and your snippets look good to me, but I don't know what are exactly your contract and mockToken variables. Can you provide a more complete setup for us to test this? Is your repo public?

zach-is-my-name commented 3 years ago

I'm having a similar issue as herman. Only my subsequent tests fail a bit differently (screenshot). I will come back with a full reproduction, but here's a snippet for now: Screenshot from 2020-11-01 22-56-16

     context('disburse on accept', function() {
        try {
        beforeEach(async function(){ 
          this.ownerBalanceBeforeReturn = await this.tokenSystem.balanceOf(owner);
          this.ownerBalanceBeforeBondReturn = await this.tokenSystem.balanceOf(owner, {from: owner}); 
          this.suggesterBalanceBeforeBondReturn = await this.tokenSystem.balanceOf(suggester)
          await this.proxiedEscrow.disburseOnAccept(id, {from: owner});
          this.suggesterBalanceAfterBondReturn = await this.tokenSystem.balanceOf(suggester)
          this.amountProtectedSuggester = await this.tokenSystem.amountProtected(suggester)  
          this.rewardAmount = await this.proxiedEscrow.rewardAmount()
          this.amountProtectedOwner = await this.tokenSystem.amountProtected(owner)
          this.ownerBondAmount =  await this.proxiedEscrow.ownerBondAmount()
          console.log('this.ownerBondAmount',this.ownerBondAmount.toString())
          console.log('this.amountProtectedOwner ',this.amountProtectedOwner.toString())
        })
         } catch(error){  new Error(error)} 

<<<<CULPRIT>>>>
  it('reverts when suggester attempts to transfer tokens under protection', async function() {
          try {
          await expectRevert(this.tokenSystem.transfer(owner, this.amountProtectedSuggester, {from: suggester}), "your tokens are under protection period, check timeToLiftProtection() for time until you have tokens available, and/or check amountProtected to see how many of your tokens are currently under the protection period") ;
          } catch (error) { new Error(error)}
        }) 

<<<<FAILS HERE, BUT ARGS LOG IN beforeEach() >>>>
        it('protects owner tokens', function() {
            try { 
          expect(new BN(this.amountProtectedOwner)).to.be.bignumber.equal(new BN(this.ownerBondAmount))
          } catch (error) { new Error(error)}
        })
abcoathup commented 3 years ago

Hi @herman-rogers ,

Unfortunately I wasn't able to reproduce the issue that you were experiencing.

I used the following contract and test on Windows Subsystem for Linux (WSL2)

$ npx truffle version
Truffle v5.1.53 (core: 5.1.53)
Solidity - 0.6.12 (solc-js)
Node v10.22.1
Web3.js v1.2.9

MyContract.sol

// SPDX-License-Identifier: MIT
pragma solidity ^0.6.0;

contract MyContract {

  address private _creator;
  event DoneStuff();
  uint256 public value;

  constructor() public {
    _creator = msg.sender;
    value = 42;
  }

  function doStuff() public {
    require(_creator == msg.sender, "MyContract: not creator");
    emit DoneStuff();
  }
}

MyContract.test.js

// test/MyContract.test.js
// Load dependencies
const { expect } = require('chai');

// Import utilities from Test Helpers
const { BN, expectEvent, expectRevert } = require('@openzeppelin/test-helpers');

// Load compiled artifacts
const MyContract = artifacts.require('MyContract');

// Start test block
contract('MyContract', function ([ creator, other ]) {

  beforeEach(async function () {
    this.myContract = await MyContract.new({ from: creator });
  });

  it('it reverts when attempting to do stuff from other account', async function () {
    // Test a transaction reverts
    await expectRevert(
      this.myContract.doStuff({ from: other }),
      'MyContract: not creator'
    );
  });

  it('it emits event when doing stuff from creator', async function () {
    const receipt = await this.myContract.doStuff({ from: creator });

    expectEvent(receipt, 'DoneStuff');
  });

  it('retrieve value', async function () {
    // Use large integers ('big numbers')
    const value = new BN('42');

    // Use large integer comparisons
    expect(await this.myContract.value()).to.be.bignumber.equal(value);
  });
});

Test

$ npx truffle test

Compiling your contracts...
===========================
> Compiling ./contracts/Migrations.sol
> Compiling ./contracts/MyContract.sol
> Artifacts written to /tmp/test--8050-F6RKfzyCbsGF
> Compiled successfully using:
   - solc: 0.6.12+commit.27d51765.Emscripten.clang

  Contract: MyContract
    ✓ it reverts when attempting to do stuff from other account (136ms)
    ✓ it emits event when doing stuff from creator (103ms)
    ✓ retrieve value

  3 passing (644ms)