Remove psutil from the mini_installer test framework.

Remove psutil from the mini_installer test framework.

The mini_installer test framework used the psutil module to find process IDs and paths, but this module was not available on the buildslave. Remove psutil and call Windows API directly instead.

NOTRY=True
BUG=264859
1) Uninstall Chrome (if it's installed.)
2) Put mini_installer.exe in the same folder as test_installer.py
3) Use Python 2.6.x. Make sure that psutil is not installed. Run "python test_installer.py config\config.config". The test should pass.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=220650

[sukolsak, 2013-08-30 06:44:33]

Could you please take a look?

[robertshield, 2013-08-30 12:29:14]

lgtm, do you also need to change process_verifier.py?

[sukolsak, 2013-08-30 13:01:06]

On 2013/08/30 12:29:14, robertshield wrote:
> lgtm, do you also need to change process_verifier.py?

Oops, I completely forgot about it. Fixed now. Thank you.

[gab, 2013-08-30 16:20:21]

lgtm

I'm curious, if we know about these issues does that mean our FYI bot is up?! Am
I late on good news :)?!

Cheers,
Gab

https://codereview.chromium.org/23814002/diff/5001/chrome/test/mini_installer...
File chrome/test/mini_installer/chrome_helper.py (right):

https://codereview.chromium.org/23814002/diff/5001/chrome/test/mini_installer...
chrome/test/mini_installer/chrome_helper.py:18: """Returns a list of 2-tuples of
(process id, process path)."""
Nice work; perhaps add a comment here saying that this is needed because psutil
is only available as of python 2.7 (with a link to the infrastructure bug)?

So that if someone in the future wonders why this doesn't use the much simpler
psutil approach instead they can know that it's safe to replace this with psutil
as long as python 2.7 made it onto the bots?

[sukolsak, 2013-08-30 19:03:36]

Thanks for your review. Yes, the bot is up :) But it's still not working because
of the psutil issue.
http://build.chromium.org/p/chromium.fyi/builders/Chromium%20Win%20MiniInstal...

https://codereview.chromium.org/23814002/diff/5001/chrome/test/mini_installer...
File chrome/test/mini_installer/chrome_helper.py (right):

https://codereview.chromium.org/23814002/diff/5001/chrome/test/mini_installer...
chrome/test/mini_installer/chrome_helper.py:18: """Returns a list of 2-tuples of
(process id, process path)."""
On 2013/08/30 16:20:21, gab wrote:
> Nice work; perhaps add a comment here saying that this is needed because
psutil
> is only available as of python 2.7 (with a link to the infrastructure bug)?
> 
> So that if someone in the future wonders why this doesn't use the much simpler
> psutil approach instead they can know that it's safe to replace this with
psutil
> as long as python 2.7 made it onto the bots?

Done. psutil is available on Python 2.6, but it's not installed on Windows slave
machines (see https://code.google.com/p/chromium/issues/detail?id=257696.) It
seems unlikely to be fixed anytime soon.

[gab, 2013-08-30 19:19:09]

On 2013/08/30 19:03:36, sukolsak wrote:
> Thanks for your review. Yes, the bot is up :) But it's still not working
because
> of the psutil issue.
>
http://build.chromium.org/p/chromium.fyi/builders/Chromium%2520Win%2520MiniIn...

That's awesome :)!! Congrats! LGTM++

