dart-lang / mockito

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

Implementation of deprecated `typed()` member prevents migration #188

Open seanburke-wf opened 5 years ago

seanburke-wf commented 5 years ago

When 3.x was released, typed() (which was now deprecated) was implemented as an arrow function returning null with Null as the return type in the signature. This means that any code which uses typed() will cease to function rather than providing a smooth transition, particularly if a dependency chain prevents moving unconditionally to 3.x or greater.

srawlins commented 5 years ago

Can you give an example? From the Mockito 2.2.2 README, this is how you would use typed:

when(cat.eatFood(typed(any)))

Mock's noSuchMethod receives the result of typed(any) (as you say, it is null), and drops it on the floor. What use of typed would cease to function?

seanburke-wf commented 5 years ago

When you say it drops it on the floor, will it continue to match appropriately? Additionally, if the named param of typed() is used (when(cat.eatFood(location: typed(any, named: 'location')))), the following is spit out: Invalid argument(s): An argument matcher (like `any`) was used as a named argument, but did not use a Mockito "named" API. Each argument matcher that is used as a named argument needs to specify the name of the argument it is being used in. For example: `when(obj.fn(x: anyNamed("x")))`.

srawlins commented 5 years ago

Yes, it's not really in the README, but when(cat.eatFood(typed(any))) will first "store" the any matcher on the side, and then Mock's noSuchMethod, when it sees the first argument is null, will fetch the stored argument matcher.

I'd need to see code that generates the error you see. It looks like you used named: correctly.

seanburke-wf commented 5 years ago

A bawdlerized version of the code follows:

  when(foo.createBar(id: typed(any as ArgMatcher, named: 'id')))
      .thenAnswer((invocation) => new Bar(
          id: invocation.namedArguments['id'].toString()));
srawlins commented 5 years ago

Ah, I see. I was pointing to the wrong "named" API. The Upgrading to Mockito 3 doc points to the following syntax, which is valid for the new API in Mockito 3:

when(obj.fn(foo: anyNamed('foo')))...

This is available as of Mockito 3.0.0-alpha+4, documented in the Upgrading doc as well.

seanburke-wf commented 5 years ago

This doesn't provide a smooth transition path. The package in question exports some of its mocks and so takes mockito as a full dependency. Downstream consumers therefore need to use a compatible version of mockito. It would be necessary to expand the version range to include 3.0.0-alpha+4 but make no changes to retain 2.x compatibility, expand the version range for all consumers, then change to the new API.

Is there any chance that anyNamed() could be backported to 2.x to allow us to use it while temporarily retaining 2.x compatibility?

srawlins commented 5 years ago

It looks like the y: typed(any, named: "y") style works in 3.0.0-alpha+4 as well. It least, it has tests for that:

https://github.com/dart-lang/mockito/blob/3.0.0-alpha%2B4/test/mockito_test.dart#L189

I don't have a copy of Dart 1 any more, so I can't execute the tests myself...

seanburke-wf commented 5 years ago

For the reasons stated above, the 3.0.0-alpha+4 transition path isn't a practical option for us due to dependency chains.

matanlurey commented 5 years ago

I think you might need to consider forking as a mockito-wf package for migration purposes. I can't see any effort being put into back-porting at this point (in fact Mockito might need to eventually change more to support non-nullability in the short-term).

srawlins commented 5 years ago

@seanburke-wf you stated above:

It would be necessary to expand the version range to include 3.0.0-alpha+4 but make no changes to retain 2.x compatibility, expand the version range for all consumers, then change to the new API.

Why would it not be practical to upgrade all packages to depend on 3.0.0-alpha+4, but it would be practical to upgrade all packages to some yet-to-be-written 2.x?

seanburke-wf commented 5 years ago

The packages in question all have some variant on mockito: ^2.0.0 already and can pull in updates there without change. Upgrading to 3.0.0-alpha+4 would require modifying deps for each package in the chain, then going back through and doing it again once the transition occurs. In the meantime, we can't work on transitioning those packages to support dart 2 because the repo this occurs in can't support dart 2 due to being locked to a version of mockito without dart 2 support.

seanburke-wf commented 5 years ago

A workaround has been found for the repository in question, which may obviate the need for addressing this upstream.