Modified the rtp_receiver_unittests.

Modified the rtp_receiver_unittests.

Implemented operator == in RtpSource and use the gmock EXPECT_THAT to make the test cleaner.

Related CL: https://codereview.webrtc.org/2770233003/

BUG=chromium:703122
Review-Url: https://codereview.webrtc.org/2813753002
Cr-Commit-Position: refs/heads/master@{#17659}
Committed: https://chromium.googlesource.com/external/webrtc/+/0426222f4c8a2832349ddb857bc45d748f7952fa

[Zhi Huang, 2017-04-10 21:19:11]

Patchset #1 (id:1) has been deleted

[Zhi Huang, 2017-04-10 21:29:47]

zhihuang@webrtc.org changed reviewers:
+ danilchap@webrtc.org, deadbeef@webrtc.org

[Zhi Huang, 2017-04-10 21:29:48]

Please take a look.
Thanks.

https://codereview.webrtc.org/2813753002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_unittest.cc (right):

https://codereview.webrtc.org/2813753002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_receiver_unittest.cc:174: RtpSource(now_ms,
kSsrc1, RtpSourceType::SSRC)));
It's true that this looks much cleaner but my concern is that when using this on
a container of objects, the error messages would not very clear when the EXPECT
fails. It would not tell us which attribute is mismatched, would it?

https://codereview.webrtc.org/2813753002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_receiver_unittest.cc:199:
header.arrOfCSRCs[0] = (i + 1);
This is just an arbitrary number and it doesn't have to be i + 1.

[Zhi Huang, 2017-04-10 21:30:23]

Description was changed from

==========
Modified the rtp_receiver_unittests.

Implemented operator == in RtpSource and use the gmock EXPECT_THAT to make
the test cleaner.

BUG=chromium:703122
==========

to

==========
Modified the rtp_receiver_unittests.

Implemented operator == in RtpSource and use the gmock EXPECT_THAT to make the
test cleaner.

Related CL: https://codereview.webrtc.org/2770233003/

BUG=chromium:703122
==========

[Taylor Brandstetter, 2017-04-10 22:35:14]

lgtm

https://codereview.webrtc.org/2813753002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc (right):

https://codereview.webrtc.org/2813753002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc:245: }
Can remove a level of indentation now.

https://codereview.webrtc.org/2813753002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_unittest.cc (right):

https://codereview.webrtc.org/2813753002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_receiver_unittest.cc:90: bool kInOrder =
true;
kInOrder could move to the top with the other constants

https://codereview.webrtc.org/2813753002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_receiver_unittest.cc:174: RtpSource(now_ms,
kSsrc1, RtpSourceType::SSRC)));
On 2017/04/10 21:29:48, Zhi Huang wrote:
> It's true that this looks much cleaner but my concern is that when using this
on
> a container of objects, the error messages would not very clear when the
EXPECT
> fails. It would not tell us which attribute is mismatched, would it?

It actually does print a pretty detailed message. It includes something like
this:

matcher #0: is equal to 16-byte object <7B-00 00-00 00-00 00-00 DE-00 00-00
00-00 00-00>
and where the following elements don't match any matchers:
element #0: 16-byte object <7B-00 00-00 00-00 00-00 7B-00 00-00 00-00 00-00>

Though the message could be more descriptive if there was a stream insertion
operator for RtpSource (so that the individual fields are printed, rather than
"16-byte object...").

[danilchap, 2017-04-11 07:48:33]

lgtm % Taylor's comments

https://codereview.webrtc.org/2813753002/diff/20001/webrtc/modules/rtp_rtcp/s...
File webrtc/modules/rtp_rtcp/source/rtp_receiver_unittest.cc (right):

https://codereview.webrtc.org/2813753002/diff/20001/webrtc/modules/rtp_rtcp/s...
webrtc/modules/rtp_rtcp/source/rtp_receiver_unittest.cc:208:
ASSERT_TRUE(sources.size() == 2 * kSourceListSize);
ASSERT_EQ

[Zhi Huang, 2017-04-11 17:50:47]

The CQ bit was checked by zhihuang@webrtc.org

[Zhi Huang, 2017-04-11 17:50:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
danilchap@webrtc.org, deadbeef@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2813753002/#ps40001 (title:
"Address the comments before submitting.")

[commit-bot: I haz the power, 2017-04-11 17:50:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-04-11 18:28:10]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491933047674100,
"parent_rev": "00d802b6ee72259b0f8a83865961afc26c0f7e1f", "commit_rev":
"0426222f4c8a2832349ddb857bc45d748f7952fa"}

[commit-bot: I haz the power, 2017-04-11 18:28:14]

Description was changed from

==========
Modified the rtp_receiver_unittests.

Implemented operator == in RtpSource and use the gmock EXPECT_THAT to make the
test cleaner.

Related CL: https://codereview.webrtc.org/2770233003/

BUG=chromium:703122
==========

to

==========
Modified the rtp_receiver_unittests.

Implemented operator == in RtpSource and use the gmock EXPECT_THAT to make the
test cleaner.

Related CL: https://codereview.webrtc.org/2770233003/

BUG=chromium:703122

Review-Url: https://codereview.webrtc.org/2813753002
Cr-Commit-Position: refs/heads/master@{#17659}
Committed:
https://chromium.googlesource.com/external/webrtc/+/0426222f4c8a2832349ddb857...
==========

[commit-bot: I haz the power, 2017-04-11 18:28:15]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/external/webrtc/+/0426222f4c8a2832349ddb857...