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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1290913004: Add a library for running external commands, providing timeouts and test injection. (Closed)

Created:
5 years, 4 months ago by dogben
Modified:
5 years, 4 months ago
Reviewers:
jcgregorio, rmistry
CC:
reviews_skia.org
Base URL:
https://skia.googlesource.com/buildbot@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Add a library for running external commands, providing timeouts and test injection. BUG=skia:4106 Committed: https://skia.googlesource.com/buildbot/+/b65fde6749a3e060938cd280c7e281e29516edc1

Patch Set 1 : #

Total comments: 2

Patch Set 2 : Remove temp directories in tests. #

Patch Set 3 : Fix bug with nil io.Writer. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+547 lines, -85 lines) Patch
M ct/go/adb/adb.go View 3 chunks +3 lines, -4 lines 0 comments Download
M ct/go/util/util.go View 1 2 2 chunks +17 lines, -59 lines 0 comments Download
A go/exec/exec.go View 1 2 1 chunk +235 lines, -0 lines 0 comments Download
A go/exec/exec_test.go View 1 2 1 chunk +289 lines, -0 lines 0 comments Download
M go/util/command.go View 1 chunk +3 lines, -22 lines 0 comments Download

Messages

Total messages: 10 (4 generated)
dogben
5 years, 4 months ago (2015-08-14 13:18:22 UTC) #3
jcgregorio
LGTM modulo the nit of cleaning up dirs in tests. I'm looking forward to converting ...
5 years, 4 months ago (2015-08-14 13:41:30 UTC) #4
dogben
https://codereview.chromium.org/1290913004/diff/20001/go/exec/exec_test.go File go/exec/exec_test.go (right): https://codereview.chromium.org/1290913004/diff/20001/go/exec/exec_test.go#newcode70 go/exec/exec_test.go:70: dir, err := ioutil.TempDir("", "exec_test") On 2015/08/14 13:41:30, jcgregorio ...
5 years, 4 months ago (2015-08-14 14:03:30 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1290913004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1290913004/40001
5 years, 4 months ago (2015-08-14 17:31:23 UTC) #8
commit-bot: I haz the power
Committed patchset #2 (id:40001) as https://skia.googlesource.com/buildbot/+/b65fde6749a3e060938cd280c7e281e29516edc1
5 years, 4 months ago (2015-08-14 17:32:40 UTC) #9
rmistry
5 years, 4 months ago (2015-08-15 18:08:07 UTC) #10
Message was sent while issue was closed. 
A revert of this CL (patchset #2 id:40001) has been created in
https://codereview.chromium.org/1295043002/ by rmistry@google.com.

The reason for reverting is: CT Chromium Perf tasks are failing with:

E0815 11:07:22.649344   13091 exec.go:193] Command exited with invalid argument:
python /b/skia-repo/go/src/go.skia.org/infra/ct/py/sync_skia_in_chrome.py
--destination=/b/storage/chromium_builds/linux_base --fetch_target=chromium
--chrome_revision=6ae54795f6d077e31f4fb42540ead46f4141f2bb
--skia_revision=fb28cd2b13d76c324570341e1155aaadc1b532b6.

Powered by Google App Engine
This is Rietveld 408576698