Skip to content

Commit 91ef50a

Browse files
authored
Fixed bug in toolkit experiment event publication (#3164)
* Fixed bug in toolkit experiment event publication * Cleaning up tests a bit * Making listener a functional interface for syntacic sugar
1 parent 1cf6a1c commit 91ef50a

File tree

2 files changed

+23
-19
lines changed

2 files changed

+23
-19
lines changed

jetbrains-core/src/software/aws/toolkits/jetbrains/core/experiments/ToolkitExperiment.kt

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,15 @@ internal class ToolkitExperimentManager : PersistentStateComponent<ExperimentSta
102102
EP_NAME.extensionList.contains(experiment) && enabledState.getOrDefault(experiment.id, getDefault(experiment))
103103

104104
fun setState(experiment: ToolkitExperiment, enabled: Boolean) {
105-
if (enabled != isEnabled(experiment)) {
106-
ApplicationManager.getApplication().messageBus.syncPublisher(EXPERIMENT_CHANGED).enableSettingsStateChanged(experiment)
107-
}
105+
val previousState = isEnabled(experiment)
108106
if (enabled == getDefault(experiment)) {
109107
enabledState.remove(experiment.id)
110108
} else {
111109
enabledState[experiment.id] = enabled
112110
}
111+
if (enabled != previousState) {
112+
ApplicationManager.getApplication().messageBus.syncPublisher(EXPERIMENT_CHANGED).enableSettingsStateChanged(experiment)
113+
}
113114
AwsTelemetry.experimentActivation(
114115
experimentId = experiment.id,
115116
experimentState = if (enabled) {
@@ -170,6 +171,6 @@ internal class ExperimentState : BaseState() {
170171
val nextSuggestion by map<String, Long>()
171172
}
172173

173-
interface ToolkitExperimentStateChangedListener {
174+
fun interface ToolkitExperimentStateChangedListener {
174175
fun enableSettingsStateChanged(toolkitExperiment: ToolkitExperiment)
175176
}

jetbrains-core/tst/software/aws/toolkits/jetbrains/core/experiments/ToolkitExperimentManagerTest.kt

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import org.mockito.kotlin.verify
1616
import org.mockito.kotlin.verifyNoInteractions
1717
import software.aws.toolkits.core.rules.SystemPropertyHelper
1818
import software.aws.toolkits.core.utils.test.aString
19+
import software.aws.toolkits.jetbrains.core.experiments.ToolkitExperimentManager.Companion.EXPERIMENT_CHANGED
1920
import software.aws.toolkits.jetbrains.utils.deserializeState
2021
import software.aws.toolkits.jetbrains.utils.rules.RegistryRule
2122
import software.aws.toolkits.jetbrains.utils.serializeState
@@ -188,17 +189,14 @@ class ToolkitExperimentManagerTest {
188189
val anExperiment = DummyExperiment()
189190
assertThat(anExperiment.isEnabled()).isFalse
190191
val mockListener: ToolkitExperimentStateChangedListener = mock()
191-
val conn = ApplicationManager.getApplication().messageBus.connect()
192-
conn.subscribe(ToolkitExperimentManager.EXPERIMENT_CHANGED, mockListener)
192+
subscribeToTopic(mockListener)
193193
anExperiment.setState(true)
194194

195195
argumentCaptor<ToolkitExperiment>().apply {
196196
verify(mockListener).enableSettingsStateChanged(capture())
197197
assertThat(allValues).hasSize(1)
198198
assertThat(firstValue.id).isEqualTo(anExperiment.id)
199199
}
200-
201-
conn.dispose()
202200
}
203201

204202
@Test
@@ -207,17 +205,22 @@ class ToolkitExperimentManagerTest {
207205
ExtensionTestUtil.maskExtensions(ToolkitExperimentManager.EP_NAME, listOf(experiment), disposableRule.disposable)
208206
assertThat(experiment.isEnabled()).isTrue
209207
val mockListener: ToolkitExperimentStateChangedListener = mock()
210-
val conn = ApplicationManager.getApplication().messageBus.connect()
211-
conn.subscribe(ToolkitExperimentManager.EXPERIMENT_CHANGED, mockListener)
208+
subscribeToTopic(mockListener)
212209
experiment.setState(false)
213210

214211
argumentCaptor<ToolkitExperiment>().apply {
215212
verify(mockListener).enableSettingsStateChanged(capture())
216213
assertThat(allValues).hasSize(1)
217214
assertThat(firstValue.id).isEqualTo(experiment.id)
218215
}
216+
}
219217

220-
conn.dispose()
218+
@Test
219+
fun `updated experiment state is reflected when event is consumed`() {
220+
val anExperiment = DummyExperiment()
221+
ExtensionTestUtil.maskExtensions(ToolkitExperimentManager.EP_NAME, listOf(anExperiment), disposableRule.disposable)
222+
subscribeToTopic { toolkitExperiment -> assertThat(toolkitExperiment.isEnabled()).isTrue }
223+
anExperiment.setState(true)
221224
}
222225

223226
@Test
@@ -226,13 +229,11 @@ class ToolkitExperimentManagerTest {
226229
ExtensionTestUtil.maskExtensions(ToolkitExperimentManager.EP_NAME, listOf(experiment), disposableRule.disposable)
227230
assertThat(experiment.isEnabled()).isTrue
228231
val mockListener: ToolkitExperimentStateChangedListener = mock()
229-
val conn = ApplicationManager.getApplication().messageBus.connect()
230-
conn.subscribe(ToolkitExperimentManager.EXPERIMENT_CHANGED, mockListener)
232+
subscribeToTopic(mockListener)
231233

232234
experiment.setState(true)
233-
verifyNoInteractions(mockListener)
234235

235-
conn.dispose()
236+
verifyNoInteractions(mockListener)
236237
}
237238

238239
@Test
@@ -241,13 +242,10 @@ class ToolkitExperimentManagerTest {
241242
ExtensionTestUtil.maskExtensions(ToolkitExperimentManager.EP_NAME, listOf(experiment), disposableRule.disposable)
242243
assertThat(experiment.isEnabled()).isFalse
243244
val mockListener: ToolkitExperimentStateChangedListener = mock()
244-
val conn = ApplicationManager.getApplication().messageBus.connect()
245-
conn.subscribe(ToolkitExperimentManager.EXPERIMENT_CHANGED, mockListener)
245+
subscribeToTopic(mockListener)
246246

247247
experiment.setState(false)
248248
verifyNoInteractions(mockListener)
249-
250-
conn.dispose()
251249
}
252250

253251
@Test
@@ -295,6 +293,11 @@ class ToolkitExperimentManagerTest {
295293
sut.neverPrompt(experiment)
296294
assertThat(sut.shouldPrompt(experiment, now.plusMillis(2))).isFalse
297295
}
296+
297+
private fun subscribeToTopic(listener: ToolkitExperimentStateChangedListener) {
298+
val conn = ApplicationManager.getApplication().messageBus.connect(disposableRule.disposable)
299+
conn.subscribe(EXPERIMENT_CHANGED, listener)
300+
}
298301
}
299302

300303
class DummyExperiment(

0 commit comments

Comments
 (0)