Check whether Chromium or Google Chrome is being tested.

Check whether Chromium or Google Chrome is being tested.

NOTRY=True 
BUG=264859
TEST= 
1) Uninstall Google Chrome (if it's installed.)
2) Put mini_installer.exe for Google Chrome in the same folder as test_installer.py.
3) Run "python test_installer.py config\config.config". The test should pass.
4) Uninstall Chromium (if it's installed.)
5) Put mini_installer.exe for Chromium in the same folder as test_installer.py.
6) Run "python test_installer.py config\config.config". The test should pass.

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

[sukolsak, 2013-08-29 11:19:32]

Could you please take a look?

[gab, 2013-08-29 12:31:06]

robert/grt WDYT?

https://codereview.chromium.org/23757002/diff/1/chrome/test/mini_installer/pa...
File chrome/test/mini_installer/path_resolver.py (right):

https://codereview.chromium.org/23757002/diff/1/chrome/test/mini_installer/pa...
chrome/test/mini_installer/path_resolver.py:28: return
win32api.GetFileVersionInfo(file_path, subblock)
I find it weird to have to inspect the mini_installer.exe's resources for this.

I think we have to think about how we want this to be running in the wild; can
the build system (which knows about the build's brand) bake a plain text hint in
the build output or something to let us know the build's brand?

Where do we want our scripts to live? Do we want the build system to copy them
to the build output? How do similar python tests requiring EXEs work? Do they
have the EXEs copied to them; are they copied by the EXEs; or are the two simply
linked via command-line?

[robertshield, 2013-08-29 19:01:07]

lgtm, assuming Gab doesn't violently disagree with me.

https://codereview.chromium.org/23757002/diff/1/chrome/test/mini_installer/pa...
File chrome/test/mini_installer/path_resolver.py (right):

https://codereview.chromium.org/23757002/diff/1/chrome/test/mini_installer/pa...
chrome/test/mini_installer/path_resolver.py:26: subblock =
('\\StringFileInfo\\%04x%04x\\ProductName' %
nit: subblock -> product_name_entry

https://codereview.chromium.org/23757002/diff/1/chrome/test/mini_installer/pa...
chrome/test/mini_installer/path_resolver.py:28: return
win32api.GetFileVersionInfo(file_path, subblock)
On 2013/08/29 12:31:06, gab wrote:
> I find it weird to have to inspect the mini_installer.exe's resources for
this.
> 
> I think we have to think about how we want this to be running in the wild; can
> the build system (which knows about the build's brand) bake a plain text hint
in
> the build output or something to let us know the build's brand?
>


I actually kind of like the approach taken here in the original CL.
mini_installer (and all Chrome binaries) already have a plain text hint about
the brand - it just so happens to be embedded in the version resource. Working
on the principle that duplicating data is a fundamental evil, I think inspecting
that version resource makes sense.
 
> Where do we want our scripts to live? 

In the Chrome source tree.

Do we want the build system to copy them
> to the build output? How do similar python tests requiring EXEs work? Do they
> have the EXEs copied to them; are they copied by the EXEs; or are the two
simply
> linked via command-line?

I'm curious what the value of copying the scripts would be. I think it's cleaner
to have all paths explicitly specified on the command line and not rely on
copying + the scripts and binaries being in the same directory.

I vote for paths passed explicitly on the command line.

https://codereview.chromium.org/23757002/diff/1/chrome/test/mini_installer/pa...
chrome/test/mini_installer/path_resolver.py:77: mini_installer_product_name)
This has the side benefit of breaking if mini_installers resources get messed
up. I don't believe we have anything else that tests that currently.

[gab, 2013-08-29 19:35:33]

okay lgtm then; FWIW, I worked with the layout_tests and telemetry_tests today
and they also work via command-line (i.e. the script takes as a command-line
argument the build directory).

[grt (UTC plus 2), 2013-08-30 02:39:58]

lgtm.

i am also in favor of passing the paths via command line.

https://codereview.chromium.org/23757002/diff/1/chrome/test/mini_installer/pa...
File chrome/test/mini_installer/path_resolver.py (right):

https://codereview.chromium.org/23757002/diff/1/chrome/test/mini_installer/pa...
chrome/test/mini_installer/path_resolver.py:76: raise KeyError("Unknown
mini_installer's product name '%s'" %
nit: "mini_installer's" -> "mini_installer"

[sukolsak, 2013-08-30 02:59:28]

Thank you for your review.

https://codereview.chromium.org/23757002/diff/1/chrome/test/mini_installer/pa...
File chrome/test/mini_installer/path_resolver.py (right):

https://codereview.chromium.org/23757002/diff/1/chrome/test/mini_installer/pa...
chrome/test/mini_installer/path_resolver.py:26: subblock =
('\\StringFileInfo\\%04x%04x\\ProductName' %
On 2013/08/29 19:01:07, robertshield wrote:
> nit: subblock -> product_name_entry

Done.

https://codereview.chromium.org/23757002/diff/1/chrome/test/mini_installer/pa...
chrome/test/mini_installer/path_resolver.py:76: raise KeyError("Unknown
mini_installer's product name '%s'" %
On 2013/08/30 02:39:59, grt wrote:
> nit: "mini_installer's" -> "mini_installer"

Done.

[commit-bot: I haz the power, 2013-08-30 03:05:06]

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

[commit-bot: I haz the power, 2013-08-30 06:30:24]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-08-30 06:46:58]

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

[commit-bot: I haz the power, 2013-08-30 08:54:30]

Change committed as 220538