-
Notifications
You must be signed in to change notification settings - Fork 128
Update to Kotlin 1.3.11 and coroutines 1.0.1 #34
base: master
Are you sure you want to change the base?
Update to Kotlin 1.3.11 and coroutines 1.0.1 #34
Conversation
|
This is gonna be added to Retrofit, therefore I don't think this library will be updated. See the PR for that here: square/retrofit#2886 |
|
It will be updated but there's a few tests failures I saw locally. I'm assuming that's why CI failed |
|
Yeah, It's from deprecations being removed. I'm updating those now |
|
@JakeWharton So i migrated the It calls 1) But that test case is checking that the |
…on change in coroutines 0.27.0+
48cba19 to
d8ad9ca
Compare
| call.completeWithException(IOException()) | ||
| assertFalse(call.isCanceled) | ||
| assertTrue(deferred.isCompletedExceptionally) | ||
| assertTrue(deferred.isCancelled && deferred.isCompleted) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See Deferred in coroutines 0.27.0 for more details
| val call = CompletableCall<String>() | ||
| val deferred = adapter.adapt(call) | ||
| call.completeWithException(IOException()) | ||
| assertFalse(call.isCanceled) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The call is cancelled in coroutines 0.27.0+ because CompletedExceptionally is considered a "cancelled" state
|
It wouldn't be a test case if it wasn't something we needed to guarantee. We need alter the implementation to allow the test case to pass. A call should not be marked as canceled when it completes exceptionally. |
2a080a6 to
da744e7
Compare
|
|
da744e7 to
4b32e53
Compare
|
@JakeWharton that |
|
Subscribing here to give better visibility that everything is resolved and can be merged, @JakeWharton :) |
No description provided.