Skip to content

fix: operator == takes Object parameter #710

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

Merged
merged 2 commits into from
Apr 12, 2021

Conversation

markj
Copy link

@markj markj commented Apr 12, 2021

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Update to remove analyser warning when using mockito

⤵️ What is the current behavior?

When flutter analyzer runs with sound null safety enabled and a class using Position is mocked you receive an error like:

  error • 'Object.==' ('bool Function(Object)') isn't a valid concrete implementation of 'Position.==' ('bool Function(dynamic)') • test/unit-tests/geolocation/geolocation_service_test.mocks.dart:16:7 • invalid_implementation_override

🆕 What is the new behavior (if this is a feature change)?

The analyzer is happy as the operator definition matches the correct parameters.

💥 Does this PR introduce a breaking change?

No

🐛 Recommendations for testing

No other changes are required so all existing tests should succeed to verify behaviour has not changed.

📝 Links to relevant issues/docs

dart-lang/mockito#354

https://api.dart.dev/stable/2.12.2/dart-core/Object/operator_equals.html

🤔 Checklist before submitting

  • I made sure all projects build.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I followed the style guide lines (code style guide).
  • I updated the relevant documentation.
  • I rebased onto current master.

@codecov
Copy link

codecov bot commented Apr 12, 2021

Codecov Report

Merging #710 (d7d7f27) into master (881e77d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##            master      #710    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            1        15    +14     
  Lines           21       227   +206     
==========================================
+ Hits            21       227   +206     
Impacted Files Coverage Δ
...or_platform_interface/lib/src/models/position.dart 100.00% <ø> (ø)
...face/lib/src/errors/position_update_exception.dart 100.00% <0.00%> (ø)
...m_interface/lib/src/enums/location_permission.dart 100.00% <0.00%> (ø)
...ce/lib/src/errors/permission_denied_exception.dart 100.00% <0.00%> (ø)
...terface/lib/src/extensions/integer_extensions.dart 100.00% <0.00%> (ø)
...src/implementations/method_channel_geolocator.dart 100.00% <0.00%> (ø)
...orm_interface/lib/src/enums/location_accuracy.dart 100.00% <0.00%> (ø)
...orm_interface/lib/src/models/location_options.dart 100.00% <0.00%> (ø)
...e/lib/src/errors/already_subscribed_exception.dart 100.00% <0.00%> (ø)
...e/lib/src/errors/invalid_permission_exception.dart 100.00% <0.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e67130...d7d7f27. Read the comment docs.

@mvanbeusekom
Copy link
Member

@markj thank you very much for the fix. Would you mind updating the version number in de pubspec.yaml and add an entry to the CHANGELOG.md as well (see also the "Checklist before submitting")?

@markj
Copy link
Author

markj commented Apr 12, 2021

Sorry.. I had bumped it but hadn't put it up.. That's done now.

Copy link
Member

@mvanbeusekom mvanbeusekom left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for this contribution.

@mvanbeusekom mvanbeusekom merged commit 4011af5 into Baseflow:master Apr 12, 2021
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.

2 participants