Run large html tests individually.

Run large html tests individually.
Committed: https://code.google.com/p/dart/source/detail?r=14663

[Emily Fortuna, 2012-10-31 22:12:03]

There are still a number of adjustments I can do to make this a lot nicer --
like using the test description string instead of test number,  only running the
tests individually if any of the individual tests fail, and add which specific
tests fail on IE and Safari 5 (can't test on my laptop)... but, I'm sending this
out now because I wanted to make sure this what you'd like. I went through the
html tests, and really only a very small number of them (node_test and
element_test) are that huge. Do we want to make this change to the whole test
framework just for a few tests?

[blois, 2012-10-31 22:18:29]

On 2012/10/31 22:12:03, Emily Fortuna wrote:
> There are still a number of adjustments I can do to make this a lot nicer --
> like using the test description string instead of test number,  only running
the
> tests individually if any of the individual tests fail, and add which specific
> tests fail on IE and Safari 5 (can't test on my laptop)... but, I'm sending
this
> out now because I wanted to make sure this what you'd like. I went through the
> html tests, and really only a very small number of them (node_test and
> element_test) are that huge. Do we want to make this change to the whole test
> framework just for a few tests?

Looks good- how much would it take to move from index to name? Index seems
fairly fragile.

I'd just do it for specific tests at first- basically the ones that are marked
as fail which we expect to be mainline scenarios (Element, Node, SVGElement,
etc).

[Emily Fortuna, 2012-11-01 00:33:34]

On 2012/10/31 22:18:29, blois wrote:
> On 2012/10/31 22:12:03, Emily Fortuna wrote:
> > There are still a number of adjustments I can do to make this a lot nicer --
> > like using the test description string instead of test number,  only running
> the
> > tests individually if any of the individual tests fail, and add which
specific
> > tests fail on IE and Safari 5 (can't test on my laptop)... but, I'm sending
> this
> > out now because I wanted to make sure this what you'd like. I went through
the
> > html tests, and really only a very small number of them (node_test and
> > element_test) are that huge. Do we want to make this change to the whole
test
> > framework just for a few tests?
> 
> Looks good- how much would it take to move from index to name? Index seems
> fairly fragile.
> 
> I'd just do it for specific tests at first- basically the ones that are marked
> as fail which we expect to be mainline scenarios (Element, Node, SVGElement,
> etc).

Sorry, the most crucial part I left out -- running the tests individually is a
LOT slower than all together. Apparently webdriver is just slow...

[Emily Fortuna, 2012-11-01 05:19:10]

Hi Bill, Graham (and Pete),
The motivation behind this CL was that we have some very large html tests
(element_test has some 100 subtests), and rather than breaking it up into 50
small files, we figured it would be cleaner to introduce the concept of
"multitests" to the browser as well. Browser multitests are slightly different
in that we compile once and run in the browser several times. The specific test
to be run is specified by appending the test description after a # in the url.

Bill and Graham, I've mainly added you to look over the particular files you're
familiar with (tools/testing/dart for Bill, and pkg/unittest for Graham). Pete
will be doing the overall review.

https://codereview.chromium.org/11293019/diff/4/tools/testing/dart/status_fil...
File tools/testing/dart/status_file_parser.dart (left):

https://codereview.chromium.org/11293019/diff/4/tools/testing/dart/status_fil...
tools/testing/dart/status_file_parser.dart:28: const RegExp RulePattern = const
RegExp(r"\s*([^: ]*)\s*:(.*)");
I changed this regexp so that test descriptions could be used to name the
"subtests", for example, 
element_test/.html has no parent: Fail
in the status file. (".html has no parent" is the description given in a test
case in element_test.dart)

whesse, is there a specific reason why this was previously disallowed?

[Bill Hesse, 2012-11-01 20:26:00]

https://codereview.chromium.org/11293019/diff/4/tools/testing/dart/status_fil...
File tools/testing/dart/status_file_parser.dart (left):

https://codereview.chromium.org/11293019/diff/4/tools/testing/dart/status_fil...
tools/testing/dart/status_file_parser.dart:28: const RegExp RulePattern = const
RegExp(r"\s*([^: ]*)\s*:(.*)");
This might lead to exponential backtracking, because (x|space)* (space)* has
many matches.  Also, all the space between the rule header and the : will be
grabbed, then.
A efficient regexp grabbing "I am the walrus" from "     I am the walrus       :
foo foo foo" may be hard to do in one step.

On 2012/11/01 05:19:11, Emily Fortuna wrote:
> I changed this regexp so that test descriptions could be used to name the
> "subtests", for example, 
> element_test/.html has no parent: Fail
> in the status file. (".html has no parent" is the description given in a test
> case in element_test.dart)
> 
> whesse, is there a specific reason why this was previously disallowed?

https://codereview.chromium.org/11293019/diff/4/tools/testing/dart/test_suite...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11293019/diff/4/tools/testing/dart/test_suite...
tools/testing/dart/test_suite.dart:765: commandSet.add(new Command('python',
args));
All of the test cases created from a multitest are going to include the
compilation commands, with the same directory specified.  They will be running
in parallel, writing over each other's output.  And you don't really want to
compile this browser test many times, do you?  You just want to run it on the
browser, with different URLs, many times.

I think you need some sort of preliminary phase, or some locking, to release the
many tests after the compilation is done.

https://codereview.chromium.org/11293019/diff/4/tools/testing/dart/test_suite...
tools/testing/dart/test_suite.dart:1094: // always directly call "test(".
It would be nice if there was something more explicit: a new method
parallel_test, or a comment // Multitest, or something.

[Emily Fortuna, 2012-11-02 20:54:54]

PTAL -- Bill, good catch on the recompilation. I fixed that. I also put the
regex in the status file back to the way it was. As result now, though, you
*cannot* use spaces in your group descriptions.

[Bill Hesse, 2012-11-05 11:51:49]

LGTM.

https://codereview.chromium.org/11293019/diff/8013/tools/testing/dart/test_su...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11293019/diff/8013/tools/testing/dart/test_su...
tools/testing/dart/test_suite.dart:761: if (subtestIndex != 0) {
Nit: It would be better if the sense of this test was reversed, and the
initialization was to [].

[gram, 2012-11-05 17:03:46]

The config looks fine.

https://codereview.chromium.org/11293019/diff/8013/pkg/unittest/html_individu...
File pkg/unittest/html_individual_config.dart (right):

https://codereview.chromium.org/11293019/diff/8013/pkg/unittest/html_individu...
pkg/unittest/html_individual_config.dart:16: #import('unittest.dart', prefix:
'unittest_file');
Not for this CL, but this suggest we should probably be doing a re-export of
unittest in htmlconfig, so you only need to import the config file.

[Emily Fortuna, 2012-11-05 18:27:49]

https://codereview.chromium.org/11293019/diff/8013/pkg/unittest/html_individu...
File pkg/unittest/html_individual_config.dart (right):

https://codereview.chromium.org/11293019/diff/8013/pkg/unittest/html_individu...
pkg/unittest/html_individual_config.dart:16: #import('unittest.dart', prefix:
'unittest_file');
On 2012/11/05 17:03:46, gram wrote:
> Not for this CL, but this suggest we should probably be doing a re-export of
> unittest in htmlconfig, so you only need to import the config file.

sgtm.

https://codereview.chromium.org/11293019/diff/8013/tools/testing/dart/test_su...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/11293019/diff/8013/tools/testing/dart/test_su...
tools/testing/dart/test_suite.dart:761: if (subtestIndex != 0) {
On 2012/11/05 11:51:49, Bill Hesse wrote:
> Nit: It would be better if the sense of this test was reversed, and the
> initialization was to [].

Fixed!