Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1134703002:
[test-runner] Improve test execution without tmp files for output. (Closed)
|
Created:
5 years, 7 months ago by Michael Achenbach Modified:
5 years, 7 months ago CC:
v8-dev Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[test-runner] Improve test execution without tmp files for output.
BUG=chromium:485932
LOG=n
Committed: https://crrev.com/0b81f67b12550c118f1a94528c73e1cec01263a2
Cr-Commit-Position: refs/heads/master@{#28334}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Review #
Total comments: 2
Patch Set 3 : nit #Messages
Total messages: 18 (6 generated)
machenbach@chromium.org changed reviewers: + jkummerow@chromium.org, tandrii@chromium.org
PTAL This leads to a 5% speedup on linux workstations and a > 40% speedup on the windows bots! Experiment to demonstrate the behavior with timeouts: https://codereview.chromium.org/1131443003/
lgtm % comment https://codereview.chromium.org/1134703002/diff/1/tools/testrunner/local/comm... File tools/testrunner/local/commands.py (right): https://codereview.chromium.org/1134703002/diff/1/tools/testrunner/local/comm... tools/testrunner/local/commands.py:75: process.kill() This will raise exception if process is already dead. I think it should be caught and maybe just 1 line to be printed. This is my test on my Linux machine: >>> p = S.Popen(["cat", "~/.netrc"], stdout=S.PIPE, stderr=S.PIPE) >>> stdout, stderr = p.communicate() 'cat: ~/.netrc: No such file or directory\n' >>> p.kill() Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/usr/lib/python2.7/subprocess.py", line 1556, in kill self.send_signal(signal.SIGKILL) File "/usr/lib/python2.7/subprocess.py", line 1546, in send_signal os.kill(self.pid, sig) OSError: [Errno 3] No such process
LGTM. So much simpler!
https://codereview.chromium.org/1134703002/diff/1/tools/testrunner/local/comm... File tools/testrunner/local/commands.py (right): https://codereview.chromium.org/1134703002/diff/1/tools/testrunner/local/comm... tools/testrunner/local/commands.py:75: process.kill() On 2015/05/08 15:47:26, tandrii(chromium) wrote: > This will raise exception if process is already dead. I think it should be > caught and maybe just 1 line to be printed. > > > This is my test on my Linux machine: > > >>> p = S.Popen(["cat", "~/.netrc"], stdout=S.PIPE, stderr=S.PIPE) > >>> stdout, stderr = p.communicate() > 'cat: ~/.netrc: No such file or directory\n' > >>> p.kill() > Traceback (most recent call last): > File "<stdin>", line 1, in <module> > File "/usr/lib/python2.7/subprocess.py", line 1556, in kill > self.send_signal(signal.SIGKILL) > File "/usr/lib/python2.7/subprocess.py", line 1546, in send_signal > os.kill(self.pid, sig) > OSError: [Errno 3] No such process Done.
On 2015/05/11 08:13:00, Michael Achenbach wrote: > https://codereview.chromium.org/1134703002/diff/1/tools/testrunner/local/comm... > File tools/testrunner/local/commands.py (right): > > https://codereview.chromium.org/1134703002/diff/1/tools/testrunner/local/comm... > tools/testrunner/local/commands.py:75: process.kill() > On 2015/05/08 15:47:26, tandrii(chromium) wrote: > > This will raise exception if process is already dead. I think it should be > > caught and maybe just 1 line to be printed. > > > > > > This is my test on my Linux machine: > > > > >>> p = S.Popen(["cat", "~/.netrc"], stdout=S.PIPE, stderr=S.PIPE) > > >>> stdout, stderr = p.communicate() > > 'cat: ~/.netrc: No such file or directory\n' > > >>> p.kill() > > Traceback (most recent call last): > > File "<stdin>", line 1, in <module> > > File "/usr/lib/python2.7/subprocess.py", line 1556, in kill > > self.send_signal(signal.SIGKILL) > > File "/usr/lib/python2.7/subprocess.py", line 1546, in send_signal > > os.kill(self.pid, sig) > > OSError: [Errno 3] No such process > > Done. Lgtm %nit
tandrii@google.com changed reviewers: + tandrii@google.com
Actual nit https://codereview.chromium.org/1134703002/diff/20001/tools/testrunner/local/... File tools/testrunner/local/commands.py (right): https://codereview.chromium.org/1134703002/diff/20001/tools/testrunner/local/... tools/testrunner/local/commands.py:78: sys.stderr.write('Error: Process %s already ended.\n' % str(process.pid)) Why str(pid)? '%s' should call str() automatically.
https://codereview.chromium.org/1134703002/diff/20001/tools/testrunner/local/... File tools/testrunner/local/commands.py (right): https://codereview.chromium.org/1134703002/diff/20001/tools/testrunner/local/... tools/testrunner/local/commands.py:78: sys.stderr.write('Error: Process %s already ended.\n' % str(process.pid)) On 2015/05/11 08:19:10, tandrii_google wrote: > Why str(pid)? '%s' should call str() automatically. Done.
The CQ bit was checked by machenbach@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jkummerow@chromium.org, tandrii@chromium.org, tandrii@google.com Link to the patchset: https://codereview.chromium.org/1134703002/#ps40001 (title: "nit")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1134703002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_linux64_avx2_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel/builds/262)
The CQ bit was checked by machenbach@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1134703002/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/0b81f67b12550c118f1a94528c73e1cec01263a2 Cr-Commit-Position: refs/heads/master@{#28334} |
