Fixes tests and deprecation warnings for Dart2 #9
Conversation
|
Build failed on stable Dart 1.24. Not sure if the library expected to support 1.x. Looks like it targets Flutter which is all-in with Dart2. |
|
cc @yjbanov |
test/sentry_test.dart
Outdated
| when(httpMock.post(any, headers: any, body: any)) | ||
| .thenAnswer((Invocation invocation) { | ||
| httpMock.answerWith((Invocation invocation) { | ||
| if (invocation.memberName == #close) return null; |
There was a problem hiding this comment.
Can we assert after this line that memberName is #post? Also, let's use { } for the if block so that the return statement gets its own line.
test/sentry_test.dart
Outdated
| @@ -128,8 +127,7 @@ void main() { | |||
| final MockClient httpMock = new MockClient(); | |||
| final Clock fakeClock = new Clock.fixed(new DateTime(2017, 1, 2)); | |||
|
|
|||
| when(httpMock.post(any, headers: any, body: any)) | |||
| .thenAnswer((Invocation invocation) { | |||
| httpMock.answerWith((Invocation invocation) { | |||
There was a problem hiding this comment.
I think here we can also assert that only post is being called.
tool/dart2_test.sh
Outdated
| # Temporary workaround until Pub supports --preview-dart-2 flag | ||
| set -e | ||
| for filename in test/*_test.dart; do | ||
| dart --preview-dart-2 --enable_asserts "$filename" |
There was a problem hiding this comment.
Let's echo the name of the file we're running. Alternatively, set -x should have roughly the same effect.
|
Thanks for taking a look! All comments should be addressed now. |
|
This makes Dart 1 sad. Here's the plan. How about we do this PR first: #11. Then in this PR, we'll bump the version number to 2.0.0. From then on, if you are Dart 1, you have to use a version <2.0.0, and if you are on Dart 2, then you use version >=2.0.0. WDYT? |
|
Yeah, sgtm! Maybe also tag 1.0.0 so that it’s easier to backport any fixes, as a suggestion. |
|
I was about to tag it, then realized I forgot to update the changelog (#12) |
|
Ok, tagged as https:/flutter/sentry/releases/tag/1.0.0 |
|
So here's the TODO list for this PR:
|
|
Just pushed updates. |
|
Thanks for fixing this! LGTM |
No description provided.