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

Issue 1154213012: Add context-aware "time" library wrapper. (Closed)

Created:
5 years, 6 months ago by dnj
Modified:
5 years, 6 months ago
Reviewers:
iannucci
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/infra/infra.git@master
Target Ref:
refs/heads/master
Project:
infra
Visibility:
Public.

Description

Add context-aware "time" library wrapper. TEST=unittest R=iannucci@chromium.org BUG= Committed: https://chromium.googlesource.com/infra/infra/+/a6bb752f1c7a93854d3bb17c31d608282d539a69

Patch Set 1 #

Total comments: 7

Patch Set 2 : Refactored into sub-packages. #

Total comments: 12

Patch Set 3 : Refactored test timer to trigger off of underlying clock. #

Total comments: 2

Patch Set 4 : Fixed some stuff. #

Patch Set 5 : Switched GAE testing clock implementation to new clocklib. #

Patch Set 6 : Removed goroutine safety from testtimer. #

Total comments: 31

Patch Set 7 : Updated from comments. #

Patch Set 8 : (Oops) forgot a file. #

Patch Set 9 : Added coverage files. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1017 lines, -138 lines) Patch
M go/src/infra/gae/libs/wrapper/dummy_test.go View 1 2 3 4 5 6 2 chunks +14 lines, -26 lines 0 comments Download
M go/src/infra/gae/libs/wrapper/mathrand.go View 1 2 3 4 2 chunks +3 lines, -2 lines 0 comments Download
M go/src/infra/gae/libs/wrapper/memory/context.go View 1 2 3 4 5 6 4 chunks +4 lines, -6 lines 0 comments Download
M go/src/infra/gae/libs/wrapper/memory/datastore_data.go View 1 2 3 4 5 6 4 chunks +3 lines, -3 lines 0 comments Download
M go/src/infra/gae/libs/wrapper/memory/memcache.go View 1 2 3 4 5 chunks +6 lines, -5 lines 0 comments Download
M go/src/infra/gae/libs/wrapper/memory/memcache_test.go View 1 2 3 4 5 6 4 chunks +6 lines, -11 lines 0 comments Download
M go/src/infra/gae/libs/wrapper/memory/taskqueue.go View 1 2 3 4 5 6 7 chunks +10 lines, -16 lines 0 comments Download
M go/src/infra/gae/libs/wrapper/memory/taskqueue_data.go View 1 2 3 4 5 6 6 chunks +9 lines, -8 lines 0 comments Download
M go/src/infra/gae/libs/wrapper/memory/taskqueue_test.go View 1 2 3 4 5 6 7 chunks +8 lines, -13 lines 0 comments Download
D go/src/infra/gae/libs/wrapper/time.go View 1 2 3 4 1 chunk +0 lines, -46 lines 0 comments Download
M go/src/infra/gae/libs/wrapper/wrapper.infra_testing View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -2 lines 0 comments Download
A go/src/infra/libs/clock/clock.go View 1 2 3 4 5 6 1 chunk +21 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/clock.infra_testing View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/clockflag/duration.go View 1 2 3 4 5 6 1 chunk +61 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/clockflag/duration_test.go View 1 2 3 4 5 6 1 chunk +64 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/clockflag/time.go View 1 2 3 4 5 6 1 chunk +59 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/clockflag/time_test.go View 1 2 3 4 5 6 1 chunk +62 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/context.go View 1 2 3 4 5 6 1 chunk +71 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/systemclock.go View 1 2 3 4 5 6 1 chunk +39 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/systemtimer.go View 1 2 1 chunk +38 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/systemtimer_test.go View 1 2 3 4 5 6 1 chunk +65 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/testclock/context.go View 1 2 3 4 5 6 7 1 chunk +20 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/testclock/testclock.go View 1 2 3 4 5 6 1 chunk +178 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/testclock/testclock.infra_testing View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/testclock/testclock_test.go View 1 2 3 4 5 6 1 chunk +57 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/testclock/testtimer.go View 1 2 3 4 5 6 1 chunk +77 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/testclock/testtimer_test.go View 1 2 3 4 5 6 1 chunk +116 lines, -0 lines 0 comments Download
A go/src/infra/libs/clock/timer.go View 1 2 1 chunk +16 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (6 generated)
dnj
PTAL
5 years, 6 months ago (2015-06-02 03:10:42 UTC) #1
iannucci
wdyt? https://codereview.chromium.org/1154213012/diff/1/go/src/infra/libs/clock/clock.go File go/src/infra/libs/clock/clock.go (right): https://codereview.chromium.org/1154213012/diff/1/go/src/infra/libs/clock/clock.go#newcode22 go/src/infra/libs/clock/clock.go:22: // Implementation of Clock that uses Go's standard ...
5 years, 6 months ago (2015-06-02 06:15:34 UTC) #2
dnj
The one thing we lose for sure is the ability to have the context default ...
5 years, 6 months ago (2015-06-02 17:03:12 UTC) #4
iannucci
talked about some stuff offline. Basically the proposal for timers is to have a testing ...
5 years, 6 months ago (2015-06-02 22:19:03 UTC) #5
dnj
Updated! https://chromiumcodereview.appspot.com/1154213012/diff/40001/go/src/infra/libs/clock/systemclock/systemclock.go File go/src/infra/libs/clock/systemclock/systemclock.go (right): https://chromiumcodereview.appspot.com/1154213012/diff/40001/go/src/infra/libs/clock/systemclock/systemclock.go#newcode1 go/src/infra/libs/clock/systemclock/systemclock.go:1: // Copyright (c) 2014 The Chromium Authors. All ...
5 years, 6 months ago (2015-06-03 00:25:28 UTC) #7
dnj
Oh forgot to note: updated gae tests. Also, FWIW golang Timer isn't goroutine-safe. Think I ...
5 years, 6 months ago (2015-06-03 03:41:43 UTC) #8
dnj
Updated again to make testtimer not goroutine-safe. I wouldn't want to give the false impression ...
5 years, 6 months ago (2015-06-03 17:03:36 UTC) #9
iannucci
lg, though I think a lot of the names can improve now that you're all ...
5 years, 6 months ago (2015-06-03 17:37:20 UTC) #10
dnj
Updated! https://codereview.chromium.org/1154213012/diff/140001/go/src/infra/gae/libs/wrapper/dummy_test.go File go/src/infra/gae/libs/wrapper/dummy_test.go (right): https://codereview.chromium.org/1154213012/diff/140001/go/src/infra/gae/libs/wrapper/dummy_test.go#newcode27 go/src/infra/gae/libs/wrapper/dummy_test.go:27: c = clock.SetClock(c, tc) On 2015/06/03 17:37:19, iannucci ...
5 years, 6 months ago (2015-06-03 18:21:27 UTC) #11
iannucci
lgtm
5 years, 6 months ago (2015-06-04 00:20:47 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1154213012/180001
5 years, 6 months ago (2015-06-04 00:21:09 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: infra_tester on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/infra_tester/builds/1915)
5 years, 6 months ago (2015-06-04 00:26:52 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1154213012/200001
5 years, 6 months ago (2015-06-04 00:31:33 UTC) #19
commit-bot: I haz the power
5 years, 6 months ago (2015-06-04 00:34:33 UTC) #20
Message was sent while issue was closed.
Committed patchset #9 (id:200001) as
https://chromium.googlesource.com/infra/infra/+/a6bb752f1c7a93854d3bb17c31d60...

Powered by Google App Engine
This is Rietveld 408576698