Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Captures use global vars, causing cross-talk between tests. Need a tearDown function #50

Closed
ochafik opened this issue Jan 17, 2017 · 6 comments

Comments

@ochafik
Copy link

ochafik commented Jan 17, 2017

See the following repro: captures are not hermetic across tests, even though I'm using different mock instances between tests!!!

import 'package:mockito/mockito.dart';
import 'package:test/test.dart';

class Foo {
  foo(x) {}
}
class MockFoo extends Mock implements Foo {}

main() {
  MockFoo foo;
  setUp(() {
    foo = new MockFoo();
    when(foo.foo(captureAny)).thenAnswer((i) => null);
  });

  test('calls', () {
    foo.foo(1);
  });
  test('calls but gets all tests\' captures!', () {
    foo.foo(2);
    expect(verify(foo.foo(any)).captured, [1, 2]);
  });
}

Note that some things do seem to reset captures:

  • the (advertised as testing-only) function resetMockitoState, which we'll be using as a temporary workaround.

  • Any test which call to verify fails to match any invocation:

    test('does not call', () {
      expect(() => verify(foo.foo(any)), throws);
    });

We'd really need something like:

  tearDown(() => mockitoTearDown());

(hit the bug with some real-life tests)

@ochafik
Copy link
Author

ochafik commented Jan 17, 2017

cc/ @JeromeA

@ochafik
Copy link
Author

ochafik commented Jan 17, 2017

Not sure, but could be the same bug as #45

@TedSander
Copy link
Contributor

So the invalid assumption here is that when you ask for a capture you will do a verify in the test. The first test does not do a verify, and so the capture state isn't reset.

Let me see if we can reset the capture state on the captureAny call. I'll also look into throwing an error if you don't do a verify when you asked for a capture, but I think that would be harder and maybe less developer friendly. There are competing ideas that mocks should error more and not be so lenient.

I'm not sure if we want to have the teardown function. Most of the time you shouldn't need it.

@ochafik
Copy link
Author

ochafik commented Jan 17, 2017

So actually the real core issue is mockito captures are extremely non-picky / broken: they just confuse captures between mock instances, even on different classes, even with methods of different names.

Repro:

import 'package:mockito/mockito.dart';
import 'package:test/test.dart';

class Foo1 {
  foo1(x) {}
}
class Foo2 {
  foo2(x) {}
}
class MockFoo1 extends Mock implements Foo1 {}
class MockFoo2 extends Mock implements Foo2 {}

main() {
  MockFoo1 foo1;
  MockFoo2 foo2;

  setUp(() {
    foo1 = new MockFoo1();
    foo2 = new MockFoo2();
    when(foo1.foo1(captureAny)).thenAnswer((i) => null);
    when(foo2.foo2(captureAny)).thenAnswer((i) => null);
  });

  test('wrong target', () {
    foo1.foo1(1);
    foo2.foo2(2);
    expect(verify(foo1.foo1(any)).captured, [1]); // Fails: actually gets [1, 2]
    expect(verify(foo2.foo2(any)).captured, [2]); // Fails: actually gets []
  });
}

@srawlins
Copy link
Member

Haha ahh! That is super broken. I think resetMockitoState is actually going to be public. It has real use cases. So that can go into your teardown.

However the other issue you raise about captures is super bad.

@srawlins
Copy link
Member

The above comment is correct. resetMockitoState is exported by package:mockito/mockito.dart, and in fact, it is used in each of Mockito's own tests' tearDown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants