rrousselGit / riverpod

A reactive caching and data-binding framework. Riverpod makes working with asynchronous code a breeze.
https://riverpod.dev
MIT License
6.2k stars 948 forks source link

Warn if a notifier mock doesn't extend a notifier base class #2374

Open rrousselGit opened 1 year ago

rrousselGit commented 1 year ago

Bad:

class ExampleMock extends Mock implements Example

Good:

class ExampleMock extends Notifier<T> with Mock implements Example
cc-nogueira commented 1 year ago

Hi Remi,

I am struggling with Mocking Notifiers. First because a Mockito generated Mock does not extends a Notifier base class, missing the required _setElement method.

I created a work-around but I will first make a question about your "Good" example above.

I had to let Mockito generate a mock for me and copy paste all mockito overrides. A lot of stuff:

class MockPlayerTimerNotifier extends FamilyNotifier<PlayerTimer, Player> with Mock implements PlayerTimerNotifier {
  @override
  PlayerTimer build(Player arg) {
    return PlayerTimer(player: arg);
  }

  @override
  PlayerTimer get state => (super.noSuchMethod(
        Invocation.getter(#state),
        returnValue: _FakePlayerTimer(
          this,
          Invocation.getter(#state),
        ),
      ) as PlayerTimer);

  @override
  set state(PlayerTimer? value) => super.noSuchMethod(
        Invocation.setter(
          #state,
          value,
        ),
        returnValueForMissingStub: null,
      );

  @override
  bool play() => (super.noSuchMethod(
        Invocation.method(
          #play,
          [],
        ),
        returnValue: false,
      ) as bool);

  // More methods omitted.
}

class _FakePlayerTimer extends SmartFake implements PlayerTimer {
  _FakePlayerTimer(
    Object parent,
    Invocation parentInvocation,
  ) : super(
          parent,
          parentInvocation,
        );
}

The good news is that it works! But required copy/paste from Mockito code generation.
After creating the ProviderContainer with overrideWithProvider I can use:

test('Play should invoke play on the family notifier for that player', () {
  final mock = container.read(playerTimerProvider(player1).notifier);
  when(mock.play()).thenReturn(true);

  useCase.play(player1);

  verify(mock.play());
  verifyNoMoreInteractions(mock);
});

Here the work-around I implemented before seeing this post.
I subclassed my Notifier (thus extending a base notifier) as a proxy for the generated mock. Ugly? Maybe, but no mingling with copy/paste.


final mockProxyPlayerTimerNotifierProvider =
  NotifierProvider.family<MockProxyPlayerTimerNotifier, PlayerTimer, Player>(MockProxyPlayerTimerNotifier.new);

class MockProxyPlayerTimerNotifier extends PlayerTimerNotifier {
  MockPlayerTimerNotifier mock = MockPlayerTimerNotifier();

  @override
  PlayerTimer get state => mock.state;

  @override
  set state(PlayerTimer value)  => mock.state = value;

  @override
  bool play() => mock.play();

  // More methods omitted.
}

The usage is only a single line different:

  final mock = container.read(playerTimerProvider(player1).notifier).asProxy.mock;

I hope that I am missing something on your solution, that there is a right way to support Mockito when without copy/paste.

Kind regards!

rrousselGit commented 1 year ago

Do not mock the state getter/setter

cc-nogueira commented 1 year ago

Yes sure! No need to mock protected members, it came for "free" from Mockito code generator. My bad, just need to mock the API (play, etc).

Then, would the "Good" implementation be written by hand like this?

class MockPlayerTimerNotifier extends FamilyNotifier<PlayerTimer, Player> with Mock implements PlayerTimerNotifier {
  @override
  PlayerTimer build(Player arg) {
    return PlayerTimer(player: arg);
  }

  @override
  bool play() => (super.noSuchMethod(
        Invocation.method(
          #play,
          [],
        ),
        returnValue: false,
      ) as bool);

  // More public methods omitted.
}
rrousselGit commented 1 year ago

Yes

rrousselGit commented 1 year ago

An alternative is https://github.com/rrousselGit/riverpod/issues/2596.

Although I'm not sure if that variant is worth the added class keyword necessary to define a notifier

imtoori commented 1 year ago

@rrousselGit I'm having an issue mocking a AutoDisposeFamilyAsyncNotifier generated by code-generation

the problem is that during the tests I get this error

type 'MockAlertGroupsController' is not a subtype of type 'AutoDisposeAsyncNotifier<AlertGroupsUiState>' in type cast

but this is my mock

class MockAlertGroupsController
    extends AutoDisposeFamilyAsyncNotifier<AlertGroupsUiState, AGListFilter?>
    with Mock
    implements AlertGroupsController {
  @override
  Future<AlertGroupsUiState> build(AGListFilter? filter) async {
    return const AlertGroupsUiState(alertGroups: []);
  }

  @override
  Future<void> getMoreAlertGroups() async {}
}

and I get the error whenever I call ref.watch/ref.read on the provider

rrousselGit commented 1 year ago

Generated notifiers need to extend the _$ base class


class MockAlertGroupsController
    extends _$AlertGroupsController
    with Mock
    implements AlertGroupsController
akvus commented 1 year ago

Generated notifiers need to extend the _$ base class

class MockAlertGroupsController
    extends _$AlertGroupsController
    with Mock
    implements AlertGroupsController

In such a case, should the generator generate a non-private version of _$AlertGroupsController? Mocks tend to be defined in the test directory and do not have access to the file's private generated classes. The workaround is to copy the generated code to the mock.

rrousselGit commented 1 year ago

I'd define mocks next to notifiers in this case.

To begin with, now that we have keywords like "sealed"& stuff, this practice makes sense.

akvus commented 1 year ago

Can you elaborate a little on how the sealed classes come in handy with placing mocks in the lib directory?

rrousselGit commented 1 year ago

It's not that they come in handy. But rather that keywords like sealed/final means you cannot have your mock in the test directory, as you'd get a compilation error.

janosgy commented 1 year ago

hi all, First, I'm not sure I got everything right, but this topic caused me some trouble. I'm leaving my solution here, in case you have suggestions or someone else needs it

class MockBookings extends AsyncNotifier<List<Booking>>
    with Mock
    implements Bookings {

  @override
  Future<List<Booking>> build() async {
    return super.noSuchMethod(
      Invocation.method(#build, []),
      returnValue: Future.value(<Booking>[]),
      returnValueForMissingStub: Future.value(<Booking>[]),
    );
  }
}

and in the test cases, I use (value depends on the test): when(mockBookings.build()).thenAnswer((_) async => allBookings);

yoiang commented 6 months ago

@rrousselGit to clarify when you say Mock you're referring to the mocks generated by mockito or some other one?