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

Implementation of deprecated typed() member prevents migration #188

Open
seanburke-wf opened this issue Feb 14, 2019 · 12 comments
Open

Implementation of deprecated typed() member prevents migration #188

seanburke-wf opened this issue Feb 14, 2019 · 12 comments

Comments

@seanburke-wf
Copy link

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

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

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

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

seanburke-wf commented Feb 15, 2019

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

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

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

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

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

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

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

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants