Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start of a framework for WebMediaPlayerImpl tests #1521

Closed
wants to merge 14 commits into from

Conversation

sideb0ard
Copy link
Contributor

b/299135670

Initial setup for WebMediaPlayerImpl unit test.
Currently creates an object and asserts it is empty.

@sideb0ard sideb0ard force-pushed the media_player_tests branch 2 times, most recently from 139869a to 5ae9a31 Compare September 8, 2023 22:48
Change-Id: I3bba96a1d17f6eeedf72df7f7d57c822f3dfb0bd
Change-Id: I810d406f719e9b78e20ce5ed25a0f90f60ec9a1b

namespace {
SbDecodeTargetGraphicsContextProvider*
MockGetSbDecodeTargetGraphicsContextProvider() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider naming it Stub****, as it's not a mock.

using ::cobalt::media::WebMediaPlayerImpl;
using ::media::ChunkDemuxer;
using ::testing::NiceMock;

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove redundant empty line.

Comment on lines +86 to +89
bool HasAudio() { return wmpi_->HasAudio(); }
bool HasVideo() { return wmpi_->HasVideo(); }
bool IsPaused() { return wmpi_->IsPaused(); }
float GetPlaybackRate() { return wmpi_->GetPlaybackRate(); }
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider removing these functions, and let the individual test to verify on them directly.

EXPECT_TRUE(wmpi_->IsPaused()); should be as good as (maybe a little better than) EXPECT_TRUE(IsPaused());

base::Bind(MockGetSbDecodeTargetGraphicsContextProvider), &client_,
&client_, true, false, true,
#if SB_API_VERSION >= 15
10, 10,
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to double check if they are as expected, as these values are 10 microseconds.

Copy link

codecov bot commented Dec 7, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (fc372aa) 58.55% compared to head (3fe8d80) 58.70%.
Report is 105 commits behind head on main.

❗ Current head 3fe8d80 differs from pull request most recent head 96133e2. Consider uploading reports for the commit 96133e2 to get more accurate results

Files Patch % Lines
cobalt/media/player/web_media_player_impl.cc 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1521      +/-   ##
==========================================
+ Coverage   58.55%   58.70%   +0.14%     
==========================================
  Files        1909     1909              
  Lines       94580    94581       +1     
==========================================
+ Hits        55385    55522     +137     
+ Misses      39195    39059     -136     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sideb0ard and others added 11 commits December 8, 2023 14:03
Change-Id: Iec338bb075e2b8298288b8bd6fd259158449d361
Change-Id: I1ad6f48753206cbcfa921a8d473322dbac9705e0
Change-Id: I4a71fed1e99f8ca1612acea02948755eca099422
Change-Id: I4c46a0f658cab8e594db01309bdeb91548607747
…ient

Change-Id: Ic2a712bb8e45876b3292ab2c4e117c02855c6249
Change-Id: Ibbba0afccf4a99d71f1c3fe0cb7d93bcd798abc8
Change-Id: I1e8b2adbbad9e95e9fbad87a4e15a887150b5036
Change-Id: I693775eb45c8040d425218ba11a46e577701033e
@sideb0ard sideb0ard closed this Jan 13, 2025
@sideb0ard sideb0ard deleted the media_player_tests branch January 13, 2025 21:21
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.

2 participants