Issue 9960050: Add runtime=dartium webdriver support to test.dart

Issue 9960050: Add runtime=dartium webdriver support to test.dart (Closed)

Created:
8 years, 8 months ago by vsm

Modified:
8 years, 8 months ago

Reviewers:
Emily Fortuna, William Hesse

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Add runtime=dartium webdriver support to test.dart Committed: https://code.google.com/p/dart/source/detail?r=6381

Patch Set 2 : Fix for batching #

Total comments: 3

Patch Set 3 : Address comment #

Created: 8 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+92 lines, -36 lines)			Patch
M	tools/testing/dart/drt_updater.dart	View		1 chunk	+44 lines, -13 lines	0 comments	Download
M	tools/testing/dart/test_options.dart	View	1	5 chunks	+20 lines, -8 lines	0 comments	Download
M	tools/testing/dart/test_runner.dart	View	1	1 chunk	+2 lines, -1 line	0 comments	Download
M	tools/testing/dart/test_suite.dart	View	1	4 chunks	+9 lines, -6 lines	0 comments	Download
M	tools/testing/run_selenium.py	View	1 2	6 chunks	+17 lines, -8 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

Emily Fortuna

lgtm! Very exciting! https://chromiumcodereview.appspot.com/9960050/diff/1/tools/testing/dart/test_options.dart File tools/testing/dart/test_options.dart (right): https://chromiumcodereview.appspot.com/9960050/diff/1/tools/testing/dart/test_options.dart#newcode394 tools/testing/dart/test_options.dart:394: 'ie', 'opera', 'none']).indexOf(config['runtime']) >= 0; > ...

8 years, 8 months ago (2012-04-09 23:56:20 UTC) #2

vsm

Emily, can you take a quick look at Patch #2? I had inadvertently broken batch ...

8 years, 8 months ago (2012-04-10 20:00:45 UTC) #3

Emily Fortuna

I'm glad you caught that. lgtm with comment! https://chromiumcodereview.appspot.com/9960050/diff/5001/tools/testing/run_selenium.py File tools/testing/run_selenium.py (right): https://chromiumcodereview.appspot.com/9960050/diff/5001/tools/testing/run_selenium.py#newcode261 tools/testing/run_selenium.py:261: timeout, ...

8 years, 8 months ago (2012-04-10 20:47:45 UTC) #4

vsm

https://chromiumcodereview.appspot.com/9960050/diff/5001/tools/testing/run_selenium.py File tools/testing/run_selenium.py (right): https://chromiumcodereview.appspot.com/9960050/diff/5001/tools/testing/run_selenium.py#newcode261 tools/testing/run_selenium.py:261: timeout, mode) = parse_args(line.split()) On 2012/04/10 20:47:45, Emily Fortuna ...

8 years, 8 months ago (2012-04-10 22:11:44 UTC) #5

Emily Fortuna

8 years, 8 months ago (2012-04-10 22:22:59 UTC) #6

https://chromiumcodereview.appspot.com/9960050/diff/5001/tools/testing/run_se...
File tools/testing/run_selenium.py (right):

https://chromiumcodereview.appspot.com/9960050/diff/5001/tools/testing/run_se...
tools/testing/run_selenium.py:261: timeout, mode) = parse_args(line.split())
On 2012/04/10 22:11:44, vsm wrote:
> On 2012/04/10 20:47:45, Emily Fortuna wrote:
> > should this line be indented? My read of the Google Python style is unclear
> > here.
> 
> Going by the Google python elisp style.  Note, 'timeout' is aligned with
> 'html_out'.
> 
> > 
> > Also, nit: can we call "browser_path" "executable_path" or some other name?
> When
> > I see "browser_path" I think of the URL that the browser is visiting instead
> of
> > the path to open the browser exeutable. But, it's fine if you prefer to keep
> the
> > name.
> 
> Done.

sgtm.

Expand Messages | Collapse Messages