Skip to content

Commit a83ab97

Browse files
committed
create test to prove OperationRepo isn't sleeping
Create test to confirm that OperationRepo.processQueueForever keeps poll when it should suspend the thread while waiting for the next operation. My theory is this polling every 5 seconds means the app process may never fully sleep, which contributes to battery drain. We moved code to a new getNextOps() function simply to make it testable, no logic changes in this commit.
1 parent 25924dc commit a83ab97

File tree

3 files changed

+62
-11
lines changed

3 files changed

+62
-11
lines changed

OneSignalSDK/onesignal/core/src/main/java/com/onesignal/core/internal/operations/impl/OperationRepo.kt

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ internal class OperationRepo(
2222
private val _configModelStore: ConfigModelStore,
2323
private val _time: ITime,
2424
) : IOperationRepo, IStartableService {
25-
private class OperationQueueItem(
25+
internal class OperationQueueItem(
2626
val operation: Operation,
2727
val waiter: WaiterWithValue<Boolean>? = null,
2828
var retries: Int = 0,
@@ -107,16 +107,7 @@ internal class OperationRepo(
107107
return
108108
}
109109
try {
110-
var ops: List<OperationQueueItem>? = null
111-
112-
synchronized(queue) {
113-
val startingOp = queue.firstOrNull { it.operation.canStartExecute }
114-
115-
if (startingOp != null) {
116-
queue.remove(startingOp)
117-
ops = getGroupableOperations(startingOp)
118-
}
119-
}
110+
val ops = getNextOps()
120111

121112
// if the queue is empty at this point, we are no longer in force flush mode. We
122113
// check this now so if the execution is unsuccessful with retry, we don't find ourselves
@@ -253,6 +244,19 @@ internal class OperationRepo(
253244
delay(delayFor)
254245
}
255246

247+
internal fun getNextOps(): List<OperationQueueItem>? {
248+
return synchronized(queue) {
249+
val startingOp = queue.firstOrNull { it.operation.canStartExecute }
250+
251+
if (startingOp != null) {
252+
queue.remove(startingOp)
253+
getGroupableOperations(startingOp)
254+
} else {
255+
null
256+
}
257+
}
258+
}
259+
256260
/**
257261
* Given a starting operation, find and remove from the queue all other operations that
258262
* can be executed along with the starting operation. The full list of operations, with

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/core/internal/operations/OperationRepoTests.kt

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,57 @@ import io.mockk.mockk
1717
import io.mockk.runs
1818
import io.mockk.slot
1919
import io.mockk.spyk
20+
import io.mockk.verify
21+
import kotlinx.coroutines.delay
2022

2123
class OperationRepoTests : FunSpec({
2224

2325
beforeAny {
2426
Logging.logLevel = LogLevel.NONE
2527
}
2628

29+
// Ensures we are not continuously waking the CPU
30+
test("ensure processQueueForever suspends when queue is empty") {
31+
// Given
32+
val mockExecutor = mockk<IOperationExecutor>()
33+
every { mockExecutor.operations } returns listOf("DUMMY_OPERATION")
34+
coEvery { mockExecutor.execute(any()) } returns ExecutionResponse(ExecutionResult.SUCCESS)
35+
36+
val mockOperationModelStore = mockk<OperationModelStore>()
37+
every { mockOperationModelStore.list() } returns listOf()
38+
every { mockOperationModelStore.add(any()) } just runs
39+
every { mockOperationModelStore.remove(any()) } just runs
40+
41+
val operationRepo =
42+
spyk(
43+
OperationRepo(
44+
listOf(mockExecutor),
45+
mockOperationModelStore,
46+
MockHelper.configModelStore(),
47+
MockHelper.time(1000),
48+
),
49+
)
50+
51+
val operationIdSlot = slot<String>()
52+
val operation = mockOperation(operationIdSlot = operationIdSlot)
53+
54+
// When
55+
operationRepo.start()
56+
val response = operationRepo.enqueueAndWait(operation)
57+
// Must wait for background logic to spin to see how many times it
58+
// will call getNextOps()
59+
delay(1_000)
60+
61+
// Then
62+
response shouldBe true
63+
verify(exactly = 2) {
64+
// 1st: gets the operation
65+
// 2nd: will be empty
66+
// 3rd: shouldn't be called, loop should be waiting on next operation
67+
operationRepo.getNextOps()
68+
}
69+
}
70+
2771
test("enqueue operation executes and is removed when executed") {
2872
// Given
2973
val mockExecutor = mockk<IOperationExecutor>()

OneSignalSDK/onesignal/testhelpers/src/main/java/com/onesignal/mocks/MockHelper.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ object MockHelper {
4444
fun configModelStore(action: ((ConfigModel) -> Unit)? = null): ConfigModelStore {
4545
val configModel = ConfigModel()
4646

47+
configModel.opRepoExecutionInterval = 1
48+
configModel.opRepoPostWakeDelay = 1
49+
4750
configModel.appId = DEFAULT_APP_ID
4851

4952
if (action != null) {

0 commit comments

Comments
 (0)