Skip to content

VPLAY-12242: create l1 or l2 tests reflecting app expectations#1189

Open
psiva01 wants to merge 18 commits intodev_sprint_25_2from
feature/VPLAY-12242_l1
Open

VPLAY-12242: create l1 or l2 tests reflecting app expectations#1189
psiva01 wants to merge 18 commits intodev_sprint_25_2from
feature/VPLAY-12242_l1

Conversation

@psiva01
Copy link
Contributor

@psiva01 psiva01 commented Mar 17, 2026

related to player state changes

@psiva01 psiva01 requested a review from a team as a code owner March 17, 2026 08:43
Copilot AI review requested due to automatic review settings March 17, 2026 08:43
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves an existing unit test for the PrivateInstanceAAMP::Stop() method, replacing a legacy test (stopTest_11) with a more focused, descriptive test (Stop_ActualImplementation_SetsPlayerStateToIdle) that validates the expected player state transition from eSTATE_PLAYING to eSTATE_IDLE after calling Stop().

Changes:

  • Replaced the old stopTest_11 with Stop_ActualImplementation_SetsPlayerStateToIdle, which explicitly sets the player to eSTATE_PLAYING, calls Stop(false), and asserts the final state is eSTATE_IDLE.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR replaces a legacy, poorly-named unit test (stopTest_11) in PrivAampTests with a clear, intent-driven test (Stop_ActualImplementation_SetsPlayerStateToIdle) that verifies the player transitions from eSTATE_PLAYING to eSTATE_IDLE upon calling Stop(false). This aligns with the goal of creating L1/L2 tests that reflect app expectations around player state changes.

Changes:

  • Replaced stopTest_11 (which tested unrelated Fog TSB behavior) with Stop_ActualImplementation_SetsPlayerStateToIdle, a focused test verifying the PLAYING → IDLE state transition on Stop().

related to player state changes

Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com>
@psiva01 psiva01 force-pushed the feature/VPLAY-12242_l1 branch from c57f6b1 to 32df1a9 Compare March 18, 2026 14:49
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds new unit tests intended to validate player state transitions for both the internal PrivateInstanceAAMP and the public PlayerInstanceAAMP surfaces, and wires them into the corresponding utest targets.

Changes:

  • Add PrivateInstanceAAMP state transition tests covering tune lifecycle, buffering, EOS, and error paths.
  • Add a PlayerInstanceAAMP state test focused on Seek()-driven state transitions.
  • Extend stream-abstraction test infrastructure to mock IsInitialCachingSupported() and route the fake implementation through the mock.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
test/utests/tests/PrivateInstanceAAMP/PlayerStateTests.cpp New state transition tests for PrivateInstanceAAMP using mocks/fakes.
test/utests/tests/PrivateInstanceAAMP/CMakeLists.txt Adds the new PlayerStateTests.cpp to the PrivateInstanceAAMP test target.
test/utests/tests/PrivAampTests/PrivAampTests.cpp Updates a Stop-related test to explicitly assert state transition to IDLE.
test/utests/tests/PlayerInstanceAAMP/PlayerStateTests.cpp New player-facing state transition test (Tune + Seek).
test/utests/tests/PlayerInstanceAAMP/CMakeLists.txt Adds the new PlayerStateTests.cpp to the PlayerInstanceAAMP test target.
test/utests/mocks/MockStreamAbstractionAAMP.h Adds a mock method for IsInitialCachingSupported().
test/utests/fakes/FakeStreamAbstractionAamp.cpp Routes StreamAbstractionAAMP::IsInitialCachingSupported() through the mock when present.

psiva01 added 2 commits March 19, 2026 12:00
Reason for change: Addressed co-pilot review comments

Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com>
Copilot AI review requested due to automatic review settings March 19, 2026 13:55
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds/updates unit tests to validate AAMP player state transitions (notably tune lifecycle, buffering, error handling, and seek behavior) to better reflect application expectations around state changes.

Changes:

  • Add new PlayerStateTests.cpp suites for both PrivateInstanceAAMP and PlayerInstanceAAMP.
  • Extend mocks/fakes to support initial caching and first-frame/first-video-frame-displayed driven state transitions.
  • Update existing PrivAampTests stop-state test naming and assertions; wire new tests into CMake targets.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/utests/tests/PrivateInstanceAAMP/PlayerStateTests.cpp New unit tests covering end-to-end state transitions for PrivateInstanceAAMP (tune → playing/paused/buffering/complete/error → idle).
test/utests/tests/PrivateInstanceAAMP/CMakeLists.txt Adds the new PrivateInstanceAAMP state test source to the build.
test/utests/tests/PrivAampTests/PrivAampTests.cpp Renames/updates a Stop() test to assert PLAYING → IDLE via Stop(false).
test/utests/tests/PlayerInstanceAAMP/PlayerStateTests.cpp New unit tests for PlayerInstanceAAMP seek flows, including “seek while paused” behavior.
test/utests/tests/PlayerInstanceAAMP/CMakeLists.txt Adds the new PlayerInstanceAAMP state test source to the build.
test/utests/mocks/MockStreamAbstractionAAMP.h Adds IsInitialCachingSupported() to the stream abstraction mock.
test/utests/mocks/MockPrivateInstanceAAMP.h Adds mock declarations needed by updated fakes/state tests (first-frame callbacks + pause pipeline).
test/utests/fakes/FakeStreamAbstractionAamp.cpp Routes IsInitialCachingSupported() through g_mockStreamAbstractionAAMP.
test/utests/fakes/FakePrivateInstanceAAMP.cpp Updates fake state-gating logic for first-frame/first-video-frame-displayed and seek-while-paused; delegates PausePipeline to mock.

