Issue #8319: Update to Kotlin 1.4 and Coroutines 1.3.9. #8359
Conversation
RelEng is currently trying to fix this. |
pocmo
force-pushed
the
kotlin-update
branch
4 times, most recently
from
1816d5e
to
ca3e255
Compare
pocmo
force-pushed
the
kotlin-update
branch
6 times, most recently
from
20ebc03
to
340a7aa
Compare
… Android lint 27.0.1.
pocmo
force-pushed
the
kotlin-update
branch
from
340a7aa
to
f664d89
Compare
| @@ -1,47 +0,0 @@ | |||
| /* This Source Code Form is subject to the terms of the Mozilla Public | |||
There was a problem hiding this comment.
We're removing these tests for the Kotlin update?
There was a problem hiding this comment.
At first I wanted to fix them since using a null value for a non-null variable in Kotlin now throws NullPointerException instead of IllegalStateException as before, ... then I noticed that they do not actually test anything:
What happens is that GeckoViewWebNotification is mocked (so everything returns null). Once we pass it into onShowNotification() the code will call toWebNotification() on the mock. This will throw since tag is not nullable (in WebNotification), but that's what the mock returns for tag. Then back in the test we catch that exception and do an assert on the message. So we do not really test our code at all. What we are testing is that the Kotlin compiler adds a null check with a specific message, a good test, but now for us? :)
There was a problem hiding this comment.
That sounds quite reasonable, thanks!
| @@ -235,6 +234,8 @@ internal class RustPushConnection( | |||
| ) | |||
|
|
|||
| return DecryptedMessage(scope, data) | |||
| } else { | |||
| return null | |||
There was a problem hiding this comment.
Interesting that the previous compiler didn't catch this! I wonder if it did some implicit return null.
jonalmeida
added
🛬 needs landing
pocmo
added
✏️ needs changes
|
Not ready yet. :) |
|
Oops! |
|
Looks like the last things left are some issues with Glean. The team currently has a work week. End of the week we want to look at getting the Gradle plugin working with Glean, maybe that will also fix issues here: #8360. |
|
Added that change to #8360. |
Let's see what taskcluster says. I am able to build sample browser locally with those changes.