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

Flip to Dart 2.0-only syntax #85

Merged
merged 6 commits into from
Apr 17, 2018
Merged

Flip to Dart 2.0-only syntax #85

merged 6 commits into from
Apr 17, 2018

Conversation

srawlins
Copy link
Member

@srawlins srawlins commented Nov 16, 2017

TODO:

  • test internally
  • write upgrading-to-3.0.0.md

Summary:

  • The old typed system of storing argument matchers is now the only system 😄 typed goes away.
  • README is updated.
  • Breaking changes:
    • any, and captureAny cannot be used for named arguments; instead anyNamed() and captureAnyNamed() must be used ☹️ .
    • An argument matcher passed as a named argument must declare it's name. x: argThat(isNotNull) must be x: argThat(isNotNull, named: 'x'). ☹️
    • null cannot be passed as a named argument.
    • null cannot be passed as a positional argument when argument matchers are used in the same invocation.
  • Update tests. Add a few new tests.
  • typed is now a no-op: it returns it's first argument.

The upgrade path is not super obvious, so I want to write a upgrading-to-3.0.0.md document. Things like x: typed(any, named: 'x') becomes x: anyNamed('x'), etc.

@srawlins
Copy link
Member Author

Hi @matanlurey and @TedSander . No rush, but this is essentially ready for review. The code and tests are complete. I'm looking over the README though and want to write an upgrade path doc.

I'm also thinking I want to hold off on releasing for a few weeks/months while the void type stuff makes it into Dart dev releases, and maybe can make use of the planned new Invocation constructor and delete the _InvocationForMatchedArguments class.

Copy link
Contributor

@TedSander TedSander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it. Seems more straight forward to me.

README.md Outdated
## Named arguments

Mockito currently has an awkward nuisance to it's syntax: named arguments and
argument matchers require more specification than you might think. Namely, you
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we can do the 1000 foot explaination on this. Something like: This is because the language doesn't provide the name of the argument in the invocation and we can't rely on the position of a named argument.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. Done.

* master:
  Capture T in `verify`+ so T may be `void` in dart 2: `verify(m.void())` (#90)
  Tag the version as 3.0.0-alpha
  Bump version to 3.0.0 (#89)
  Throw an ArgumentError if `thenReturn` is called with futures or streams. (#88)
  Fix uses_dynamic_as_bottom error (#86)
  Remove Dev from the pubspec version
@srawlins
Copy link
Member Author

srawlins commented Feb 8, 2018

Rebased and tested on Dart 2.0.0-dev.19.0. Looks good.

@chalin
Copy link
Contributor

chalin commented Mar 1, 2018

This PR is happily approved by all reviewers. Might it be merged soon?

I've hit an issue with Angular 5 example app component tests under Dart 2-dev:

Type 'ArgMatcher' is not a subtype of type 'String'

because of

    var c = verify(mockRouter.navigate(captureAny, captureAny));

cc @kwalrath

@srawlins
Copy link
Member Author

srawlins commented Mar 1, 2018

I need to check off those two boxes at the top, but I hope to focus next week on Mockito and get 3.0.0 out the door.

For your issue above with DDC, you need the pre-3.0 typed API:

var c = verify(mockRouter.navigate(typed(captureAny), typed(captureAny)));

* master:
  Remove upper case constants (#113)
  Bump to 3.0.0-alpha+3 (#112)
  Switch back to Chrome for Travis (#104)
  Try using a staged, fancy travis config. (#100)
  Update travis script to actually run dartanalyzer (#102)
  verify*Interactions methods throw helpfully that they expect Mock (#92)
  First draft of upgrade guide for Mockito 3.0 (#96)
  Generic support for `thenReturn` and `thenAnswer` (#101)
  Remove references to `@proxy`. (#99)
  Remove Spy docs (#97)
  Remove mirrors implementation (#91)
@srawlins
Copy link
Member Author

OK this is looking good. I'm going to do a spot check internally, and then I'll land this to master.

@srawlins srawlins merged commit eed19f9 into master Apr 17, 2018
@srawlins srawlins deleted the all-is-typed branch April 17, 2018 18:07
mosuem pushed a commit to dart-lang/test that referenced this pull request Oct 17, 2024
* Flip to Dart 2.0-only syntax
* Touch up README and tests
* Change some error text
* @deprecate typed
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

Successfully merging this pull request may close these issues.

5 participants