defi-wonderland / smock

The Solidity mocking library
MIT License
319 stars 40 forks source link

bug: the order in which fake are declared messes with the test's behaviour #105

Open 0xng opened 2 years ago

0xng commented 2 years ago

Describe the bug When testing a function that uses fake.address and you have more than one fake declared/deployed, the behaviour of the tests changes according to the order in which these fakes are declared. For example, let's say we have fakeA and fakeB, and a test that makes function fakeA.myFunction() revert and expects for it to actually revert when called with ethers.provider.call({ to: fakeA.address }). If fakeA is declared after fakeB, the test will pass, but if fakeB is declared after fakeA the test will fail.

Reproduction steps I've pushed a branch that includes the failing test with this behaviour. In the beforeEach block of the text, you can switch the order in which the fakes are declared to see how the behaviour of the test changes.

  1. Go to this test on the feat/fake-test-at-address branch.
  2. Run yarn test. This test should fail.
  3. Switch the order in which fake and fakeAtAddress are declared in the beforeEach block
  4. Run yarn test. The test should work.

Expected behavior Test should pass despite the order in which the fakes are declared.

austinbv commented 2 years ago

This bug is actually more insidious than just the order of the functions

if you add this test

  it('not call other reverts', async () => {
    fake.fallback.reverts();
    try {
      await ethers.provider.call({to: fake.address})
    } catch {}

    expect(fakeAtAddress.fallback).not.to.have.been.called;
  });

The revert function actually references the same underlying object.