Sketch plan for version 6.0.0

[yanok]

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 like

// generated file
class MockXConfig {
  // all Xs methods (including inherited ones):
  Expectation<int> m(Matcher<int> v);
  // ...
}

class MockX extends Mock<MockXConfig> implements X {
  // doesn't have to override anything.
  MockX() : super(MockXConfig()) {}
}

and make when return a configuration object for a mock. So stubbing becomes

when(mock).m(any).thenReturn(1);

instead of

when(mock.m(any)).thenReturn(1);

Personally I like the existing syntax more. But this allow us to overcome a number of problems:

1. No overrides;
2. => No need to revive default argument values.
3. No need to came up with return values;
4. => No need for dummies for stubbing (will still need them for nice mocks though, but I think nice mocks are ok to only be "best effort nice".
5. I didn't put a lot of thought into it yet, but we likely could do something reasonable for private types as well.

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).

[frederikstonge]

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.

[yanok]

Yes, with this change we will only need dummies for nice mocks, not for stubbing.

[yanok]

On the bad side, to the best of my knowledge there is no way to make Matcher<int> also accept ints 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.

[yanok]

@frederikstonge I don't quite follow your comments, are you commenting on the right bug?

[frederikstonge]

Mybad

[yanok]

Meanwhile, adding one more pro for the change of the API:

• We could probably get rid of internal state (and problems with messing it, see Trying to set an expectation on unsupported method messes up Mockito internal state. #551): currently matchers could only return 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.

[yanok]

On the bad side, to the best of my knowledge there is no way to make Matcher<int> also accept ints 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 dart-lang/language#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.

[srawlins]

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.

[yanok]

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.

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.

[chiphan-makebetter]

Can I ask is there a task to update for the sealed class to work?

[yanok]

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.