Basic framework for devtools-based scrolling tests.

Basic framework for devtools-based scrolling tests.

BUG=144483
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=153616

[dtu, 2012-08-24 08:55:43]

Some changes that made the unit tests pass on my machine.

https://chromiumcodereview.appspot.com/10875044/diff/1/tools/chrome_remote_co...
File tools/chrome_remote_control/chrome_remote_control/browser_finder.py
(right):

https://chromiumcodereview.appspot.com/10875044/diff/1/tools/chrome_remote_co...
tools/chrome_remote_control/chrome_remote_control/browser_finder.py:44:
mac_canary = "/Applications/Google Chrome Canary.app/" +
Parentheses around these string literals, and you don't need the +

https://chromiumcodereview.appspot.com/10875044/diff/1/tools/chrome_remote_co...
File
tools/chrome_remote_control/chrome_remote_control/browser_finder_unittest.py
(right):

https://chromiumcodereview.appspot.com/10875044/diff/1/tools/chrome_remote_co...
tools/chrome_remote_control/chrome_remote_control/browser_finder_unittest.py:7:
import browser_finder
import browser_options

https://chromiumcodereview.appspot.com/10875044/diff/1/tools/chrome_remote_co...
tools/chrome_remote_control/chrome_remote_control/browser_finder_unittest.py:49:
self._options = browser.BrowserOptions()
browser_options.BrowserOptions()

https://chromiumcodereview.appspot.com/10875044/diff/1/tools/chrome_remote_co...
File tools/chrome_remote_control/chrome_remote_control/run_tests.py (right):

https://chromiumcodereview.appspot.com/10875044/diff/1/tools/chrome_remote_co...
tools/chrome_remote_control/chrome_remote_control/run_tests.py:5: 
import fnmatch

https://chromiumcodereview.appspot.com/10875044/diff/1/tools/chrome_remote_co...
tools/chrome_remote_control/chrome_remote_control/run_tests.py:8: import sys
import traceback

https://chromiumcodereview.appspot.com/10875044/diff/1/tools/chrome_remote_co...
File tools/chrome_remote_control/examples/rendering_microbenchmark_test.py
(right):

https://chromiumcodereview.appspot.com/10875044/diff/1/tools/chrome_remote_co...
tools/chrome_remote_control/examples/rendering_microbenchmark_test.py:15: class
TestUtils(object):
Class is never used. Remove.

https://chromiumcodereview.appspot.com/10875044/diff/1/tools/chrome_remote_co...
tools/chrome_remote_control/examples/rendering_microbenchmark_test.py:33: if not
url.startswith("http://"):
if not url.startswith("http://") and not url.startswith("https://"):

[alokp, 2012-08-27 18:13:33]

I think it will be better to move:
tools/chrome_remote_control/chrome_remote_control to
tools/chrome_remote_control/src

[nduca, 2012-08-27 18:43:35]

Thats not how python modules work. If you did that, then a library using chrome
remote control would say

import src

which isn't quite right either.

Some projects solve this by making their directory layout be
src/<project_name>

or
pylib/<project_name>

or
<source code right in the root>

I still think a source directory is good, so this is the layout that I think was
the best compromise given the constraints.

[alokp, 2012-08-27 19:38:31]

lgtm

[commit-bot: I haz the power, 2012-08-27 21:23:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nduca@chromium.org/10875044/8001

[dtu, 2012-08-27 22:27:27]

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
File tools/chrome_remote_control/chrome_remote_control/browser.py (right):

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/browser.py:31: self.Close()
Makes more sense in __del__, so it'll take effect for all instances, not just
those in with statements.

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
File tools/chrome_remote_control/chrome_remote_control/browser_finder.py
(right):

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/browser_finder.py:11: """Finds
browsers that can be used controlled by chrome_remote_control."""
Remove "used"

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/browser_finder.py:17: """A
browser that can be controled.
controlled

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
File
tools/chrome_remote_control/chrome_remote_control/desktop_browser_backend.py
(right):

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/desktop_browser_backend.py:25:
# TODO(nduca): Delete the user data dir or create one that is unique.
This is just tempfile.mkdtemp(). And then delete it on cleanup.

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
File tools/chrome_remote_control/chrome_remote_control/local_browser.py (right):

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/local_browser.py:1: #
Copyright (c) 2012 The Chromium Authors. All rights reserved.
Was this file intended to be committed? Seems redundant with browser.py.

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/local_browser.py:14: """A
running browser instance that can be controled in a limited way by
controlled

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
File tools/chrome_remote_control/chrome_remote_control/run_tests.py (right):

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/run_tests.py:1: #!/usr/bin/env
python
Is this intended to be used for all tests, or just the chrome_remote_control
unit tests?

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/run_tests.py:34: module =
__import__(fqn,fromlist=[True])
Space after comma.

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
File tools/chrome_remote_control/chrome_remote_control/tab.py (right):

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/tab.py:57: self.Close()
Makes more sense in __del__, so it'll take effect for all instances, not just
those in with statements.

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/tab.py:65: while True:
while self.runtime.Evaluate("document.location.href") != "about:blank":

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/tab.py:72: while True:
while self.runtime.Evaluate("document.location.href") == "about:blank":

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/tab.py:79: while True:
while self.runtime.Evaluate("document.readyState") != 'complete':

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
File tools/chrome_remote_control/examples/rendering_microbenchmark_test.py
(right):

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/examples/rendering_microbenchmark_test.py:38: with
browser_to_create.CreateBrowser() as b:
Create()

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/examples/rendering_microbenchmark_test.py:43: if
tab.RuntimeEvaluate("window.chrome.gpuBenchmarking === undefined"):
tab.runtime.Evaluate() (3 times in this file)

[nduca, 2012-08-28 00:20:16]

Thanks d2, great feedback. Mostly all applied.

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
File tools/chrome_remote_control/chrome_remote_control/browser.py (right):

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/browser.py:31: self.Close()
Remember, __del__ is tied to when GC runs, not when the object is deleted.

I'm pretty sure that the with/enter/exit/close idiom is the correct one to use
here. browser objects are closer to files than they are to regular objects.
Their shutdown is explicit, not something you want to be at the mercy of the
garbage collector. E.g. you dont want the third loop through a test to suddenly
kill an old browser instance.

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
File tools/chrome_remote_control/chrome_remote_control/local_browser.py (right):

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/local_browser.py:1: #
Copyright (c) 2012 The Chromium Authors. All rights reserved.
Thanks!

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
File tools/chrome_remote_control/chrome_remote_control/run_tests.py (right):

http://codereview.chromium.org/10875044/diff/8001/tools/chrome_remote_control...
tools/chrome_remote_control/chrome_remote_control/run_tests.py:1: #!/usr/bin/env
python
Just remote control unit tests. I think we'd dupe it to our rendering tests
folder. Wdyt?

[dtu, 2012-08-28 00:47:45]

lgtm

[commit-bot: I haz the power, 2012-08-28 00:49:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nduca@chromium.org/10875044/10006

[commit-bot: I haz the power, 2012-08-28 03:13:29]

Change committed as 153616