Add client/tests/client web tests to the test.dart test script.

Add client/tests/client web tests to the test.dart test script.


BUG=
TEST=tools/test.dart --component=dartium,chromium,frogium client

Committed: https://code.google.com/p/dart/source/detail?r=2939

[Bill Hesse, 2011-12-23 15:05:25]

http://codereview.chromium.org/9036009/diff/1/tools/testing/dart/test_suite.dart
File tools/testing/dart/test_suite.dart (right):

http://codereview.chromium.org/9036009/diff/1/tools/testing/dart/test_suite.d...
tools/testing/dart/test_suite.dart:375: 
Eliminate stray whitespace.

http://codereview.chromium.org/9036009/diff/1/tools/testing/dart/test_suite.d...
tools/testing/dart/test_suite.dart:496: dartWrapperFilename = testPath;
Rename to dartScript?

http://codereview.chromium.org/9036009/diff/1/tools/testing/dart/test_suite.d...
tools/testing/dart/test_suite.dart:497: // return;  // TODO(whesse): Implement
client web tests on dartium.
Delete this comment.

[ahe, 2012-01-02 10:42:23]

LGTM with some comments about Windows.

http://codereview.chromium.org/9036009/diff/8001/client/tests/client/samples/...
File client/tests/client/samples/dartcombat/dartcombat_tests.dart (right):

http://codereview.chromium.org/9036009/diff/8001/client/tests/client/samples/...
client/tests/client/samples/dartcombat/dartcombat_tests.dart:24: link.href =
'../../../client/samples/dartcombat/dartcombat.css';
Why this change?

http://codereview.chromium.org/9036009/diff/8001/client/tests/dartc/dartc.status
File client/tests/dartc/dartc.status (right):

http://codereview.chromium.org/9036009/diff/8001/client/tests/dartc/dartc.sta...
client/tests/dartc/dartc.status:7: tests/dartc/test_config.dart: Skip      # Not
a test case.
Wouldn't it be better to filter out the name "test_config.dart"?

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/browser_t...
File tools/testing/dart/browser_test.dart (right):

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/browser_t...
tools/testing/dart/browser_test.dart:44: <script type="$scriptType"
src="$sourceScript"></script>
Why this change?

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
File tools/testing/dart/test_suite.dart (right):

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:349: // if (isWebTest) tempDirTemplate =
'client/' + tempDirTemplate;
Delete this line?

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:359: scriptPath = 'file://$dartTestFilename';
This doesn't work on Windows. Please add an abstraction to compute this and
consider adding it to the File interface.

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:375: 
Trailing whitespace.

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:505: // return;  // TODO(whesse): Implement
client web tests on dartium.
Remove this line.

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:590: return
filename.replaceAll(pathSeparator, '_') + 'dartium.html';
This doesn't work on Windows. Consider that a windows file path may contain
forward slashes as well as backslashes.

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:592: return
filename.replaceAll(pathSeparator, '_') + 'chromium.html';
Ditto.

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:594: return
filename.replaceAll(pathSeparator, '_') + 'frogium.html';
Ditto.

[Bill Hesse, 2012-01-02 12:50:42]

http://codereview.chromium.org/9036009/diff/8001/client/tests/client/samples/...
File client/tests/client/samples/dartcombat/dartcombat_tests.dart (right):

http://codereview.chromium.org/9036009/diff/8001/client/tests/client/samples/...
client/tests/client/samples/dartcombat/dartcombat_tests.dart:24: link.href =
'../../../client/samples/dartcombat/dartcombat.css';
On 2012/01/02 10:42:23, ahe wrote:
> Why this change?

We want the dartcombat sample to work from from any "second cousin" directory of
its directory, not just from any first cousin directory.  This lets us put the
generated js code in [dartDir]/out/[buildDir]/generated_tests/dartcombattest, so
that all generated tests are in the build directory.

http://codereview.chromium.org/9036009/diff/8001/client/tests/dartc/dartc.status
File client/tests/dartc/dartc.status (right):

http://codereview.chromium.org/9036009/diff/8001/client/tests/dartc/dartc.sta...
client/tests/dartc/dartc.status:7: tests/dartc/test_config.dart: Skip      # Not
a test case.
On 2012/01/02 10:42:23, ahe wrote:
> Wouldn't it be better to filter out the name "test_config.dart"?

Yes, I suppose so.  But then special names are built into the system of
iterating over tests, in code, rather than in data.

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/browser_t...
File tools/testing/dart/browser_test.dart (right):

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/browser_t...
tools/testing/dart/browser_test.dart:44: <script type="$scriptType"
src="$sourceScript"></script>
On 2012/01/02 10:42:23, ahe wrote:
> Why this change?

This makes test.dart consistent with test.py.  I had thought it might be the
cause of a bug, but it wasn't, but the two files should generate the same code.

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
File tools/testing/dart/test_suite.dart (right):

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:349: // if (isWebTest) tempDirTemplate =
'client/' + tempDirTemplate;
Yes.  But the real change should be to refactor, and merge the Dartium case
(which here generates temporary directories) with the Chromium and Frogium case,
which generate subdirectories of generated_tests.

