Skip to content

Commit 952b5b8

Browse files
typottertypotter
committed
Fix comments, tests, safe calls
1 parent 740d7e7 commit 952b5b8

File tree

7 files changed

+77
-101
lines changed

7 files changed

+77
-101
lines changed

‎detekt_custom_safe_calls.yml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,8 +1083,6 @@ datadog:
10831083
- "kotlinx.coroutines.flow.FlowCollector.emit(kotlin.Any?)"
10841084
- "kotlinx.coroutines.flow.FlowCollector(kotlin.coroutines.SuspendFunction1)"
10851085
- "kotlinx.coroutines.flow.flow(kotlin.coroutines.SuspendFunction1)"
1086-
- "kotlinx.coroutines.flow.MutableStateFlow(com.datadog.android.flags.model.FlagsClientState)"
1087-
- "kotlinx.coroutines.flow.MutableStateFlow.asStateFlow()"
10881086
# endregion
10891087
# region Kronos
10901088
- "com.lyft.kronos.AndroidClockFactory.createKronosClock(android.content.Context, com.lyft.kronos.SyncListener?, kotlin.collections.List, kotlin.Long, kotlin.Long, kotlin.Long, kotlin.Long)"

‎features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsClient.kt‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,17 +155,13 @@ interface FlagsClient {
155155
*
156156
* Provides three ways to observe state:
157157
* - Synchronous: [StateObservable.getCurrentState] for immediate queries (Java-friendly)
158-
* - Reactive: [StateObservable.flow] for coroutine-based updates (Kotlin)
159158
* - Callback: [StateObservable.addListener] for traditional observers (Java-friendly)
160159
*
161160
* Example:
162161
* ```kotlin
163162
* // Synchronous
164163
* val current = client.state.getCurrentState()
165164
*
166-
* // Reactive Flow
167-
* client.state.flow.collect { state -> /* ... */ }
168-
*
169165
* // Callback
170166
* client.state.addListener(listener)
171167
* ```

‎features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/NoOpFlagsClient.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ internal class NoOpFlagsClient(
3737

3838
override val state: StateObservable = object : StateObservable {
3939
override fun getCurrentState(): FlagsClientState = FlagsClientState.Error(null)
40-
override fun addListener(listener: FlagsStateListener) { /* no-op */ }
41-
override fun removeListener(listener: FlagsStateListener) { /* no-op */ }
40+
override fun addListener(listener: FlagsStateListener) = Unit
41+
override fun removeListener(listener: FlagsStateListener) = Unit
4242
}
4343

4444
/**

‎features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsTest.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ internal class FlagsTest {
5858
@BeforeEach
5959
fun `set up`() {
6060
whenever(mockSdkCore.internalLogger) doReturn mockInternalLogger
61-
whenever(mockSdkCore.createSingleThreadExecutorService(org.mockito.kotlin.any())) doReturn
62-
mockExecutorService
61+
whenever(mockSdkCore.createSingleThreadExecutorService(org.mockito.kotlin.any())) doReturn mockExecutorService
6362

6463
whenever(mockDatadogContext.clientToken) doReturn fakeClientToken
6564
whenever(mockDatadogContext.site) doReturn DatadogSite.US1

‎features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/DatadogFlagsClientTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,7 +1316,7 @@ internal class DatadogFlagsClientTest {
13161316
@Test
13171317
fun `M delegate to state manager W state_addListener()`() {
13181318
// Given
1319-
val mockListener = mock(FlagsStateListener::class.java)
1319+
val mockListener = mock<FlagsStateListener>()
13201320

13211321
// When
13221322
testedClient.state.addListener(mockListener)
@@ -1329,7 +1329,7 @@ internal class DatadogFlagsClientTest {
13291329
@Test
13301330
fun `M delegate to state manager W state_removeListener()`() {
13311331
// Given
1332-
val mockListener = mock(FlagsStateListener::class.java)
1332+
val mockListener = mock<FlagsStateListener>()
13331333

13341334
// When
13351335
testedClient.state.removeListener(mockListener)

‎features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsStateManagerTest.kt‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import org.mockito.kotlin.inOrder
2323
import org.mockito.kotlin.mock
2424
import org.mockito.kotlin.times
2525
import org.mockito.kotlin.verify
26+
import org.mockito.kotlin.verifyNoMoreInteractions
2627
import org.mockito.kotlin.whenever
2728
import org.mockito.quality.Strictness
2829
import java.util.concurrent.ExecutorService
@@ -85,21 +86,6 @@ internal class FlagsStateManagerTest {
8586

8687
// region addListener / removeListener
8788

88-
@Test
89-
fun `M notify listener W addListener() and notify`() {
90-
// Given
91-
testedManager.addListener(mockListener)
92-
93-
// When
94-
testedManager.updateState(FlagsClientState.Ready)
95-
96-
// Then
97-
inOrder(mockListener) {
98-
verify(mockListener).onStateChanged(FlagsClientState.NotReady) // Current state on add
99-
verify(mockListener).onStateChanged(FlagsClientState.Ready) // State update
100-
}
101-
}
102-
10389
@Test
10490
fun `M not notify listener after removal W removeListener() and notify`() {
10591
// Given
@@ -113,7 +99,7 @@ internal class FlagsStateManagerTest {
11399
testedManager.updateState(FlagsClientState.Ready)
114100

115101
// Then - no further notifications after removal
116-
org.mockito.kotlin.verifyNoMoreInteractions(mockListener)
102+
verifyNoMoreInteractions(mockListener)
117103
}
118104

119105
@Test

‎features/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/repository/DefaultFlagsRepositoryTest.kt‎

Lines changed: 70 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,12 @@ internal class DefaultFlagsRepositoryTest {
5252

5353
private lateinit var testedRepository: DefaultFlagsRepository
5454

55+
private lateinit var testContext: EvaluationContext
56+
private lateinit var singleFlagMap: Map<String, PrecomputedFlag>
57+
private lateinit var multipleFlagsMap: Map<String, PrecomputedFlag>
58+
5559
@BeforeEach
56-
fun `set up`() {
60+
fun `set up`(forge: Forge) {
5761
whenever(mockFeatureSdkCore.internalLogger) doReturn mockInternalLogger
5862
whenever(
5963
mockDataStore.value<FlagsStateEntry>(
@@ -73,6 +77,42 @@ internal class DefaultFlagsRepositoryTest {
7377
dataStore = mockDataStore,
7478
instanceName = "default"
7579
)
80+
81+
// Setup test fixtures for hasFlags tests
82+
testContext = EvaluationContext(forge.anAlphabeticalString(), emptyMap())
83+
84+
singleFlagMap = mapOf(
85+
forge.anAlphabeticalString() to PrecomputedFlag(
86+
variationType = "string",
87+
variationValue = forge.anAlphabeticalString(),
88+
doLog = false,
89+
allocationKey = forge.anAlphabeticalString(),
90+
variationKey = forge.anAlphabeticalString(),
91+
extraLogging = JSONObject(),
92+
reason = "DEFAULT"
93+
)
94+
)
95+
96+
multipleFlagsMap = mapOf(
97+
forge.anAlphabeticalString() to PrecomputedFlag(
98+
variationType = "string",
99+
variationValue = forge.anAlphabeticalString(),
100+
doLog = false,
101+
allocationKey = forge.anAlphabeticalString(),
102+
variationKey = forge.anAlphabeticalString(),
103+
extraLogging = JSONObject(),
104+
reason = "DEFAULT"
105+
),
106+
forge.anAlphabeticalString() to PrecomputedFlag(
107+
variationType = "boolean",
108+
variationValue = "true",
109+
doLog = false,
110+
allocationKey = forge.anAlphabeticalString(),
111+
variationKey = forge.anAlphabeticalString(),
112+
extraLogging = JSONObject(),
113+
reason = "TARGETING_MATCH"
114+
)
115+
)
76116
}
77117

78118
@Test
@@ -188,88 +228,44 @@ internal class DefaultFlagsRepositoryTest {
188228
// region hasFlags
189229

190230
@Test
191-
fun `M return expected value W hasFlags() { for various states }`(forge: Forge) {
192-
data class TestCase(val given: () -> Unit, val then: Boolean)
231+
fun `M return false W hasFlags() { no state set }`() {
232+
// When + Then
233+
assertThat(testedRepository.hasFlags()).isFalse()
234+
}
193235

194-
val testCases = listOf(
195-
TestCase(
196-
given = { /* no state set */ },
197-
then = false
198-
),
199-
TestCase(
200-
given = {
201-
testedRepository.setFlagsAndContext(
202-
EvaluationContext(forge.anAlphabeticalString(), emptyMap()),
203-
emptyMap()
204-
)
205-
},
206-
then = false
207-
),
208-
TestCase(
209-
given = {
210-
testedRepository.setFlagsAndContext(
211-
EvaluationContext(forge.anAlphabeticalString(), emptyMap()),
212-
mapOf(
213-
forge.anAlphabeticalString() to PrecomputedFlag(
214-
variationType = "string",
215-
variationValue = forge.anAlphabeticalString(),
216-
doLog = false,
217-
allocationKey = forge.anAlphabeticalString(),
218-
variationKey = forge.anAlphabeticalString(),
219-
extraLogging = JSONObject(),
220-
reason = "DEFAULT"
221-
)
222-
)
223-
)
224-
},
225-
then = true
226-
),
227-
TestCase(
228-
given = {
229-
testedRepository.setFlagsAndContext(
230-
EvaluationContext(forge.anAlphabeticalString(), emptyMap()),
231-
mapOf(
232-
forge.anAlphabeticalString() to PrecomputedFlag(
233-
variationType = "string",
234-
variationValue = forge.anAlphabeticalString(),
235-
doLog = false,
236-
allocationKey = forge.anAlphabeticalString(),
237-
variationKey = forge.anAlphabeticalString(),
238-
extraLogging = JSONObject(),
239-
reason = "DEFAULT"
240-
),
241-
forge.anAlphabeticalString() to PrecomputedFlag(
242-
variationType = "boolean",
243-
variationValue = "true",
244-
doLog = false,
245-
allocationKey = forge.anAlphabeticalString(),
246-
variationKey = forge.anAlphabeticalString(),
247-
extraLogging = JSONObject(),
248-
reason = "TARGETING_MATCH"
249-
)
250-
)
251-
)
252-
},
253-
then = true
254-
)
236+
@Test
237+
fun `M return false W hasFlags() { empty flags map }`(forge: Forge) {
238+
// Given
239+
testedRepository.setFlagsAndContext(
240+
EvaluationContext(forge.anAlphabeticalString(), emptyMap()),
241+
emptyMap()
255242
)
256243

257-
testCases.forEach { testCase ->
258-
// Given
259-
testCase.given()
244+
// When + Then
245+
assertThat(testedRepository.hasFlags()).isFalse()
246+
}
260247

261-
// When
262-
val result = testedRepository.hasFlags()
248+
@Test
249+
fun `M return true W hasFlags() { single flag }`() {
250+
// Given
251+
testedRepository.setFlagsAndContext(testContext, singleFlagMap)
263252

264-
// Then
265-
assertThat(result).isEqualTo(testCase.then)
266-
}
253+
// When + Then
254+
assertThat(testedRepository.hasFlags()).isTrue()
255+
}
256+
257+
@Test
258+
fun `M return true W hasFlags() { multiple flags }`() {
259+
// Given
260+
testedRepository.setFlagsAndContext(testContext, multipleFlagsMap)
261+
262+
// When + Then
263+
assertThat(testedRepository.hasFlags()).isTrue()
267264
}
268265

269266
@Test
270267
fun `M not block W hasFlags() { persistence still loading }`() {
271268
// Given
272-
val startTime = System.currentTimeMillis()
273269
doAnswer {
274270
// Never call the callback - simulate slow persistence
275271
null
@@ -287,6 +283,7 @@ internal class DefaultFlagsRepositoryTest {
287283
)
288284

289285
// When
286+
val startTime = System.currentTimeMillis()
290287
val result = slowRepository.hasFlags()
291288
val elapsedTime = System.currentTimeMillis() - startTime
292289

0 commit comments

Comments
 (0)