related to player state change

Reason for change: Addressed co-piolt review comments
Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com>
@psiva01 psiva01 requested a review from Copilot March 19, 2026 14:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds new L1/L2-style unit tests to validate AAMP player state transitions (both PrivateInstanceAAMP internals and the PlayerInstanceAAMP public wrapper), aligned to application-observed state changes.

Changes:

  • Add new PlayerStateTests.cpp test suites for PrivateInstanceAAMP and PlayerInstanceAAMP, covering tune/seek/buffering/error flows.
  • Wire the new test sources into the relevant test targets’ CMakeLists.txt.
  • Extend/adjust unit-test mocks and fakes to support initial caching and first-frame/first-video-frame-displayed state behaviors.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/utests/tests/PrivateInstanceAAMP/PlayerStateTests.cpp New state-transition tests for PrivateInstanceAAMP (tune lifecycle, buffering, error).
test/utests/tests/PrivateInstanceAAMP/CMakeLists.txt Adds PlayerStateTests.cpp to the PrivateInstanceAAMP test target.
test/utests/tests/PrivAampTests/PrivAampTests.cpp Reworks one stop-related test to assert Stop(false) moves state to IDLE.
test/utests/tests/PlayerInstanceAAMP/PlayerStateTests.cpp New state-transition tests for PlayerInstanceAAMP seek flows (including seek-while-paused).
test/utests/tests/PlayerInstanceAAMP/CMakeLists.txt Adds PlayerStateTests.cpp to the PlayerInstanceAAMP test target.
test/utests/mocks/MockStreamAbstractionAAMP.h Adds IsInitialCachingSupported() mock to support buffering/initial caching logic.
test/utests/mocks/MockPrivateInstanceAAMP.h Adds missing mocked methods used by fakes/tests (NotifyFirstFrameReceived, NotifyFirstVideoFrameDisplayed, PausePipeline).
test/utests/fakes/FakeStreamAbstractionAamp.cpp Routes IsInitialCachingSupported() through the global mock when present.
test/utests/fakes/FakePrivateInstanceAAMP.cpp Updates fake state gating for seek-while-paused + first-frame notifications; delegates PausePipeline() to mock.

Copilot AI review requested due to automatic review settings March 20, 2026 06:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds new unit tests to validate AAMP player state transitions (both PrivateInstanceAAMP and PlayerInstanceAAMP) and extends the existing fake/mock layer to support the new state-related behaviors.

Changes:

  • Add PlayerStateTests.cpp suites for PrivateInstanceAAMP and PlayerInstanceAAMP state transition expectations.
  • Update test CMakeLists to compile the new test sources.
  • Extend fakes/mocks to support initial caching checks and seek-while-paused “first video frame displayed” behaviors.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
test/utests/tests/PrivateInstanceAAMP/PlayerStateTests.cpp New lifecycle/buffering/error state transition tests using Tune + notifications
test/utests/tests/PrivateInstanceAAMP/CMakeLists.txt Adds new PrivateInstance state test source to build
test/utests/tests/PrivAampTests/PrivAampTests.cpp Renames/updates a Stop() state transition test
test/utests/tests/PlayerInstanceAAMP/PlayerStateTests.cpp New seek-related state transition tests at PlayerInstance layer
test/utests/tests/PlayerInstanceAAMP/CMakeLists.txt Adds new PlayerInstance state test source to build
test/utests/mocks/MockStreamAbstractionAAMP.h Adds mockable IsInitialCachingSupported()
test/utests/mocks/MockPrivateInstanceAAMP.h Adds additional mocked methods used by new seek/pause behavior
test/utests/fakes/FakeStreamAbstractionAamp.cpp Forwards IsInitialCachingSupported() to the mock when present
test/utests/fakes/FakePrivateInstanceAAMP.cpp Implements seek-while-paused/first-frame state behavior in the fake

@psiva01
Copy link
Contributor Author

psiva01 commented Mar 20, 2026

@copilot open a new pull request to apply changes based on the comments in this thread

Copy link
Contributor

Copilot AI commented Mar 20, 2026

@psiva01 I've opened a new pull request, #1211, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 3 commits March 20, 2026 14:17
Co-authored-by: psiva01 <214551386+psiva01@users.noreply.github.com>
VPLAY-12242: Fix reviewer feedback on player state test infrastructure
Copilot AI review requested due to automatic review settings March 20, 2026 17:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@psiva01 psiva01 requested a review from Copilot March 20, 2026 17:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

@psiva01
Copy link
Contributor Author

psiva01 commented Mar 23, 2026

@copilot open a new pull request to apply changes based on the comments in this thread

Copy link
Contributor

Copilot AI commented Mar 23, 2026

@psiva01 I've opened a new pull request, #1215, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 3 commits March 23, 2026 08:32
- Create g_mockAampConfig before PrivateInstanceAAMP in SetUp() with safe ON_CALL defaults
- Use NotifyFirstVideoFrameDisplayed() in buffering test (real app trigger path)
- Align FakePrivateInstanceAAMP::TuneHelper with real gating: gate pause-on-first-frame flags on seekWhilePaused && mSeekOperationInProgress

Co-authored-by: psiva01 <214551386+psiva01@users.noreply.github.com>
Agent-Logs-Url: https://github.com/rdkcentral/aamp/sessions/d1a46d63-4799-427a-b437-e895b6646cef
…d to player state changes

Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com>
VPLAY-12242: Fix player state test fixture ordering and fake TuneHelper flag gating
Copilot AI review requested due to automatic review settings March 23, 2026 10:08
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants