Fix issues with rtc_stats_unittests tests so that they can run on bots

Fix issues with rtc_stats_unittests tests so that they can run on bots.
This target is not run on bots so a couple of issues went under the
radar. If we expose the tests and run them on the bots[1] two issues are
surfaced which this CL fixes. After this CL lands we can enable this
target on the bots without it going red.

rtcstats_unittest.cc: Fix const char* string comparison issue by
comparing with strcmp instead of equality check.

rtcstatscollector_unittest.cc: Fix TSAN issue by constructing
ScopedFakeClock before spawning Threads.

[1] https://codereview.webrtc.org/2340443002/

BUG=chromium:627816
NOTRY=True

Committed: https://crrev.com/fdafab84bc9db9eabdf54b1186c27abfe871801a
Cr-Commit-Position: refs/heads/master@{#14215}

[hbos, 2016-09-14 07:35:29]

Patchset #1 (id:1) has been deleted

[hbos, 2016-09-14 07:41:24]

Description was changed from

==========
Fix TSAN issue: Construct ScopedFakeClock before Threads

BUG=chromium:627816
==========

to

==========
Fix issues with rtc_stats_unittests tests so that they can run on bots.
This target is not run on bots so a couple of issues went under the
radar. If we expose the tests and run them on the bots[1] two issues are
surfaced which this CL fixes. After this CL lands we can enable this
target on the bots without it going red.

rtcstats_unittest.cc: Fix const char* string comparison issue by
comparing with strcmp instead of equality check.

rtcstatscollector_unittest.cc: Fix TSAN issue by constructing
ScopedFakeClock before spawning Threads.

[1] https://codereview.webrtc.org/2340443002/

BUG=chromium:627816
==========

[hbos, 2016-09-14 07:41:24]

hbos@webrtc.org changed reviewers:
+ nisse@webrtc.org

[hbos, 2016-09-14 07:42:24]

Please take a look, nisse.

[nisse-webrtc, 2016-09-14 07:53:08]

https://codereview.webrtc.org/2333343002/diff/20001/webrtc/stats/rtcstats_uni...
File webrtc/stats/rtcstats_unittest.cc (right):

https://codereview.webrtc.org/2333343002/diff/20001/webrtc/stats/rtcstats_uni...
webrtc/stats/rtcstats_unittest.cc:135: EXPECT_EQ(strcmp(*stats.m_static_string,
"123"), 0);
Makes me think: Do you really need stats using the type const char*? If all
strings can be std::string, that would make things a little simpler. But that's
perhaps best addressed in a different cl.

https://codereview.webrtc.org/2333343002/diff/20001/webrtc/stats/rtcstats_uni...
webrtc/stats/rtcstats_unittest.cc:142:
EXPECT_EQ(*stats.m_sequence_static_string, std::vector<const char*>());
Can you change to non-empty sequences for all these variables, with unique
values? And ensure that you get the correct comparison also for
sequence_static_string.

[hbos, 2016-09-14 08:21:15]

PTAL nisse.

https://codereview.webrtc.org/2333343002/diff/20001/webrtc/stats/rtcstats_uni...
File webrtc/stats/rtcstats_unittest.cc (right):

https://codereview.webrtc.org/2333343002/diff/20001/webrtc/stats/rtcstats_uni...
webrtc/stats/rtcstats_unittest.cc:135: EXPECT_EQ(strcmp(*stats.m_static_string,
"123"), 0);
On 2016/09/14 07:53:07, nisse-webrtc wrote:
> Makes me think: Do you really need stats using the type const char*? If all
> strings can be std::string, that would make things a little simpler. But
that's
> perhaps best addressed in a different cl.

I think you're right, having two different string types seems kind of pointless
(the idea is const strings don't have to be copied into the std::string
container but these should be few and short). I'll remove in a separate CL.

https://codereview.webrtc.org/2333343002/diff/20001/webrtc/stats/rtcstats_uni...
webrtc/stats/rtcstats_unittest.cc:142:
EXPECT_EQ(*stats.m_sequence_static_string, std::vector<const char*>());
On 2016/09/14 07:53:07, nisse-webrtc wrote:
> Can you change to non-empty sequences for all these variables, with unique
> values? And ensure that you get the correct comparison also for
> sequence_static_string.

Done.

[nisse-webrtc, 2016-09-14 08:36:42]

https://codereview.webrtc.org/2333343002/diff/20001/webrtc/stats/rtcstats_uni...
File webrtc/stats/rtcstats_unittest.cc (right):

https://codereview.webrtc.org/2333343002/diff/20001/webrtc/stats/rtcstats_uni...
webrtc/stats/rtcstats_unittest.cc:142:
EXPECT_EQ(*stats.m_sequence_static_string, std::vector<const char*>());
On 2016/09/14 08:21:15, hbos wrote:
> On 2016/09/14 07:53:07, nisse-webrtc wrote:
> > Can you change to non-empty sequences for all these variables, with unique
> > values? And ensure that you get the correct comparison also for
> > sequence_static_string.
> 
> Done.

Not all, could you add a unique value each also to the other m_sequence_* ?

[hbos, 2016-09-14 09:39:57]

Patchset #3 (id:60001) has been deleted

[hbos, 2016-09-14 09:42:49]

PTAL nisse

https://codereview.webrtc.org/2333343002/diff/20001/webrtc/stats/rtcstats_uni...
File webrtc/stats/rtcstats_unittest.cc (right):

https://codereview.webrtc.org/2333343002/diff/20001/webrtc/stats/rtcstats_uni...
webrtc/stats/rtcstats_unittest.cc:142:
EXPECT_EQ(*stats.m_sequence_static_string, std::vector<const char*>());
On 2016/09/14 08:36:42, nisse-webrtc wrote:
> On 2016/09/14 08:21:15, hbos wrote:
> > On 2016/09/14 07:53:07, nisse-webrtc wrote:
> > > Can you change to non-empty sequences for all these variables, with unique
> > > values? And ensure that you get the correct comparison also for
> > > sequence_static_string.
> > 
> > Done.
> 
> Not all, could you add a unique value each also to the other m_sequence_* ? 

Oh right, done.

[nisse-webrtc, 2016-09-14 10:34:48]

lgtm

[hbos, 2016-09-14 10:44:10]

The CQ bit was checked by hbos@webrtc.org

[commit-bot: I haz the power, 2016-09-14 10:44:16]

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, 2016-09-14 12:44:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-14 12:44:56]

Try jobs failed on following builders:
  android_arm64_rel on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  android_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[hbos, 2016-09-14 12:59:45]

Description was changed from

==========
Fix issues with rtc_stats_unittests tests so that they can run on bots.
This target is not run on bots so a couple of issues went under the
radar. If we expose the tests and run them on the bots[1] two issues are
surfaced which this CL fixes. After this CL lands we can enable this
target on the bots without it going red.

rtcstats_unittest.cc: Fix const char* string comparison issue by
comparing with strcmp instead of equality check.

rtcstatscollector_unittest.cc: Fix TSAN issue by constructing
ScopedFakeClock before spawning Threads.

[1] https://codereview.webrtc.org/2340443002/

BUG=chromium:627816
==========

to

==========
Fix issues with rtc_stats_unittests tests so that they can run on bots.
This target is not run on bots so a couple of issues went under the
radar. If we expose the tests and run them on the bots[1] two issues are
surfaced which this CL fixes. After this CL lands we can enable this
target on the bots without it going red.

rtcstats_unittest.cc: Fix const char* string comparison issue by
comparing with strcmp instead of equality check.

rtcstatscollector_unittest.cc: Fix TSAN issue by constructing
ScopedFakeClock before spawning Threads.

[1] https://codereview.webrtc.org/2340443002/

BUG=chromium:627816
NOTRY=True
==========

[hbos, 2016-09-14 13:00:24]

Offline bots, everything else green, also these test don't yet execute (just
build) on bots, landing with NOTRY=True.

[hbos, 2016-09-14 13:00:35]

The CQ bit was checked by hbos@webrtc.org

[commit-bot: I haz the power, 2016-09-14 13:00:47]

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, 2016-09-14 13:02:15]

Description was changed from

==========
Fix issues with rtc_stats_unittests tests so that they can run on bots.
This target is not run on bots so a couple of issues went under the
radar. If we expose the tests and run them on the bots[1] two issues are
surfaced which this CL fixes. After this CL lands we can enable this
target on the bots without it going red.

rtcstats_unittest.cc: Fix const char* string comparison issue by
comparing with strcmp instead of equality check.

rtcstatscollector_unittest.cc: Fix TSAN issue by constructing
ScopedFakeClock before spawning Threads.

[1] https://codereview.webrtc.org/2340443002/

BUG=chromium:627816
NOTRY=True
==========

to

==========
Fix issues with rtc_stats_unittests tests so that they can run on bots.
This target is not run on bots so a couple of issues went under the
radar. If we expose the tests and run them on the bots[1] two issues are
surfaced which this CL fixes. After this CL lands we can enable this
target on the bots without it going red.

rtcstats_unittest.cc: Fix const char* string comparison issue by
comparing with strcmp instead of equality check.

rtcstatscollector_unittest.cc: Fix TSAN issue by constructing
ScopedFakeClock before spawning Threads.

[1] https://codereview.webrtc.org/2340443002/

BUG=chromium:627816
NOTRY=True
==========

[commit-bot: I haz the power, 2016-09-14 13:02:16]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-09-14 13:02:27]

Description was changed from

==========
Fix issues with rtc_stats_unittests tests so that they can run on bots.
This target is not run on bots so a couple of issues went under the
radar. If we expose the tests and run them on the bots[1] two issues are
surfaced which this CL fixes. After this CL lands we can enable this
target on the bots without it going red.

rtcstats_unittest.cc: Fix const char* string comparison issue by
comparing with strcmp instead of equality check.

rtcstatscollector_unittest.cc: Fix TSAN issue by constructing
ScopedFakeClock before spawning Threads.

[1] https://codereview.webrtc.org/2340443002/

BUG=chromium:627816
NOTRY=True
==========

to

==========
Fix issues with rtc_stats_unittests tests so that they can run on bots.
This target is not run on bots so a couple of issues went under the
radar. If we expose the tests and run them on the bots[1] two issues are
surfaced which this CL fixes. After this CL lands we can enable this
target on the bots without it going red.

rtcstats_unittest.cc: Fix const char* string comparison issue by
comparing with strcmp instead of equality check.

rtcstatscollector_unittest.cc: Fix TSAN issue by constructing
ScopedFakeClock before spawning Threads.

[1] https://codereview.webrtc.org/2340443002/

BUG=chromium:627816
NOTRY=True

Committed: https://crrev.com/fdafab84bc9db9eabdf54b1186c27abfe871801a
Cr-Commit-Position: refs/heads/master@{#14215}
==========

[commit-bot: I haz the power, 2016-09-14 13:02:27]

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