Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(504)

Issue 2692593002: Media Remoting: End to end integration tests. (Closed)

Created:
3 years, 10 months ago by xjz
Modified:
3 years, 8 months ago
CC:
chromium-reviews, feature-media-reviews_chromium.org, xjz+watch_chromium.org, miu+watch_chromium.org, apacible+watch_chromium.org, erickung+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Media Remoting: End to end integration tests. Add end to end integration tests for Media Remoting. Refactors PipelineIntegrationTest to test both media and media remoting pipeline. Re-use current tests. No new tests are added in this CL. BUG=684065 Review-Url: https://codereview.chromium.org/2692593002 Cr-Commit-Position: refs/heads/master@{#462739} Committed: https://chromium.googlesource.com/chromium/src/+/60d4e0a63eb3f2be26aca1281be29c64b0f5c670

Patch Set 1 #

Total comments: 68

Patch Set 2 : Addressed comments. Rebased. #

Total comments: 25

Patch Set 3 : Addressed comments. #

Patch Set 4 : File rename only. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2290 lines, -850 lines) Patch
M media/remoting/BUILD.gn View 1 2 3 1 chunk +6 lines, -0 lines 0 comments Download
A media/remoting/end2end_test_renderer.h View 1 1 chunk +71 lines, -0 lines 0 comments Download
A media/remoting/end2end_test_renderer.cc View 1 2 3 1 chunk +217 lines, -0 lines 0 comments Download
A media/remoting/receiver.h View 1 2 3 1 chunk +88 lines, -0 lines 0 comments Download
A media/remoting/receiver.cc View 1 2 3 1 chunk +304 lines, -0 lines 0 comments Download
A media/remoting/stream_provider.h View 1 2 3 1 chunk +69 lines, -0 lines 0 comments Download
A media/remoting/stream_provider.cc View 1 2 3 1 chunk +485 lines, -0 lines 0 comments Download
M media/test/BUILD.gn View 1 1 chunk +7 lines, -0 lines 0 comments Download
M media/test/pipeline_integration_test.cc View 1 2 49 chunks +913 lines, -814 lines 0 comments Download
M media/test/pipeline_integration_test_base.h View 1 2 5 chunks +54 lines, -30 lines 0 comments Download
M media/test/pipeline_integration_test_base.cc View 1 2 6 chunks +76 lines, -6 lines 0 comments Download

Messages

Total messages: 32 (21 generated)
xjz
PTAL
3 years, 10 months ago (2017-02-10 21:36:24 UTC) #4
miu
Very nice work here! Sorry it took so long to review. It'll be great to ...
3 years, 8 months ago (2017-03-29 01:39:15 UTC) #7
xjz
Thanks miu@ for reviewing. Addressed your comments. PTAL again. Thanks! https://codereview.chromium.org/2692593002/diff/1/media/remoting/end2end_test_renderer.cc File media/remoting/end2end_test_renderer.cc (right): https://codereview.chromium.org/2692593002/diff/1/media/remoting/end2end_test_renderer.cc#newcode63 ...
3 years, 8 months ago (2017-03-30 23:21:32 UTC) #13
miu
Comments on PS2: https://codereview.chromium.org/2692593002/diff/80001/media/remoting/remote_receiver.cc File media/remoting/remote_receiver.cc (right): https://codereview.chromium.org/2692593002/diff/80001/media/remoting/remote_receiver.cc#newcode207 media/remoting/remote_receiver.cc:207: base::TimeDelta max_time = media_time; Isn't max_time ...
3 years, 8 months ago (2017-04-03 21:05:01 UTC) #16
xjz
Addressed miu's comments. PTAL again. Thanks! https://codereview.chromium.org/2692593002/diff/80001/media/remoting/remote_receiver.cc File media/remoting/remote_receiver.cc (right): https://codereview.chromium.org/2692593002/diff/80001/media/remoting/remote_receiver.cc#newcode207 media/remoting/remote_receiver.cc:207: base::TimeDelta max_time = ...
3 years, 8 months ago (2017-04-04 01:07:56 UTC) #17
miu
PS3 lgtm...How about a `git cl try` to see these running on the try bots? ...
3 years, 8 months ago (2017-04-05 21:14:48 UTC) #18
xjz
sandersd: Need RS on media/test/*. Re-used current tests to test media remoting pipeline. PTAL. Thanks.
3 years, 8 months ago (2017-04-06 17:18:03 UTC) #24
sandersd (OOO until July 31)
lgtm
3 years, 8 months ago (2017-04-06 21:46:18 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2692593002/120001
3 years, 8 months ago (2017-04-06 22:45:04 UTC) #28
commit-bot: I haz the power
Committed patchset #4 (id:120001) as https://chromium.googlesource.com/chromium/src/+/60d4e0a63eb3f2be26aca1281be29c64b0f5c670
3 years, 8 months ago (2017-04-07 01:38:48 UTC) #31
findit-for-me
3 years, 8 months ago (2017-04-07 02:43:40 UTC) #32
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:120001) has been created in
https://codereview.chromium.org/2804183002/ by
findit-for-me@appspot.gserviceaccount.com.

The reason for reverting is: 
Findit identified CL at revision 462739 as the culprit for
failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb....

Powered by Google App Engine
This is Rietveld 408576698