On 2012/01/02 10:42:23, ahe wrote:
> Delete this line?

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:359: scriptPath = 'file://$dartTestFilename';
On 2012/01/02 10:42:23, ahe wrote:
> This doesn't work on Windows. Please add an abstraction to compute this and
> consider adding it to the File interface.

The Path and URI libraries are the next thing I will be working on this quarter.
 This issue would be solved by using them.  I will put in a fix, but not add it
to the file interface.

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:590: return
filename.replaceAll(pathSeparator, '_') + 'dartium.html';
On 2012/01/02 10:42:23, ahe wrote:
> This doesn't work on Windows. Consider that a windows file path may contain
> forward slashes as well as backslashes.

Aaarrrrgh.

[ahe, 2012-01-02 13:00:59]

http://codereview.chromium.org/9036009/diff/8001/client/tests/dartc/dartc.status
File client/tests/dartc/dartc.status (right):

http://codereview.chromium.org/9036009/diff/8001/client/tests/dartc/dartc.sta...
client/tests/dartc/dartc.status:7: tests/dartc/test_config.dart: Skip      # Not
a test case.
On 2012/01/02 12:50:42, Bill Hesse wrote:
> On 2012/01/02 10:42:23, ahe wrote:
> > Wouldn't it be better to filter out the name "test_config.dart"?
> 
> Yes, I suppose so.  But then special names are built into the system of
> iterating over tests, in code, rather than in data.

The special names are part of the system, not the data. As far as I'm concerned,
the greater sin is to pollute the data with this information.

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
File tools/testing/dart/test_suite.dart (right):

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:359: scriptPath = 'file://$dartTestFilename';
On 2012/01/02 12:50:42, Bill Hesse wrote:
> On 2012/01/02 10:42:23, ahe wrote:
> > This doesn't work on Windows. Please add an abstraction to compute this and
> > consider adding it to the File interface.
> 
> The Path and URI libraries are the next thing I will be working on this
quarter.
>  This issue would be solved by using them.  I will put in a fix, but not add
it
> to the file interface.

SGTM, I guess a TODO would serve the same purpose.

[Bill Hesse, 2012-01-02 15:45:21]

I looks to me like the best way to handle Windows paths is to use a forward
slash everywhere, and to convert backslashes to forward slash in the (very few)
cases where we get a path with separators returned to us from the Win32 API.

We can pass the forward slash paths in to the API, and they will be converted
internally, so we only need to worry about backslashes received from the API.

I have a tiny change that does this, and I will be testing it on Windows to see
if it works.

[ahe, 2012-01-02 15:51:23]

On 2012/01/02 15:45:21, Bill Hesse wrote:
> I looks to me like the best way to handle Windows paths is to use a forward
> slash everywhere, and to convert backslashes to forward slash in the (very
few)
> cases where we get a path with separators returned to us from the Win32 API.
> 
> We can pass the forward slash paths in to the API, and they will be converted
> internally, so we only need to worry about backslashes received from the API.
> 
> I have a tiny change that does this, and I will be testing it on Windows to
see
> if it works.

This is immediately attractive to me. I hope it works. I can't see anything
obviously wrong with it, but I fear it won't be as simple (but I really hope it
will work).

[Bill Hesse, 2012-01-03 15:01:35]

OK, this should be ready now.  I am testing it on Windows.  Can you take a look,
ager?

http://codereview.chromium.org/9036009/diff/8001/client/tests/dartc/dartc.status
File client/tests/dartc/dartc.status (right):

http://codereview.chromium.org/9036009/diff/8001/client/tests/dartc/dartc.sta...
client/tests/dartc/dartc.status:7: tests/dartc/test_config.dart: Skip      # Not
a test case.
OK, these lines removed, check added to test_suite.StandardTestSuite



On 2012/01/02 13:00:59, ahe wrote:
> On 2012/01/02 12:50:42, Bill Hesse wrote:
> > On 2012/01/02 10:42:23, ahe wrote:
> > > Wouldn't it be better to filter out the name "test_config.dart"?
> > 
> > Yes, I suppose so.  But then special names are built into the system of
> > iterating over tests, in code, rather than in data.
> 
> The special names are part of the system, not the data. As far as I'm
concerned,
> the greater sin is to pollute the data with this information.

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
File tools/testing/dart/test_suite.dart (right):

http://codereview.chromium.org/9036009/diff/8001/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:590: return
filename.replaceAll(pathSeparator, '_') + 'dartium.html';
Now all arguments to this have only forward slashes.

On 2012/01/02 10:42:23, ahe wrote:
> This doesn't work on Windows. Consider that a windows file path may contain
> forward slashes as well as backslashes.

[Bill Hesse, 2012-01-03 18:29:06]

http://codereview.chromium.org/9036009/diff/18001/tools/testing/dart/test_sui...
File tools/testing/dart/test_suite.dart (right):

http://codereview.chromium.org/9036009/diff/18001/tools/testing/dart/test_sui...
tools/testing/dart/test_suite.dart:357: dartTestFilename =
dartTestFilename.replaceAll('\\', '/');
Let's remove these again, since the test doesn't even run on Windows, and this
is not the proper solution when we do run it on Windows.

[Mads Ager (google), 2012-01-04 10:19:02]

lgtm