> 
>
https://codereview.chromium.org/23814002/diff/5001/chrome/test/mini_installer...
> File chrome/test/mini_installer/chrome_helper.py (right):
> 
>
https://codereview.chromium.org/23814002/diff/5001/chrome/test/mini_installer...
> chrome/test/mini_installer/chrome_helper.py:18: """Returns a list of 2-tuples
of
> (process id, process path)."""
> On 2013/08/30 16:20:21, gab wrote:
> > Nice work; perhaps add a comment here saying that this is needed because
> psutil
> > is only available as of python 2.7 (with a link to the infrastructure bug)?
> > 
> > So that if someone in the future wonders why this doesn't use the much
simpler
> > psutil approach instead they can know that it's safe to replace this with
> psutil
> > as long as python 2.7 made it onto the bots?
> 
> Done. psutil is available on Python 2.6, but it's not installed on Windows
slave
> machines (see https://code.google.com/p/chromium/issues/detail?id=257696.) It
> seems unlikely to be fixed anytime soon.

[grt (UTC plus 2), 2013-08-30 19:25:09]

lgtm

[Mathieu, 2013-08-30 19:34:10]

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
File chrome/test/mini_installer/chrome_helper.py (right):

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
chrome/test/mini_installer/chrome_helper.py:21:
https://code.google.com/p/chromium/issues/detail?id=257696). When it becomes
How about:

This is needed because psutil is not available on Windows slave machines (see:
http://crbug.com/257696).
TODO(sukolsak): Use psutil.process_iter() once it becomes available.

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
chrome/test/mini_installer/chrome_helper.py:26: process_handle =
ctypes.windll.kernel32.OpenProcess(
Make sure this never produces an exception

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
chrome/test/mini_installer/chrome_helper.py:53: return process_ids
How about:
return [pid for (pid, path) in GetProcessIDAndPathPairs() if
        path == process_path]

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
File chrome/test/mini_installer/process_verifier.py (right):

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
chrome/test/mini_installer/process_verifier.py:23: running_process_paths = []
running_process_paths = map(lambda t: t[1], 
                            chrome_helper.GetProcessIDAndPathPairs())

if you think that's not too cryptic.

A list comprehension would also do it.

[sukolsak, 2013-08-30 20:06:07]

Thanks. I like Python's list comprehensions :-)

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
File chrome/test/mini_installer/chrome_helper.py (right):

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
chrome/test/mini_installer/chrome_helper.py:21:
https://code.google.com/p/chromium/issues/detail?id=257696). When it becomes
On 2013/08/30 19:34:10, Mathieu Perreault wrote:
> How about:
> 
> This is needed because psutil is not available on Windows slave machines (see:
> http://crbug.com/257696).
> TODO(sukolsak): Use psutil.process_iter() once it becomes available.

Done.

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
chrome/test/mini_installer/chrome_helper.py:26: process_handle =
ctypes.windll.kernel32.OpenProcess(
On 2013/08/30 19:34:10, Mathieu Perreault wrote:
> Make sure this never produces an exception

OpenProcess doesn't produce an exception. It returns NULL when it fails.

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
chrome/test/mini_installer/chrome_helper.py:53: return process_ids
On 2013/08/30 19:34:10, Mathieu Perreault wrote:
> How about:
> return [pid for (pid, path) in GetProcessIDAndPathPairs() if
>         path == process_path]

Done.

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
File chrome/test/mini_installer/process_verifier.py (right):

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
chrome/test/mini_installer/process_verifier.py:23: running_process_paths = []
On 2013/08/30 19:34:10, Mathieu Perreault wrote:
> running_process_paths = map(lambda t: t[1], 
>                             chrome_helper.GetProcessIDAndPathPairs())
> 
> if you think that's not too cryptic.
> 
> A list comprehension would also do it.

Done.

[Mathieu, 2013-08-30 20:11:15]

lgtm with a question below. I trust you'll do the right thing :)

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
File chrome/test/mini_installer/chrome_helper.py (right):

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
chrome/test/mini_installer/chrome_helper.py:26: process_handle =
ctypes.windll.kernel32.OpenProcess(
On 2013/08/30 20:06:07, sukolsak wrote:
> On 2013/08/30 19:34:10, Mathieu Perreault wrote:
> > Make sure this never produces an exception
> 
> OpenProcess doesn't produce an exception. It returns NULL when it fails.

Do you mean None or the string 'NULL'? Verify that it does what you expect it to
do (that is, not add it to process_id_and_path_pairs)

[sukolsak, 2013-08-30 20:22:02]

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
File chrome/test/mini_installer/chrome_helper.py (right):

https://codereview.chromium.org/23814002/diff/4001/chrome/test/mini_installer...
chrome/test/mini_installer/chrome_helper.py:26: process_handle =
ctypes.windll.kernel32.OpenProcess(
On 2013/08/30 20:11:15, Mathieu Perreault wrote:
> On 2013/08/30 20:06:07, sukolsak wrote:
> > On 2013/08/30 19:34:10, Mathieu Perreault wrote:
> > > Make sure this never produces an exception
> > 
> > OpenProcess doesn't produce an exception. It returns NULL when it fails.
> 
> Do you mean None or the string 'NULL'? Verify that it does what you expect it
to
> do (that is, not add it to process_id_and_path_pairs)

Sorry, I mean 0 (int). I have verified it with "print type(process_handle)". It
does what I expect it to do.

[commit-bot: I haz the power, 2013-08-30 20:22:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sukolsak@chromium.org/23814002/21001

[commit-bot: I haz the power, 2013-08-30 20:51:33]

Change committed as 220650