Working SimpleHTML test front end

Working SimpleHTML test front end

BUG=
R=ricow@google.com

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

[Bill Hesse, 2014-11-03 09:21:08]

whesse@google.com changed reviewers:
+ ricow@google.com

[Bill Hesse, 2014-11-03 09:21:09]

https://codereview.chromium.org/695893002/diff/1/tools/testing/dart/test_suit...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/695893002/diff/1/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:690: bool isHtmlTestFile(String filename) =>
filename.endsWith('_htmltest.html');
I would like to use _test.html, but that is used by the current system of custom
html files for existing tests.  We could rename all the existing ones, and make
the new ones _test.html.

[ricow1, 2014-11-04 09:18:03]

https://codereview.chromium.org/695893002/diff/20001/tests/html/no_linked_scr...
File tests/html/no_linked_scripts_htmltest.html (right):

https://codereview.chromium.org/695893002/diff/20001/tests/html/no_linked_scr...
tests/html/no_linked_scripts_htmltest.html:1: START_HTML_DART_TEST
copyright header

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/html_...
File tools/testing/dart/html_test.dart (right):

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/html_...
tools/testing/dart/html_test.dart:8: * HTML tests are valid HTML files whose
name ends in _test.html, and that
this is not really what you say in the other files, there you state that it
needs to end in _htmltest.html

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/html_...
tools/testing/dart/html_test.dart:24: String contents = new
File(filename).readAsStringSync();
assert that the file ending is correct

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/html_...
tools/testing/dart/html_test.dart:26: print(match);
remove debug print

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/html_...
tools/testing/dart/html_test.dart:28: print(match[1]);
remove debug print

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/test_...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/test_...
tools/testing/dart/test_suite.dart:596: 
two newlines

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/test_...
tools/testing/dart/test_suite.dart:687: throw new Exception('This suite uses
obsolete "Test.dart" filenames');
do this in a separate cl please

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/test_...
tools/testing/dart/test_suite.dart:788: print("Invalid HtmlTest $filename");
so I don't think this will catch peoples attention, shouldn't the test fail if
this is the case? (we want the actual test to fail, not the testing script to
exit)

[Bill Hesse, 2014-11-04 16:29:01]

PTAL.

https://codereview.chromium.org/695893002/diff/20001/tests/html/no_linked_scr...
File tests/html/no_linked_scripts_htmltest.html (right):

https://codereview.chromium.org/695893002/diff/20001/tests/html/no_linked_scr...
tests/html/no_linked_scripts_htmltest.html:1: START_HTML_DART_TEST
On 2014/11/04 09:18:03, ricow1 wrote:
> copyright header

Done.

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/html_...
File tools/testing/dart/html_test.dart (right):

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/html_...
tools/testing/dart/html_test.dart:8: * HTML tests are valid HTML files whose
name ends in _test.html, and that
On 2014/11/04 09:18:03, ricow1 wrote:
> this is not really what you say in the other files, there you state that it
> needs to end in _htmltest.html

Done.

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/html_...
tools/testing/dart/html_test.dart:24: String contents = new
File(filename).readAsStringSync();
On 2014/11/04 09:18:03, ricow1 wrote:
> assert that the file ending is correct

Done.  But this is not really a concern of the HTML test module - what name is
used
to identify the files does not affect their processing.

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/html_...
tools/testing/dart/html_test.dart:26: print(match);
On 2014/11/04 09:18:03, ricow1 wrote:
> remove debug print

Done.

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/html_...
tools/testing/dart/html_test.dart:28: print(match[1]);
On 2014/11/04 09:18:03, ricow1 wrote:
> remove debug print

Done.

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/test_...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/test_...
tools/testing/dart/test_suite.dart:596: 
On 2014/11/04 09:18:03, ricow1 wrote:
> two newlines

Put two newlines between all classes in the file.

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/test_...
tools/testing/dart/test_suite.dart:687: throw new Exception('This suite uses
obsolete "Test.dart" filenames');
On 2014/11/04 09:18:03, ricow1 wrote:
> do this in a separate cl please

Done.

https://codereview.chromium.org/695893002/diff/20001/tools/testing/dart/test_...
tools/testing/dart/test_suite.dart:788: print("Invalid HtmlTest $filename");
On 2014/11/04 09:18:03, ricow1 wrote:
> so I don't think this will catch peoples attention, shouldn't the test fail if
> this is the case? (we want the actual test to fail, not the testing script to
> exit)

This is rather hard to do.  At this point, we would need to create a dummy
TestInformation object, perhaps of a new subclass, that would indicate to the
BrowserTestCreator (and maybe non-browser test creators), that it should create
a TestCase object that would always fail, with a certain message.  This would
then require Command objects that would do nothing, perhaps.

Maybe a better solution is to create an HtmlTestInfo with no scripts, and a
dummy message that would not be matched, so that the test would definitely time
out when it was run, and print this dummy message.

For now, logging as an error.

[ricow1, 2014-11-05 12:22:55]

lgtm

https://codereview.chromium.org/695893002/diff/40001/tests/html/no_linked_scr...
File tests/html/no_linked_scripts_htmltest.html (right):

https://codereview.chromium.org/695893002/diff/40001/tests/html/no_linked_scr...
tests/html/no_linked_scripts_htmltest.html:29: }
strange indentation

https://codereview.chromium.org/695893002/diff/40001/tools/testing/dart/multi...
File tools/testing/dart/multitest.dart (right):

https://codereview.chromium.org/695893002/diff/40001/tools/testing/dart/multi...
tools/testing/dart/multitest.dart:268: bool hasCompileErrorIfChecked =
outcome.contains('checked mode compile-time error');
not yours, but long line

[Bill Hesse, 2014-11-05 12:54:45]

Committed patchset #3 (id:40001) manually as 41517 (presubmit successful).

[Bill Hesse, 2014-11-05 16:00:32]

https://codereview.chromium.org/695893002/diff/40001/tests/html/no_linked_scr...
File tests/html/no_linked_scripts_htmltest.html (right):

https://codereview.chromium.org/695893002/diff/40001/tests/html/no_linked_scr...
tests/html/no_linked_scripts_htmltest.html:29: }
On 2014/11/05 12:22:55, ricow1 wrote:
> strange indentation
Fixed in followup CL already.