Issue 11093033: Added ability to have custom test HTTP server handlers in the test directories.

Issue 11093033: Added ability to have custom test HTTP server handlers in the test directories. (Closed)

Created:
8 years, 2 months ago by gram

Modified:
8 years, 2 months ago

Reviewers:
Siggi Cherem (dart-lang)

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Added ability to have custom test HTTP server handlers in the test directories. Committed: https://code.google.com/p/dart/source/detail?r=13490

Patch Set 2 : #

Total comments: 8

Created: 8 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+167 lines, -224 lines)			Patch
A	utils/testrunner/http_server.dart	View		1 chunk	+103 lines, -0 lines	2 comments	Download
A +	utils/testrunner/http_server_runner.dart	View		2 chunks	+3 lines, -98 lines	0 comments	Download
D	utils/testrunner/http_server_test_runner.dart	View		1 chunk	+0 lines, -116 lines	0 comments	Download
M	utils/testrunner/options.dart	View		1 chunk	+1 line, -1 line	0 comments	Download
M	utils/testrunner/pipeline_utils.dart	View		1 chunk	+17 lines, -0 lines	2 comments	Download
M	utils/testrunner/run_pipeline.dart	View	1	1 chunk	+19 lines, -5 lines	2 comments	Download
M	utils/testrunner/testrunner.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	utils/testrunner/utils.dart	View	1	2 chunks	+23 lines, -3 lines	2 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

gram

There isn't yet a facility to communicate the port number to the client; I'll look ...

8 years, 2 months ago (2012-10-10 00:40:09 UTC) #1

Siggi Cherem (dart-lang)

lgtm (with minor comments below) https://codereview.chromium.org/11093033/diff/1001/utils/testrunner/http_server.dart File utils/testrunner/http_server.dart (right): https://codereview.chromium.org/11093033/diff/1001/utils/testrunner/http_server.dart#newcode103 utils/testrunner/http_server.dart:103: remove empty line https://codereview.chromium.org/11093033/diff/1001/utils/testrunner/pipeline_utils.dart ...

8 years, 2 months ago (2012-10-10 17:51:32 UTC) #2

gram

8 years, 2 months ago (2012-10-17 00:03:28 UTC) #3

Old drafts sitting around :-)

http://codereview.chromium.org/11093033/diff/1001/utils/testrunner/http_serve...
File utils/testrunner/http_server.dart (right):

http://codereview.chromium.org/11093033/diff/1001/utils/testrunner/http_serve...
utils/testrunner/http_server.dart:103: 
On 2012/10/10 17:51:32, sigmund wrote:
> remove empty line

Done.

http://codereview.chromium.org/11093033/diff/1001/utils/testrunner/pipeline_u...
File utils/testrunner/pipeline_utils.dart (right):

http://codereview.chromium.org/11093033/diff/1001/utils/testrunner/pipeline_u...
utils/testrunner/pipeline_utils.dart:38: String getDirectory(String file) =>
getAbsolutePath(file).directoryPath.toString();
On 2012/10/10 17:51:32, sigmund wrote:
> 80 col

Done.

http://codereview.chromium.org/11093033/diff/1001/utils/testrunner/run_pipeli...
File utils/testrunner/run_pipeline.dart (right):

http://codereview.chromium.org/11093033/diff/1001/utils/testrunner/run_pipeli...
utils/testrunner/run_pipeline.dart:257: serverPath = '${serverPath.substring(0,
serverPath.length-5)}_server.dart';
On 2012/10/10 17:51:32, sigmund wrote:
> nit: spaces around '-', I might prefer '.dart'.length since it's a bit more
> explicit.

Done.

http://codereview.chromium.org/11093033/diff/1001/utils/testrunner/utils.dart
File utils/testrunner/utils.dart (right):

http://codereview.chromium.org/11093033/diff/1001/utils/testrunner/utils.dart...
utils/testrunner/utils.dart:44: * If [symLinks] is false, symlinks will be
excluded.
On 2012/10/10 17:51:32, sigmund wrote:
> I rather name the variable 'includeSymLinks' and remove this comment :)

Done.

Expand Messages | Collapse Messages