Use Kotlin's regex API in MediaType #6962
Conversation
| * This test includes tests from [Guava's](https://code.google.com/p/guava-libraries/) | ||
| * MediaTypeTest. | ||
| */ | ||
| open class MediaTypeTest { |
There was a problem hiding this comment.
Do we lose any public API protection from moving these to kotlin? I thought we had some API only java tests, but I think we remove them?
There was a problem hiding this comment.
Mainly about the different preferred forms for kotlin and java. MediaType.parse(string) vs string.toMediaType()
There was a problem hiding this comment.
I don't think so. We had two strategies for API compatibility: japicmp (Java source and binary compatibility) and KotlinSourceModernTest (Kotlin source compatibility). These tests jobs are testing the behavior, not the API.
| @@ -71,6 +71,9 @@ object Dependencies { | |||
| const val kotlinReflect = "org.jetbrains.kotlin:kotlin-reflect:${Versions.kotlin}" | |||
| const val kotlinStdlibOsgi = "org.jetbrains.kotlin:kotlin-osgi-bundle:${Versions.kotlin}" | |||
| const val kotlinJunit5 = "org.jetbrains.kotlin:kotlin-test-junit5:${Versions.kotlin}" | |||
| const val kotlinTest = "org.jetbrains.kotlin:kotlin-test-common:${Versions.kotlin}" | |||
There was a problem hiding this comment.
You don't need all these dependencies anymore. Just kotlin-test on commonTest.
There was a problem hiding this comment.
I want that to be true but it didn't work for me on my first attempt. I'll try it again.
There was a problem hiding this comment.
It doesn’t work as-is. I might need to upgrade things? Later.
> Task :okhttp:compileTestKotlinJvm FAILED
e: /Users/jwilson/Projects/okhttp/okhttp/src/jvmTest/java/okhttp3/MediaTypeGetTest.kt: (18, 20): Unresolved reference: assertEquals
e: /Users/jwilson/Projects/okhttp/okhttp/src/jvmTest/java/okhttp3/MediaTypeGetTest.kt: (19, 20): Unresolved reference: assertFailsWith
e: /Users/jwilson/Projects/okhttp/okhttp/src/jvmTest/java/okhttp3/MediaTypeTest.kt: (21, 20): Unresolved reference: Test
| * MediaTypeTest. | ||
| */ | ||
| open class MediaTypeTest { | ||
| protected open fun parse(string: String): MediaType = string.toMediaType() |
There was a problem hiding this comment.
I think this should be string.toMediaTypeOrNull() and the return type MediaType?, since it was
protected MediaType parse(String string) {
return MediaType.parse(string);
}
while
@JvmStatic
@JvmName("get")
fun String.toMediaType(): MediaType {
and
@JvmStatic
@JvmName("parse")
fun String.toMediaTypeOrNull(): MediaType? {
There was a problem hiding this comment.
(I switched it cause I had to get rid of !! in the source.)
c9dee80
to
4e3af25
Compare
Preparing to promote this class to commonMain
4e3af25
to
6198a28
Compare
Preparing to promote this class to commonMain