-
Notifications
You must be signed in to change notification settings - Fork 130
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
@@ -36,7 +36,7 @@ class CancelTest { | |||
val deferred = adapter.adapt(call) | |||
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
@@ -36,7 +36,7 @@ class CancelTest { | |||
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.