Fix finding chrome binary for ChromeDriver.

Fix finding chrome binary for ChromeDriver.
NOTRY=true
BUG=none

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

[kkania, 2012-12-03 20:34:07]

Also, clean up your description for the CL.

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
File chrome/test/chromedriver/run_all_tests.py (right):

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:20: if path in lib_path:
if path not in lib_path:
  os.environ[env_name] += os.pathsep + path

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:27: def _FindChromeBinary(loc):
loc is not a very common abbreviation for location; instead use path or
build_dir

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:29: exexutables = [
should be executables, not exexutables. You can just name it 'exes' if you want

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:30: "google-chrome",
use ' instead of " for python

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:31: "chrome",
this is the only one you need

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:42:
"Google\\Chrome\\Application\\chrome.exe",
executables = ['chrome.exe']

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:66: _FindChromeBinary(build_dir),
i think we should also support, if the user hasn't built chrome, just using the
installed version of chrome

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:69:
_AppendEnvironmentPath('LD_LIBRARY_PATH', os.path.join(build_dir, 'lib'))
how about short comment here why this is needed

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:71:
os.environ['VERSIONER_PYTHON_PREFER_32_BIT'] = 'yes'
how about short comment here why this is needed

[chrisgao (Use stgao instead), 2012-12-04 00:13:59]

done.

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
File chrome/test/chromedriver/run_all_tests.py (right):

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:20: if path in lib_path:
On 2012/12/03 20:34:07, kkania wrote:
> if path not in lib_path:
>   os.environ[env_name] += os.pathsep + path

Done.

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:27: def _FindChromeBinary(loc):
On 2012/12/03 20:34:07, kkania wrote:
> loc is not a very common abbreviation for location; instead use path or
> build_dir

Done.

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:29: exexutables = [
On 2012/12/03 20:34:07, kkania wrote:
> should be executables, not exexutables. You can just name it 'exes' if you
want

Done.

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:30: "google-chrome",
On 2012/12/03 20:34:07, kkania wrote:
> use ' instead of " for python

Done.

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:31: "chrome",
On 2012/12/03 20:34:07, kkania wrote:
> this is the only one you need

Done.

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:42:
"Google\\Chrome\\Application\\chrome.exe",
On 2012/12/03 20:34:07, kkania wrote:
> executables = ['chrome.exe']

Done.

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:66: _FindChromeBinary(build_dir),
On 2012/12/03 20:34:07, kkania wrote:
> i think we should also support, if the user hasn't built chrome, just using
the
> installed version of chrome

Set the path to chrome only when it is built.
When not built, default to the one found by C++ code.

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:69:
_AppendEnvironmentPath('LD_LIBRARY_PATH', os.path.join(build_dir, 'lib'))
On 2012/12/03 20:34:07, kkania wrote:
> how about short comment here why this is needed

Done.

https://codereview.chromium.org/11414267/diff/2002/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:71:
os.environ['VERSIONER_PYTHON_PREFER_32_BIT'] = 'yes'
On 2012/12/03 20:34:07, kkania wrote:
> how about short comment here why this is needed

Done.

[kkania, 2012-12-04 01:30:25]

make your cl description a bit more detailed; this is just for finding chrome
for the tests, not actually chromedriver;

lgtm

https://codereview.chromium.org/11414267/diff/3006/chrome/test/chromedriver/r...
File chrome/test/chromedriver/run_all_tests.py (right):

https://codereview.chromium.org/11414267/diff/3006/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:57: # Set the built chrome binary
'.' at end of comments

[chrisgao (Use stgao instead), 2012-12-04 01:51:33]

https://codereview.chromium.org/11414267/diff/3006/chrome/test/chromedriver/r...
File chrome/test/chromedriver/run_all_tests.py (right):

https://codereview.chromium.org/11414267/diff/3006/chrome/test/chromedriver/r...
chrome/test/chromedriver/run_all_tests.py:57: # Set the built chrome binary
On 2012/12/04 01:30:25, kkania wrote:
> '.' at end of comments

Done.

[commit-bot: I haz the power, 2012-12-04 01:55:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/chrisgao@chromium.org/11414267/3007

[commit-bot: I haz the power, 2012-12-04 01:57:39]

Change committed as 170861