Improve RTCP unit tests

Improve RTCP unit tests

Remove the callback mocks and validate data for various types of
reports.

Also removes redundant code.

BUG=530843
Committed: https://crrev.com/59f7664f538095900d17f139050ea011fd18094b
Cr-Commit-Position: refs/heads/master@{#367174}

[Irfan, 2015-12-12 01:02:07]

Description was changed from

==========
Improve RTCP unit tests

Remove the callback mocks and validate data for various types of
reports.

Also removes redundant code.

BUG=530843
==========

to

==========
Improve RTCP unit tests

Remove the callback mocks and validate data for various types of
reports.

Also removes redundant code.

BUG=530843
==========

[Irfan, 2015-12-12 01:02:07]

isheriff@chromium.org changed reviewers:
+ miu@chromium.org, xjz@chromium.org

[Irfan, 2015-12-12 01:02:20]

PTAL

[miu, 2015-12-12 01:40:54]

lgtm % minor things:

https://codereview.chromium.org/1521173002/diff/1/media/cast/net/rtcp/rtcp_un...
File media/cast/net/rtcp/rtcp_unittest.cc (right):

https://codereview.chromium.org/1521173002/diff/1/media/cast/net/rtcp/rtcp_un...
media/cast/net/rtcp/rtcp_unittest.cc:228:
base::TimeDelta::FromMilliseconds(kTargetDelayMs), NULL, NULL);
nit: nullptr instead of NULL.

https://codereview.chromium.org/1521173002/diff/1/media/cast/net/rtcp/rtcp_un...
media/cast/net/rtcp/rtcp_unittest.cc:285: EXPECT_TRUE(last_logs_.size() == 1);
Should be an ASSERT since the test will crash if this is not true: ASSERT_EQ(1,
last_logs_.size());

https://codereview.chromium.org/1521173002/diff/1/media/cast/net/rtcp/rtcp_un...
media/cast/net/rtcp/rtcp_unittest.cc:289:
EXPECT_TRUE(frame_log.event_log_messages_.size() == 1);
ditto: ASSERT here.

https://codereview.chromium.org/1521173002/diff/1/media/cast/net/rtcp/rtcp_un...
media/cast/net/rtcp/rtcp_unittest.cc:302: TEST(MisplacedCastTest, NtpAndTime) {
Hey!  I think this belongs in rtcp_utility_unittest.cc now.  :)

[Irfan, 2015-12-12 01:54:48]

https://codereview.chromium.org/1521173002/diff/1/media/cast/net/rtcp/rtcp_un...
File media/cast/net/rtcp/rtcp_unittest.cc (right):

https://codereview.chromium.org/1521173002/diff/1/media/cast/net/rtcp/rtcp_un...
media/cast/net/rtcp/rtcp_unittest.cc:228:
base::TimeDelta::FromMilliseconds(kTargetDelayMs), NULL, NULL);
On 2015/12/12 01:40:53, miu wrote:
> nit: nullptr instead of NULL.

Done.

https://codereview.chromium.org/1521173002/diff/1/media/cast/net/rtcp/rtcp_un...
media/cast/net/rtcp/rtcp_unittest.cc:285: EXPECT_TRUE(last_logs_.size() == 1);
On 2015/12/12 01:40:53, miu wrote:
> Should be an ASSERT since the test will crash if this is not true:
ASSERT_EQ(1,
> last_logs_.size());

Done.

https://codereview.chromium.org/1521173002/diff/1/media/cast/net/rtcp/rtcp_un...
media/cast/net/rtcp/rtcp_unittest.cc:289:
EXPECT_TRUE(frame_log.event_log_messages_.size() == 1);
On 2015/12/12 01:40:53, miu wrote:
> ditto: ASSERT here.

Done.

https://codereview.chromium.org/1521173002/diff/1/media/cast/net/rtcp/rtcp_un...
media/cast/net/rtcp/rtcp_unittest.cc:302: TEST(MisplacedCastTest, NtpAndTime) {
On 2015/12/12 01:40:53, miu wrote:
> Hey!  I think this belongs in rtcp_utility_unittest.cc now.  :)

Done.

[miu, 2015-12-12 02:11:45]

lgtm

https://codereview.chromium.org/1521173002/diff/20001/media/cast/net/rtcp/rtc...
File media/cast/net/rtcp/rtcp_unittest.cc (right):

https://codereview.chromium.org/1521173002/diff/20001/media/cast/net/rtcp/rtc...
media/cast/net/rtcp/rtcp_unittest.cc:285: ASSERT_TRUE(last_logs_.size() == 1);
ASSERT_EQ(1, last_logs_.size());

https://codereview.chromium.org/1521173002/diff/20001/media/cast/net/rtcp/rtc...
media/cast/net/rtcp/rtcp_unittest.cc:289:
ASSERT_TRUE(frame_log.event_log_messages_.size() == 1);
ASSERT_EQ(1, frame_log.event_log_messages_.size());

[xjz, 2015-12-14 18:05:17]

lgtm

[Irfan, 2015-12-30 18:30:07]

PTAL

[xjz, 2015-12-30 19:29:57]

lgtm

[Irfan, 2015-12-30 19:33:36]

The CQ bit was checked by isheriff@chromium.org

[Irfan, 2015-12-30 19:33:36]

The patchset sent to the CQ was uploaded after l-g-t-m from miu@chromium.org
Link to the patchset: https://codereview.chromium.org/1521173002/#ps40001
(title: "Rebased")

[commit-bot: I haz the power, 2015-12-30 19:34:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1521173002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1521173002/40001

[commit-bot: I haz the power, 2015-12-30 19:38:35]

Description was changed from

==========
Improve RTCP unit tests

Remove the callback mocks and validate data for various types of
reports.

Also removes redundant code.

BUG=530843
==========

to

==========
Improve RTCP unit tests

Remove the callback mocks and validate data for various types of
reports.

Also removes redundant code.

BUG=530843
==========

[commit-bot: I haz the power, 2015-12-30 19:38:36]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-30 19:39:30]

Description was changed from

==========
Improve RTCP unit tests

Remove the callback mocks and validate data for various types of
reports.

Also removes redundant code.

BUG=530843
==========

to

==========
Improve RTCP unit tests

Remove the callback mocks and validate data for various types of
reports.

Also removes redundant code.

BUG=530843

Committed: https://crrev.com/59f7664f538095900d17f139050ea011fd18094b
Cr-Commit-Position: refs/heads/master@{#367174}
==========

[commit-bot: I haz the power, 2015-12-30 19:39:31]

Patchset 3 (id:??) landed as
https://crrev.com/59f7664f538095900d17f139050ea011fd18094b
Cr-Commit-Position: refs/heads/master@{#367174}