Skip to content

Commit 12e59f3

Browse files
fryanpanclaude
andcommitted
ADFA-4128 (2/11): address CodeRabbit review
- F1714-1 publish the FeatureFlags snapshot with @volatile - F1714-3 assert both Quick Build sentinels turn their own flag on - F1714-4 make the ADFA-4328 repro prove the worker actually parked - F1714-5 replace the em dash in the ContentReadWrite KDoc Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FstXxJ5cwWPcvmhZ9vJgJ7
1 parent 95cad65 commit 12e59f3

4 files changed

Lines changed: 51 additions & 8 deletions

File tree

‎common/src/main/java/com/itsaky/androidide/utils/FeatureFlags.kt‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ object FeatureFlags {
4040
private val logger = LoggerFactory.getLogger(FeatureFlags::class.java)
4141

4242
private val mutex = Mutex()
43+
44+
// The getters below read this without the mutex the sole writer holds. FlagsCache is
45+
// immutable, so publishing the reference is the whole fix - without it a reader can keep
46+
// seeing the direct-boot all-false snapshot after refresh() has replaced it.
47+
@Volatile
4348
private var flags = FlagsCache.DEFAULT
4449

4550
/**

‎common/src/test/java/com/itsaky/androidide/utils/FeatureFlagsTest.kt‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,30 @@ class FeatureFlagsTest {
9999

100100
assertThat(FeatureFlags.isExperimentsEnabled).isFalse()
101101
}
102+
103+
// The two Quick Build sentinels get a test each. Nothing else asserts that either
104+
// filename maps to its flag, so a typo or a swapped pair would leave the bench harness
105+
// silently unarmed and this suite green - visible only as a device run with no events.
106+
107+
@Test
108+
fun `the qbbench sentinel arms the bench hooks and nothing else`() =
109+
runTest {
110+
tempFolder.newFile("CodeOnTheGo.qbbench")
111+
112+
FeatureFlags.initialize()
113+
114+
assertThat(FeatureFlags.isQuickBuildBenchEnabled).isTrue()
115+
assertThat(FeatureFlags.isQuickBuildWarmCompileDisabled).isFalse()
116+
}
117+
118+
@Test
119+
fun `the qbnoseed sentinel disables warm compile and nothing else`() =
120+
runTest {
121+
tempFolder.newFile("CodeOnTheGo.qbnoseed")
122+
123+
FeatureFlags.initialize()
124+
125+
assertThat(FeatureFlags.isQuickBuildWarmCompileDisabled).isTrue()
126+
assertThat(FeatureFlags.isQuickBuildBenchEnabled).isFalse()
127+
}
102128
}

‎common/src/test/java/com/itsaky/androidide/utils/KeyedDebouncingActionCancelTest.kt‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
package com.itsaky.androidide.utils
22

33
import com.google.common.truth.Truth.assertThat
4+
import kotlinx.coroutines.CompletableDeferred
45
import kotlinx.coroutines.CoroutineExceptionHandler
56
import kotlinx.coroutines.CoroutineScope
67
import kotlinx.coroutines.SupervisorJob
78
import kotlinx.coroutines.channels.ClosedReceiveChannelException
89
import kotlinx.coroutines.delay
910
import kotlinx.coroutines.runBlocking
11+
import kotlinx.coroutines.withTimeout
1012
import org.junit.Test
1113
import java.util.concurrent.atomic.AtomicReference
1214
import kotlin.coroutines.CoroutineContext
@@ -35,23 +37,33 @@ class KeyedDebouncingActionCancelTest {
3537

3638
val ctx: CoroutineContext = scope.coroutineContext
3739

40+
// Signalled from inside the action, so the test knows the worker really got that
41+
// far. A fixed delay cannot tell "the worker is parked on receive()" apart from
42+
// "the worker never started" - and in the second case the cancellation raises a
43+
// CancellationException the handler never sees, so the repro silently does not run
44+
// and the test still reports green.
45+
val actionRan = CompletableDeferred<Unit>()
46+
3847
val debouncer =
3948
KeyedDebouncingAction<String>(
4049
scope = scope,
4150
debounceDuration = 50.milliseconds,
4251
actionContext = ctx,
43-
// Never invoked: the worker is cancelled while parked on receive().
44-
action = { _, _ -> },
52+
action = { _, _ -> actionRan.complete(Unit) },
4553
)
4654

4755
// schedule() creates the entry + launches the worker. With a CONFLATED channel and
48-
// no further sends, the worker debounces the single key, runs the (empty) action,
49-
// then loops back and parks on channel.receive() waiting for the next key.
56+
// no further sends, the worker debounces the single key, runs the action, then
57+
// loops back and parks on channel.receive() waiting for the next key.
5058
debouncer.schedule("k")
5159

52-
// Give the worker time to: receive "k", run the empty action, loop, and PARK on
53-
// the next channel.receive(). 200ms >> 50ms debounce window.
54-
delay(200)
60+
// Fails loudly rather than passing vacuously if the worker never reached the action.
61+
withTimeout(5_000) { actionRan.await() }
62+
assertThat(actionRan.isCompleted).isTrue()
63+
64+
// The action has returned; the remaining hop is joining the action job and looping
65+
// back to the park, which has no observable signal of its own.
66+
delay(100)
5567

5668
// Cancel the entry while the worker is parked on receive().
5769
debouncer.cancelPending("k")

‎editor/src/main/java/com/itsaky/androidide/editor/utils/ContentReadWrite.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ object ContentReadWrite {
3434
/**
3535
* Write this [Content] to the given [File].
3636
*
37-
* Writes IN PLACE — opens [file] directly and truncates + writes sequentially; this is
37+
* Writes IN PLACE - opens [file] directly and truncates + writes sequentially; this is
3838
* NOT a temp-file-then-rename swap. A filesystem watcher observing a save from this
3939
* method sees the target path itself change, never a sibling temp file (that pattern
4040
* is specific to EXTERNAL tools like `sed -i` or `git checkout`).

0 commit comments

Comments
 (0)