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

Try subclasses while faking sealed classes #675

Open
fwemaere opened this issue Jul 13, 2023 · 13 comments
Open

Try subclasses while faking sealed classes #675

fwemaere opened this issue Jul 13, 2023 · 13 comments
Labels
contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@fwemaere
Copy link

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
Copy link
Contributor

yanok commented Jul 13, 2023

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 yanok added type-enhancement A request for a change that isn't a bug contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) P3 A lower priority bug or feature request labels Jul 13, 2023
@yanok yanok changed the title Sealed class compatibility Try subclasses while faking sealed classes Jul 13, 2023
@yanok
Copy link
Contributor

yanok commented Jul 17, 2023

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
Copy link

Any work around for this yet?

@yanok
Copy link
Contributor

yanok commented Aug 29, 2023

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
Copy link

Thanks @yanok

@martin-formigas
Copy link

@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
Copy link
Contributor

yanok commented Aug 29, 2023

@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
Copy link
Contributor

yanok commented Aug 29, 2023

@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
Copy link

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

@yanok
Copy link
Contributor

yanok commented Aug 29, 2023

There are two separate issues with sealed classes:

  • mocking them. That's not working and not going to be working because there is this fundamental incompatibility on the language level.
  • mocking non-sealed classes with methods that return sealed. This bug is about this case. Currently there is a workaround with provideDummy, we could potentially do it better (see Try subclasses while faking sealed classes #675 (comment)) and eventually this issue should be solved by changing an API in the next version, see Sketch plan for version 6.0.0 #684. I'd prefer to keep this bug open until then, so people could find it and see what the current state is, instead of filing new bugs.

@chiphan-makebetter
Copy link

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
Copy link
Contributor

yanok commented Aug 29, 2023

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

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

@chiphan-makebetter
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants