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

Issue 2392613003: Switched flaky ProfilerTest to use fake clock (Closed)

Created:
4 years, 2 months ago by skvlad
Modified:
4 years, 2 months ago
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com
Target Ref:
refs/pending/heads/master
Project:
webrtc
Visibility:
Public.

Description

Switched flaky ProfilerTest to use fake clock The test was heavily dependent on wall clock timing, so it ended up being disabled in some build configurations. This CL switches it to use a fake clock instead. BUG=webrtc:5947 Committed: https://crrev.com/cd97b2b943c643f8432d0924a5d3dc1da2868914 Cr-Commit-Position: refs/heads/master@{#14507}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Replaced approximate comparisons with exact ones #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -23 lines) Patch
M webrtc/base/profiler_unittest.cc View 1 3 chunks +19 lines, -23 lines 0 comments Download

Messages

Total messages: 13 (5 generated)
Taylor Brandstetter
lgtm, with small comment https://codereview.webrtc.org/2392613003/diff/1/webrtc/base/profiler_unittest.cc File webrtc/base/profiler_unittest.cc (right): https://codereview.webrtc.org/2392613003/diff/1/webrtc/base/profiler_unittest.cc#newcode43 webrtc/base/profiler_unittest.cc:43: EXPECT_NEAR(kWaitSec, event->mean(), kTolerance * 3); ...
4 years, 2 months ago (2016-10-04 00:48:53 UTC) #2
skvlad
https://codereview.webrtc.org/2392613003/diff/1/webrtc/base/profiler_unittest.cc File webrtc/base/profiler_unittest.cc (right): https://codereview.webrtc.org/2392613003/diff/1/webrtc/base/profiler_unittest.cc#newcode43 webrtc/base/profiler_unittest.cc:43: EXPECT_NEAR(kWaitSec, event->mean(), kTolerance * 3); On 2016/10/04 00:48:53, Taylor ...
4 years, 2 months ago (2016-10-04 01:17:45 UTC) #3
pthatcher1
Profiler is being deleted: https://codereview.webrtc.org/2374033002/ So maybe just delete the unit test?
4 years, 2 months ago (2016-10-04 19:21:35 UTC) #5
pthatcher1
lgtm In case you don't want to delete it?
4 years, 2 months ago (2016-10-04 19:22:05 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/2392613003/20001
4 years, 2 months ago (2016-10-04 19:23:42 UTC) #9
skvlad
On 2016/10/04 19:22:05, pthatcher1 wrote: > lgtm > > In case you don't want to ...
4 years, 2 months ago (2016-10-04 19:24:30 UTC) #10
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 2 months ago (2016-10-04 19:32:56 UTC) #11
commit-bot: I haz the power
4 years, 2 months ago (2016-10-04 19:33:03 UTC) #13
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/cd97b2b943c643f8432d0924a5d3dc1da2868914
Cr-Commit-Position: refs/heads/master@{#14507}

Powered by Google App Engine
This is Rietveld 408576698