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

[ECO-5014] feat: basic sandbox setup #47

Merged
merged 1 commit into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions chat-android/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ dependencies {
testImplementation(libs.junit)
testImplementation(libs.mockk)
testImplementation(libs.coroutine.test)
testImplementation(libs.bundles.ktor.client)
androidTestImplementation(libs.androidx.test.core)
androidTestImplementation(libs.androidx.test.runner)
androidTestImplementation(libs.androidx.junit)
Expand Down
61 changes: 61 additions & 0 deletions chat-android/src/test/java/com/ably/chat/Sandbox.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package com.ably.chat

import com.google.gson.JsonElement
import com.google.gson.JsonParser
import io.ably.lib.realtime.AblyRealtime
import io.ktor.client.HttpClient
import io.ktor.client.engine.cio.CIO
import io.ktor.client.plugins.HttpRequestRetry
import io.ktor.client.request.get
import io.ktor.client.request.post
import io.ktor.client.request.setBody
import io.ktor.client.statement.HttpResponse
import io.ktor.client.statement.bodyAsText
import io.ktor.http.ContentType
import io.ktor.http.contentType

val client = HttpClient(CIO) {
install(HttpRequestRetry) {
retryOnServerErrors(maxRetries = 4)
exponentialDelay()
}
}

class Sandbox private constructor(val appId: String, val apiKey: String) {
companion object {
suspend fun createInstance(): Sandbox {
val response: HttpResponse = client.post("https://sandbox-rest.ably.io/apps") {
contentType(ContentType.Application.Json)
setBody(loadAppCreationRequestBody().toString())
}
Comment on lines +27 to +30
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Make sandbox URL configurable

The sandbox URL should be configurable to support different environments (e.g., local development as suggested in past reviews).

+    companion object {
+        private const val DEFAULT_SANDBOX_URL = "https://sandbox-rest.ably.io"
+        private val SANDBOX_URL = System.getenv("SANDBOX_URL") ?: DEFAULT_SANDBOX_URL
+    }

-            val response: HttpResponse = client.post("https://sandbox-rest.ably.io/apps") {
+            val response: HttpResponse = client.post("$SANDBOX_URL/apps") {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
val response: HttpResponse = client.post("https://sandbox-rest.ably.io/apps") {
contentType(ContentType.Application.Json)
setBody(loadAppCreationRequestBody().toString())
}
companion object {
private const val DEFAULT_SANDBOX_URL = "https://sandbox-rest.ably.io"
private val SANDBOX_URL = System.getenv("SANDBOX_URL") ?: DEFAULT_SANDBOX_URL
}
val response: HttpResponse = client.post("$SANDBOX_URL/apps") {
contentType(ContentType.Application.Json)
setBody(loadAppCreationRequestBody().toString())
}

val body = JsonParser.parseString(response.bodyAsText())

return Sandbox(
appId = body.asJsonObject["appId"].asString,
// From JS chat repo at 7985ab7 — "The key we need to use is the one at index 5, which gives enough permissions to interact with Chat and Channels"
apiKey = body.asJsonObject["keys"].asJsonArray[5].asJsonObject["keyStr"].asString,
)
}
}
}

internal fun Sandbox.createSandboxChatClient(): DefaultChatClient {
val realtime = createSandboxRealtime(apiKey)
return DefaultChatClient(realtime, ClientOptions())
}

internal fun Sandbox.createSandboxRealtime(chatClientId: String = "sandbox-client"): AblyRealtime =
AblyRealtime(
io.ably.lib.types.ClientOptions().apply {
key = apiKey
environment = "sandbox"
clientId = chatClientId
},
)
Comment on lines +47 to +54
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Generate unique client IDs for parallel test safety

Using a fixed default client ID could cause issues in parallel test execution.

-internal fun Sandbox.createSandboxRealtime(chatClientId: String = "sandbox-client"): AblyRealtime =
+internal fun Sandbox.createSandboxRealtime(
+    chatClientId: String = "sandbox-client-${System.currentTimeMillis()}-${Random.nextInt(1000)}"
+): AblyRealtime =
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
internal fun Sandbox.createSandboxRealtime(chatClientId: String = "sandbox-client"): AblyRealtime =
AblyRealtime(
io.ably.lib.types.ClientOptions().apply {
key = apiKey
environment = "sandbox"
clientId = chatClientId
},
)
internal fun Sandbox.createSandboxRealtime(
chatClientId: String = "sandbox-client-${System.currentTimeMillis()}-${Random.nextInt(1000)}"
): AblyRealtime =
AblyRealtime(
io.ably.lib.types.ClientOptions().apply {
key = apiKey
environment = "sandbox"
clientId = chatClientId
},
)


private suspend fun loadAppCreationRequestBody(): JsonElement =
JsonParser.parseString(
client.get("https://raw.githubusercontent.com/ably/ably-common/refs/heads/main/test-resources/test-app-setup.json") {
contentType(ContentType.Application.Json)
}.bodyAsText(),
).asJsonObject.get("post_apps")
Comment on lines +56 to +61
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add timeout for configuration loading

Configuration loading from external URL should have a reasonable timeout to prevent tests from hanging.

+private const val CONFIG_TIMEOUT_MS = 5000L

 private suspend fun loadAppCreationRequestBody(): JsonElement =
     JsonParser.parseString(
         client.get("https://raw.githubusercontent.com/ably/ably-common/refs/heads/main/test-resources/test-app-setup.json") {
+            timeout {
+                requestTimeoutMillis = CONFIG_TIMEOUT_MS
+            }
             contentType(ContentType.Application.Json)
         }.bodyAsText(),
     ).asJsonObject.get("post_apps")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private suspend fun loadAppCreationRequestBody(): JsonElement =
JsonParser.parseString(
client.get("https://raw.githubusercontent.com/ably/ably-common/refs/heads/main/test-resources/test-app-setup.json") {
contentType(ContentType.Application.Json)
}.bodyAsText(),
).asJsonObject.get("post_apps")
private const val CONFIG_TIMEOUT_MS = 5000L
private suspend fun loadAppCreationRequestBody(): JsonElement =
JsonParser.parseString(
client.get("https://raw.githubusercontent.com/ably/ably-common/refs/heads/main/test-resources/test-app-setup.json") {
timeout {
requestTimeoutMillis = CONFIG_TIMEOUT_MS
}
contentType(ContentType.Application.Json)
}.bodyAsText(),
).asJsonObject.get("post_apps")

26 changes: 26 additions & 0 deletions chat-android/src/test/java/com/ably/chat/SandboxTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.ably.chat

import io.ably.lib.realtime.ChannelState
import java.util.UUID
import kotlinx.coroutines.test.runTest
import org.junit.Assert.assertEquals
import org.junit.Before
import org.junit.Test

class SandboxTest {

private lateinit var sandbox: Sandbox

@Before
fun setUp() = runTest {
sandbox = Sandbox.createInstance()
}
Comment on lines +14 to +17
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling and cleanup to test lifecycle methods

The current setup lacks error handling and proper cleanup. Consider adding:

  1. Error handling in setUp
  2. A tearDown method to clean up resources
  3. Timeout handling for sandbox creation
 @Before
 fun setUp() = runTest {
-    sandbox = Sandbox.createInstance()
+    try {
+        withTimeout(5000) {
+            sandbox = Sandbox.createInstance()
+        }
+    } catch (e: Exception) {
+        throw IllegalStateException("Failed to set up sandbox environment: ${e.message}", e)
+    }
 }
+
+@After
+fun tearDown() = runTest {
+    try {
+        sandbox.close()
+    } catch (e: Exception) {
+        println("Warning: Failed to tear down sandbox environment: ${e.message}")
+        throw e
+    }
+}

Committable suggestion skipped: line range outside the PR's diff.


@Test
fun basicIntegrationTest() = runTest {
val chatClient = sandbox.createSandboxChatClient()
val room = chatClient.rooms.get(UUID.randomUUID().toString())
room.attach()
assertEquals(ChannelState.attached, room.messages.channel.state)
}
}
9 changes: 8 additions & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ gson = "2.11.0"
mockk = "1.13.12"
coroutine = "1.8.1"
build-config = "5.4.0"
ktor = "3.0.1"

[libraries]
junit = { group = "junit", name = "junit", version.ref = "junit" }
Expand Down Expand Up @@ -50,9 +51,15 @@ mockk = { group = "io.mockk", name = "mockk", version.ref = "mockk" }
coroutine-core = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-core", version.ref = "coroutine" }
coroutine-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "coroutine" }

ktor-client-core = { module = "io.ktor:ktor-client-core", version.ref = "ktor" }
ktor-client-cio = { module = "io.ktor:ktor-client-cio", version.ref = "ktor" }

[bundles]
ktor-client = ["ktor-client-core", "ktor-client-cio"]

[plugins]
detekt = { id = "io.gitlab.arturbosch.detekt", version.ref = "detekt"}
android-kotlin = { id = "org.jetbrains.kotlin.android", version.ref = "kotlin" }
android-kotlin = { id = "org.jetbrains.kotlin.android", version = "2.0.21" }
ttypic marked this conversation as resolved.
Show resolved Hide resolved
android-library = { id = "com.android.library", version.ref = "agp" }
android-application = { id = "com.android.application", version.ref = "agp" }
compose-compiler = { id = "org.jetbrains.kotlin.plugin.compose", version.ref = "kotlin" }
Expand Down