mockito icon indicating copy to clipboard operation
mockito copied to clipboard

Implementation of deprecated `typed()` member prevents migration

Open seanburke-wf opened this issue 6 years ago • 12 comments

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.

seanburke-wf avatar Feb 14 '19 19:02 seanburke-wf

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?

srawlins avatar Feb 14 '19 21:02 srawlins

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

seanburke-wf avatar Feb 14 '19 21:02 seanburke-wf

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.

srawlins avatar Feb 14 '19 23:02 srawlins

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()));

seanburke-wf avatar Feb 15 '19 00:02 seanburke-wf

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.

srawlins avatar Feb 15 '19 00:02 srawlins

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?

seanburke-wf avatar Feb 15 '19 16:02 seanburke-wf

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

srawlins avatar Feb 15 '19 17:02 srawlins

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

seanburke-wf avatar Feb 15 '19 18:02 seanburke-wf

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

matanlurey avatar Feb 15 '19 18:02 matanlurey

@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?

srawlins avatar Feb 15 '19 19:02 srawlins

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 avatar Feb 15 '19 21:02 seanburke-wf

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

seanburke-wf avatar Feb 15 '19 21:02 seanburke-wf