dart-lang / mockito

Mockito-inspired mock library for Dart
https://pub.dev/packages/mockito
Apache License 2.0
623 stars 160 forks source link

Try subclasses while faking `sealed` classes #675

Open fwemaere opened 11 months ago

fwemaere commented 11 months ago

I have some trouble when using sealed class.

This is a sample :

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

// Annotation which generates the cat.mocks.dart library and the MockCat class.
@GenerateNiceMocks([MockSpec<PetsRepo>()])
import 'sample.mocks.dart';

sealed class Pet {}

class Cat implements Pet {
  const Cat();
}

class Dog implements Pet {
  const Dog();
}

// Real class
class PetsRepo {
  Pet getPet(int id) => const Dog();
}

void main() {
  // Create mock object.
  var petRepo = MockPetsRepo();

  Pet cat = Cat();

  when(petRepo.getPet(1)).thenReturn(cat);
  // ....
}

I have this error

`This means Mockito was not smart enough to generate a dummy value of type 'Pet'. Please consider using either 'provideDummy' or 'provideDummyBuilder' functions to give Mockito a proper dummy value.

Please note that due to implementation details Mockito sometimes needs users to provide dummy values for some types, even if they plan to explicitly stub all the called methods.`

I have to add provideDummy(cat); before when. If i do this it works.

It would be nice if sealed class was compatible without provide dummy value !

yanok commented 11 months ago

Yes, that would be nice and I already had this improvement in my head. But that's a low priority for us currently. Contributions are welcome.

The idea how to implement this would be quite simple: during code generation, if we have to create a fake of the sealed class S we could iterate through the subclasses of S and if some of them are not base or final, we could make fake S implement that subclass instead of S.

yanok commented 11 months ago

Thought a bit more about it. I think that's totally fine and will be an actual improvement, to pass a fake of a child class as returnValue argument of Mock.noSuchMethod: it is only used while stubbing and that value is always thrown away.

I wouldn't pass the same fake value as returnValueForMissingStub though, since that one is used for nice mocks, and picking one of the options will silently pick a code path that will always be executed. We likely don't want to make it uncontrollably.

chiphan-makebetter commented 10 months ago

Any work around for this yet?

yanok commented 10 months ago

provideDummy/provideDummyBuilder is a workaround. Eventually planned version 6 will allow mocking classes that have methods with sealed argument and/or return types (but not sealed classes themselves!) directly.

I don't have any timeline for version 6 though.

chiphan-makebetter commented 10 months ago

Thanks @yanok

martin-formigas commented 10 months ago

@yanok can you maybe provide some more details on the decision process? I think it's a big decision to not support a newly added core feature of the Dart language.

yanok commented 10 months ago

@martin-formigas Well, sealed classes are mutually incompatible with mocks, so we don't have much choice.

Consider that you have

sealed class Pet {}
class Cat extends Pet {}
class Dog extends Pet {}

void speak(Pet pet) => switch (pet) {
  Cat() => print('Meow'),
  Dog() => print('Woof')
  // Note that you don't need a "catch all" clause here, since Dart can see you exhausted all the possibilities.
}

Now imagine you managed to create a MockPet somehow, which is a subtype of Pet (but not a subtype of Cat or Dog). That means you can pass it to speak function, but it can't handle MockPet since there is no branch for it in switch, so it would be a runtime error. Fortunately, the language doesn't allow you to create MockPet, exactly for this reason.

You can mock Dog or Cat instead and this will work.

yanok commented 10 months ago

@martin-formigas so the decision process was actually on the language side: the language team wanted to get exhaustiveness for sealed classes (which is generally good), they did realize that it will break mocking and they decided exhaustiveness is more important.

We just have to follow here.

If you want my personal opinion, I'd do the same, but I wasn't involved in the process.

martin-formigas commented 10 months ago

Got it, thanks for the insight! I think this issue can be closed then, since there will never be an actual resolution?

yanok commented 10 months ago

There are two separate issues with sealed classes:

chiphan-makebetter commented 10 months ago

Hi @yanok I ran into one problem that if I have a StreamController that have a sealed Event how can I provideDummy for it

      final events = StreamController<Event>();
      when(mockService.events).thenAnswer((_) => events.stream);
yanok commented 10 months ago

provideDummy<Event>(/* an instance of Event */);

You might be able to drop the <Event> part if an instance static type is Event.

chiphan-makebetter commented 10 months ago

Thanks @yanok but that's not what I mean. I will inspect about this and get back to you later