Open yanok opened 11 months ago
I'm currently facing an issue with dummy values where my ChopperClient mock throws an exception on when
with this :
This means Mockito was not smart enough to generate a dummy value of type
'Response<AuthorizationModel>'. 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.
This is caused by their new update using dart 3 class modifiers (final/sealed).
Your point 4 says that we won't need dummies for stubbing, which makes me believe that it will fix my issue, since I am providing the expected return value and not relying on dummies.
Yes, with this change we will only need dummies for nice mocks, not for stubbing.
On the bad side, to the best of my knowledge there is no way to make Matcher<int>
also accept int
s in Dart.
So we would either have to change all the code that set expectations using values, like
when(o.doSmth(1)).thenReturn(2);
will become
when(o).doSmth(equals(1)).thenReturn(2);
or make all MockXConfig
methods' arguments dynamic
and do run-time type checks, like some other packages do.
@frederikstonge I don't quite follow your comments, are you commenting on the right bug?
Mybad
Meanwhile, adding one more pro for the change of the API:
null
, so we have to do black magic with remembering which matchers were called and then rebuilding the invocation matcher. With Matcher<T>
as an argument we could completely drop this and just pass the matchers directly as arguments. Bonus: no more need for *Named
matchers.On the bad side, to the best of my knowledge there is no way to make
Matcher<int>
also acceptint
s in Dart.
Thinking more about this, I don't think it's a big deal: we can always just accept dynamic
and do type check at run-time. And to make it better, there are two new language features currently discussed: implicit constructors https://github.com/dart-lang/language/issues/108 and union types https://github.com/dart-lang/language/blob/main/working/union-types/nominative-union-types.md, any of them would allow us to return to stricter types.
I started doing some experiments with macros meanwhile.
Personally I like the existing syntax more. But this allow us to overcome a number of problems:
I agree, the syntax is essentially the same, and the problems of the existing codegen can be headaches for users. I think the new syntax is probably worth it.
we might want to target macros directly
If it is feasible, I think this would be a good route.
Thinking more about this, I don't think it's a big deal: we can always just accept dynamic and do type check at run-time.
Yeah I think this isn't terrible. If we get analyzer plugins off the ground, you can imagine a fairly simple lint check which enforces "static type must be int
or Matcher<int>
."
Can the migration to the new syntax be done incrementally? (i.e. how will google3 be migrated?) Could introduce the new when
API as something like when2
temporarily.
Can the migration to the new syntax be done incrementally? (i.e. how will google3 be migrated?) Could introduce the new
when
API as something likewhen2
temporarily.
I think we have to, migrating everything in one CL won't fit into Piper limits. I was thinking about either making a new library, such that migrated code would import 'package:mockito/mockito6.dart' or even forking it in a completely new directory, like //third_party/dart/mockito6. The latter option is more hard core but I think we should probably be fine with the first one.
Can I ask is there a task to update for the sealed class
to work?
Can I ask is there a task to update for the
sealed class
to work?
You have to be more precise. Mocking sealed classes won't work and that is not going to change. Mocking classes with methods that have sealed classes as arguments/return types will be fixed by the changes proposed in this bug.
I guess the main thing is to split the mock object itself from the mock configuration object. That would mean a breaking API change, but the fix will be trivial and easily automated.
Currently we use the same object both to set expectations and as a mock. That bites us in many places, because a mock must be a subclass of the class it mocks, but for the configuration object the mocked class signatures might be too tight.
So, for a
class X extends Y { int m(int v); ...}
we will have something likeand make
when
return a configuration object for a mock. So stubbing becomesinstead of
Personally I like the existing syntax more. But this allow us to overcome a number of problems:
No sure if it makes a lot of sense to re-write the existing codegen though, we might want to target macros directly (still have to investigate if it's even feasible to re-implement Mockito with macros though).