fix: [SDK-4995] match SyncJobService stop and completion by job ID - #2765
fadi-george wants to merge 1 commit into
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
📊 Diff Coverage ReportDiff Coverage Report (Changed Lines Only)Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff). Changed Files Coverage
Overall (aggregate gate)33/42 touched executable lines covered (78.6% — requires ≥ 80%) Per-file detail (informational; gate is aggregate above):
❌ Coverage Check FailedAggregate coverage on touched lines is 78.6% (minimum 80%). |
abdulraqeeb33
left a comment
abdulraqeeb33
left a comment
There was a problem hiding this comment.
Approved.
- The two stop tests put the inline
launchOnIOrunner back only after the assertions. A failed assert leaves the non-running stub for later tests. Restore it inafterAny. - No test that a thrown
runBackgroundServicescallsjobFinished(parameters, true), or that a winningonStopJobdoes not alsojobFinished.
abdulraqeeb33
left a comment
abdulraqeeb33
left a comment
There was a problem hiding this comment.
Same nits, on the lines.
| result shouldBe true | ||
| verify { job.cancel() } | ||
| verify(exactly = 0) { OneSignal.getService<IBackgroundManager>() } | ||
| every { OneSignalDispatchers.launchOnIO(any<suspend () -> Unit>()) } answers { |
There was a problem hiding this comment.
This puts the inline launchOnIO runner back only after the assertions. The other stop test does the same at line 197. A failed assert leaves the non-running stub for later tests. Restore it in afterAny.
|
|
||
| // When | ||
| val result = mocks.syncJobService.onStopJob(mocks.jobParameters) | ||
| test("onStopJob does not reschedule a run that already completed") { |
There was a problem hiding this comment.
Still missing: a thrown runBackgroundServices calls jobFinished(parameters, true), and a winning onStopJob does not also jobFinished.
2 participants
Description
One Line Summary
Make
SyncJobServicestart, stop, and completion race-safe by tracking the active run and matchingonStopJobby job ID.Details
Motivation
Split out of #2712.
onStopJobpreviously reached intoIBackgroundManagerviagetService, which could fail if init had not finished, andjobFinishedcould still be called after the system had already stopped the job.Scope
onStartJobcreates aJobRunthat owns its coroutine and state (RUNNING,STOPPED,FINISHED).onStopJobmatches byjobId(the system may pass a differentJobParametersinstance), cancels the owned coroutine, and returns whether it stopped a running job.jobFinishedor a successfulonStopJobwins, via compare-and-set.OneSignalDispatchers.launchOnIOdirectly so stop cancellations are not logged as errors.Testing
Unit testing
Updated
SyncJobServiceTestsfor distinctJobParametersinstances with matching and different job IDs, no active run, and stop after completion.Manual testing
Forced JobScheduler job
2071862118on an emulator with queued offline work. The sync ran onOneSignal-IO-1, completed throughjobFinished, with no ANR or crash.Affected code checklist
Checklist
Overview
Testing
Final pass