Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate JSPackagerClientTest to Kotlin #37747

Closed

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
package com.facebook.react.packagerconnection

import com.facebook.react.packagerconnection.ReconnectingWebSocket.ConnectionCallback
import java.io.IOException
import okio.ByteString
import okio.ByteString.Companion.encodeUtf8
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.*
brunohensel marked this conversation as resolved.
Show resolved Hide resolved
import org.robolectric.RobolectricTestRunner

@RunWith(RobolectricTestRunner::class)
class JSPackagerClientTest {
private fun createRH(action: String, handler: RequestHandler): Map<String, RequestHandler> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are at this, let's call this createRequestHandler?

Plus let's move the private methods at the bottom after all the tests

Copy link
Contributor Author

@brunohensel brunohensel Jun 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I'm going to change some semantics here, I noticed that JSPackagerClient is always called passing in the same settings and clientId as test_client. Wdyt encoding this as default arguments to a function, passing in on the call site only the handler and connectionCallback? Like getClient(requestHandlers = handler, connectionCallback = connectionCallback)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup that's also a valid point for improvement 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just pushed the changes, let me know if I have to revert something.

val m: MutableMap<String, RequestHandler> = HashMap()
m[action] = handler
return m
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method should just become:

private fun createRequestHandler(action: String, handler: RequestHandler): Map<String, RequestHandler> = mapOf(action to handler)

we don't need explicit HashMap or so

}

private lateinit var mSettings: PackagerConnectionSettings
brunohensel marked this conversation as resolved.
Show resolved Hide resolved

@Before
fun setup() {
mSettings = mock(PackagerConnectionSettings::class.java)
`when`(mSettings.debugServerHost).thenReturn("ws://not_needed")
`when`(mSettings.packageName).thenReturn("my_test_package")
}

@Test
@Throws(IOException::class)
fun test_onMessage_ShouldTriggerNotification() {
val handler = mock(RequestHandler::class.java)
val client = JSPackagerClient("test_client", mSettings, createRH("methodValue", handler))

client.onMessage(
"{\"version\": 2, \"method\": \"methodValue\", \"params\": \"paramsValue\"}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use Kotlin multiline string here (i.e. """...""", then you won't need to escape the quotes?..

)
verify(handler).onNotification(eq("paramsValue"))
verify(handler, never()).onRequest(any(), any(Responder::class.java))
}

@Test
@Throws(IOException::class)
fun test_onMessage_ShouldTriggerRequest() {
val handler = mock(RequestHandler::class.java)
val client = JSPackagerClient("test_client", mSettings, createRH("methodValue", handler))

client.onMessage(
"{\"version\": 2, \"id\": \"idValue\", \"method\": \"methodValue\", \"params\": \"paramsValue\"}"
)
verify(handler, never()).onNotification(any())
verify(handler).onRequest(eq("paramsValue"), any(Responder::class.java))
}

@Test
@Throws(IOException::class)
fun test_onMessage_WithoutParams_ShouldTriggerNotification() {
val handler = mock(RequestHandler::class.java)
val client = JSPackagerClient("test_client", mSettings, createRH("methodValue", handler))

client.onMessage("{\"version\": 2, \"method\": \"methodValue\"}")
verify(handler).onNotification(eq<Any?>(null))
brunohensel marked this conversation as resolved.
Show resolved Hide resolved
verify(handler, never()).onRequest(any(), any(Responder::class.java))
}

@Test
@Throws(IOException::class)
fun test_onMessage_WithInvalidContentType_ShouldNotTriggerCallback() {
val handler = mock(RequestHandler::class.java)
val client = JSPackagerClient("test_client", mSettings, createRH("methodValue", handler))

client.onMessage("{\"version\": 2, \"method\": \"methodValue\"}".encodeUtf8())
verify(handler, never()).onNotification(any())
verify(handler, never()).onRequest(any(), any(Responder::class.java))
}

@Test
@Throws(IOException::class)
fun test_onMessage_WithoutMethod_ShouldNotTriggerCallback() {
val handler = mock(RequestHandler::class.java)
val client = JSPackagerClient("test_client", mSettings, createRH("methodValue", handler))

client.onMessage("{\"version\": 2}")
verify(handler, never()).onNotification(any())
verify(handler, never()).onRequest(any(), any(Responder::class.java))
}

@Test
@Throws(IOException::class)
fun test_onMessage_With_Null_Action_ShouldNotTriggerCallback() {
val handler = mock(RequestHandler::class.java)
val client = JSPackagerClient("test_client", mSettings, createRH("methodValue", handler))

client.onMessage("{\"version\": 2, \"method\": null}")
verify(handler, never()).onNotification(any())
verify(handler, never()).onRequest(any(), any(Responder::class.java))
}

@Test
@Throws(IOException::class)
fun test_onMessage_WithInvalidMethod_ShouldNotTriggerCallback() {
val handler = mock(RequestHandler::class.java)
val client = JSPackagerClient("test_client", mSettings, createRH("methodValue", handler))

client.onMessage(ByteString.EMPTY)
verify(handler, never()).onNotification(any())
verify(handler, never()).onRequest(any(), any(Responder::class.java))
}

@Test
@Throws(IOException::class)
fun test_onMessage_WrongVersion_ShouldNotTriggerCallback() {
val handler = mock(RequestHandler::class.java)
val client = JSPackagerClient("test_client", mSettings, createRH("methodValue", handler))

client.onMessage("{\"version\": 1, \"method\": \"methodValue\"}")
verify(handler, never()).onNotification(any())
verify(handler, never()).onRequest(any(), any(Responder::class.java))
}

@Test
@Throws(IOException::class)
fun test_onDisconnection_ShouldTriggerDisconnectionCallback() {
val connectionHandler = mock(ConnectionCallback::class.java)
val handler = mock(RequestHandler::class.java)
val client = JSPackagerClient("test_client", mSettings, HashMap(), connectionHandler)
brunohensel marked this conversation as resolved.
Show resolved Hide resolved

client.close()

verify(connectionHandler, never()).onConnected()
verify(connectionHandler, times(1)).onDisconnected()

verify(handler, never()).onNotification(any())
verify(handler, never()).onRequest(any(), any(Responder::class.java))
}
}