Add interval-based throttling tests for FindServerUTCTime #858
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reason for change: Updates UTCTiming logic to skip sync if interval hasn’t elapsed. Add Configs to control the interval Test Procedure: updated in ticket Risks: Low Signed-off-by: Anurag Krishnan <akrish513@cable.comcast.com>
deprecated eAAMPConfig_UTCSyncOnErrorRetrySec fixed bug with config order causing runtime crash added some doxygen comments Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
Merge branch 'feature/VPLAY-12359_UTCTiming' of https://github.com/rdkcentral/aamp into feature/VPLAY-12359_UTCTiming Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Extended FindServerUTCTime test coverage with 5 new test cases: 1. SyncOnStartup - Verify initial sync when UTCSyncOnStartup enabled 2. SkipSyncBeforeInterval - Verify sync skipped when interval not elapsed 3. SyncAfterInterval - Verify sync occurs when interval has elapsed 4. UseCachedOffset - Verify cached offset used when sync skipped 5. NoSyncWhenStartupDisabled - Verify no sync when startup disabled These tests verify the interval-based throttling behavior including: - Network sync calls only when appropriate - Cached offset usage between sync intervals - Proper time progression and interval checking - Configuration setting behavior Addresses PR feedback on #853 for comprehensive UTC sync testing. Co-authored-by: pstroffolino <20442081+pstroffolino@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add changes to address feedback on VPLAY-12359
Add interval-based throttling tests for FindServerUTCTime
Jan 15, 2026
Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>
…kcentral/aamp into feature/VPLAY-12359_UTCTiming
Contributor
|
@copilot this new tests crash with null pointer deference |
Contributor
|
@copilot I marked them with GTEST_SKIP() temporarily. |
Added null check for mMPDParseHelper before calling SetLocalTimeDelta to prevent crashes when the helper is not initialized. This makes the function more defensive and allows unit tests to call FindServerUTCTime directly without requiring full MPD initialization. Removed GTEST_SKIP() from all 5 UTC sync tests: - FindServerUTCTime_SyncOnStartup - FindServerUTCTime_SkipSyncBeforeInterval - FindServerUTCTime_SyncAfterInterval - FindServerUTCTime_UseCachedOffset - FindServerUTCTime_NoSyncWhenStartupDisabled Tests now run successfully with the null check in place. Co-authored-by: pstroffolino <20442081+pstroffolino@users.noreply.github.com>
Contributor
Author
Fixed in commit be0bbf7. The crash was caused by Added a null check before the call to make the function defensive. All 5 tests now run without crashes. |
Base automatically changed from
feature/VPLAY-12359_UTCTiming
to
dev_sprint_25_2
January 15, 2026 03:59
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix null pointer dereference in FindServerUTCTime tests
Issue
The new UTC sync throttling tests were crashing with null pointer dereference when calling
FindServerUTCTimedirectly.Root Cause
The
FindServerUTCTimeimplementation callsmMPDParseHelper->SetLocalTimeDelta(mDeltaTime)at the end, butmMPDParseHelperis initialized to NULL in the constructor and only gets set during full MPD initialization viaGetMPDParseHelper().When unit tests call
FindServerUTCTimedirectly with just an XML node (bypassingInitializeMPD), the helper remains null, causing a crash.Fix
Added defensive null check before calling
SetLocalTimeDelta:This makes the function more robust and allows it to be tested in isolation without requiring full MPD setup.
Changes
mMPDParseHelperGTEST_SKIP()from all 5 tests to enable themAll tests now run successfully without crashes.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.