Removing the implicit delete from Terminate and Kill.

Removing the implicit delete from Terminate and Kill.

This is due to the fact that after calling these methods the calling code is not able to read the buffered stdout and stderr output since the process has been removed completely. I ran into this in a few testing scenarios where I needed to kill Chrome after some timeout period but wasn't able to read the output after the call.

BUG=

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

[Mike Meade, 2015-03-06 17:18:42]

mmeade@chromium.org changed reviewers:
+ maruel@chromium.org

[Mike Meade, 2015-03-06 17:18:43]

https://codereview.chromium.org/985503004/diff/1/testing/legion/examples/subp...
File testing/legion/examples/subprocess/subprocess_test.py (right):

https://codereview.chromium.org/985503004/diff/1/testing/legion/examples/subp...
testing/legion/examples/subprocess/subprocess_test.py:35: dimensions={'os':
'Ubuntu-14.04', 'pool': 'Chromoting'},
We merged the Legion machines in with the Chromoting machines since they are
configured to be able to run the browser_tests. There was no reason to maintain
2 sets of machines with different pool names.

[M-A Ruel, 2015-03-09 19:50:35]

https://codereview.chromium.org/985503004/diff/60001/testing/legion/rpc_metho...
File testing/legion/rpc_methods.py (right):

https://codereview.chromium.org/985503004/diff/60001/testing/legion/rpc_metho...
testing/legion/rpc_methods.py:128: def SetCwd(cls, key, cwd):
Why not set it at Process() call instead?

https://codereview.chromium.org/985503004/diff/60001/testing/legion/rpc_metho...
testing/legion/rpc_methods.py:141: """Sets the stdout and stderr to be emitted
locally."""
I'm not sure why it is useful that this is exposed? When would you switch this?

[Mike Meade, 2015-03-09 20:24:36]

https://codereview.chromium.org/985503004/diff/60001/testing/legion/rpc_metho...
File testing/legion/rpc_methods.py (right):

https://codereview.chromium.org/985503004/diff/60001/testing/legion/rpc_metho...
testing/legion/rpc_methods.py:128: def SetCwd(cls, key, cwd):
On 2015/03/09 19:50:35, M-A Ruel wrote:
> Why not set it at Process() call instead?

I am expecting there to be more ctor args, and since XML-RPC doesn't support
keyword args the entire set of arguments needs to be passed if you would
normally specify it by keyword (Popen(cmd, cwd='foo').

For example currently we have cmd, cwd, detached. If we had a single method that
takes (cmd, cwd=None, detached=False) and the user wanted to make it detached
they would have to know the full signature and pass ([cmd], None, True).

This doesn't seem to scale well if I am right about the ctor args getting more
complex.

https://codereview.chromium.org/985503004/diff/60001/testing/legion/rpc_metho...
testing/legion/rpc_methods.py:141: """Sets the stdout and stderr to be emitted
locally."""
On 2015/03/09 19:50:35, M-A Ruel wrote:
> I'm not sure why it is useful that this is exposed? When would you switch
this?

Its been useful to have the process print out the stdout/stderr on the task
machine when developing/running the chromoting integration tests on a task
machine. I was simply able to run the command and pull up the task web page and
see the output in real time. When we get 2 and 3 machines it will help to keep
the outputs separate while running.

However when running multiple processes on a single machine (test + web server)
its useful to suppress one or more outputs to make reading a single process's
output easier.

There just seem to be too many variations and too easy to get confused to not
support the ability to enable and disable echo'ing of stdout/stderr on the task
machine. I would rather give that power to the developer.

[M-A Ruel, 2015-03-09 20:32:17]

Ok, lgtm

[Mike Meade, 2015-03-10 16:20:48]

The CQ bit was checked by mmeade@chromium.org

[commit-bot: I haz the power, 2015-03-10 16:21:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/985503004/60001

[commit-bot: I haz the power, 2015-03-10 16:56:33]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-10 16:57:19]

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