Add comprehensive unit tests for location, geofence, and media managers#4001
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| void updateActivatesGeofencesWithinBubble() { | ||
| Geofence near = createGeofence("near", 0.001, 0.001, 50, -1L); | ||
| Geofence far = createGeofence("far", 2.0, 2.0, 100, -1L); | ||
| manager.add(near, far); | ||
| locationManager.clearRecords(); | ||
|
|
||
| manager.update(1000); | ||
|
|
||
| assertTrue(locationManager.addedIds.contains("near")); | ||
| assertTrue(locationManager.addedIds.contains("$AsyncGeoStreamer.bubble")); | ||
| assertTrue(manager.isCurrentlyActive("near")); | ||
| assertFalse(manager.isCurrentlyActive("far")); | ||
| assertTrue(locationManager.backgroundCleared); | ||
| assertFalse(locationManager.removedIds.contains("near")); |
There was a problem hiding this comment.
The test assumes GeofenceManager.update() always re-registers $AsyncGeoStreamer.bubble and clears a background listener on every call. In reality the bubble is only added once (or after the device moves ~100 m) and clearing only occurs when a listener was previously set. Because the singleton is initialized in setupManager() before the assertions, addedIds never contains the bubble and backgroundCleared remains false even though the production code is behaving correctly. These assertions will fail consistently and should be removed or the state should be reset explicitly before the update.
Useful? React with 👍 / 👎.
| @Test | ||
| void getCurrentLocationSyncWithoutListenerBindsAndReturnsResult() { | ||
| Location expected = new Location(1.0, 2.0); | ||
| manager.setCurrentLocation(expected); | ||
|
|
||
| Location result = manager.getCurrentLocationSync(1000); | ||
|
|
||
| assertSame(expected, result); | ||
| assertEquals(1, manager.bindCount); | ||
| assertEquals(LocationManager.AVAILABLE, manager.getStatus()); | ||
| assertNull(manager.getStoredRequest()); |
There was a problem hiding this comment.
getCurrentLocationSync() uses LL to bind a temporary listener and LL.locationUpdated() immediately calls setLocationListener(null), which resets status to TEMPORARILY_UNAVAILABLE. The test’s expectation that manager.getStatus() remains AVAILABLE contradicts this behavior, so the assertion fails even when the implementation works as intended. The check should either expect TEMPORARILY_UNAVAILABLE or omit the status verification.
Useful? React with 👍 / 👎.
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
1 participant
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68f5c11c28108331b5380a6d927225d5