Fixes for the test framework multitests.

Fixes for the test framework multitests.

Allow imports with a metadata annotation.
Allow test keys to be alphanumeric instead of just two digits.

R=floitsch@google.com, whesse@google.com

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

Was reverted in Issue 175113002, commit r32894.

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

[floitsch, 2014-02-17 14:22:32]

DBC.

https://codereview.chromium.org/164333013/diff/1/tools/testing/dart/multitest...
File tools/testing/dart/multitest.dart (right):

https://codereview.chromium.org/164333013/diff/1/tools/testing/dart/multitest...
tools/testing/dart/multitest.dart:182:
'^(?:@\\w+\\s+)?(import|part)\\s+["\'](?!(dart:|dart-ext:|package:|/))([^"\']*)["\']');
long line.

[floitsch, 2014-02-17 14:23:49]

dbc2.

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

https://codereview.chromium.org/164333013/diff/1/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:31: RegExp multiTestRegExp = new RegExp(r"///
\w+:(.*)");
I think this would clash too easily with dartdoc comments.

[sigurdm, 2014-02-17 14:29:59]

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

https://codereview.chromium.org/164333013/diff/1/tools/testing/dart/test_suit...
tools/testing/dart/test_suite.dart:31: RegExp multiTestRegExp = new RegExp(r"///
\w+:(.*)");
On 2014/02/17 14:23:49, floitsch wrote:
> I think this would clash too easily with dartdoc comments.

Good point - it is a shame. Two digit identifiers are not the best user
experience to work with.

[floitsch, 2014-02-17 14:41:31]

On 2014/02/17 14:29:59, sigurdm wrote:
>
https://codereview.chromium.org/164333013/diff/1/tools/testing/dart/test_suit...
> File tools/testing/dart/test_suite.dart (right):
> 
>
https://codereview.chromium.org/164333013/diff/1/tools/testing/dart/test_suit...
> tools/testing/dart/test_suite.dart:31: RegExp multiTestRegExp = new
RegExp(r"///
> \w+:(.*)");
> On 2014/02/17 14:23:49, floitsch wrote:
> > I think this would clash too easily with dartdoc comments.
> 
> Good point - it is a shame. Two digit identifiers are not the best user
> experience to work with.

We could maybe get away with it, if we require something in front of the ///.
/// foo: dartdoc-comment.

x  /// test-expectation: ...


There is also the danger that users will start using characters that are not
valid as file-names (in particular "/"). The regexp should catch this.

[floitsch, 2014-02-17 14:42:46]

On 2014/02/17 14:41:31, floitsch wrote:
> On 2014/02/17 14:29:59, sigurdm wrote:
> >
>
https://codereview.chromium.org/164333013/diff/1/tools/testing/dart/test_suit...
> > File tools/testing/dart/test_suite.dart (right):
> > 
> >
>
https://codereview.chromium.org/164333013/diff/1/tools/testing/dart/test_suit...
> > tools/testing/dart/test_suite.dart:31: RegExp multiTestRegExp = new
> RegExp(r"///
> > \w+:(.*)");
> > On 2014/02/17 14:23:49, floitsch wrote:
> > > I think this would clash too easily with dartdoc comments.
> > 
> > Good point - it is a shame. Two digit identifiers are not the best user
> > experience to work with.
> 
> We could maybe get away with it, if we require something in front of the ///.
> /// foo: dartdoc-comment.
> 
> x  /// test-expectation: ...
> 
> 
> There is also the danger that users will start using characters that are not
> valid as file-names (in particular "/"). The regexp should catch this.

Never mind: "\w" does that already.

[Bill Hesse, 2014-02-17 17:59:07]

LGTM.

https://codereview.chromium.org/164333013/diff/100001/tools/testing/dart/mult...
File tools/testing/dart/multitest.dart (right):

https://codereview.chromium.org/164333013/diff/100001/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:182: '^(?:@\\w+\\s+)?' // Allow for a
meta-data annotation.
Do we only use annotations of this form here, and not
@AConstantConstructor('args','here')?

https://codereview.chromium.org/164333013/diff/100001/tools/testing/dart/test...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/164333013/diff/100001/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:31: RegExp multiTestRegExp = new
RegExp(r"\S\s*/// \w+:(.*)");
I don't think this is needed, but I can't see a case in which it will cause much
confusion or errors, so OK.

[sigurdm, 2014-02-18 08:24:57]

https://codereview.chromium.org/164333013/diff/100001/tools/testing/dart/mult...
File tools/testing/dart/multitest.dart (right):

https://codereview.chromium.org/164333013/diff/100001/tools/testing/dart/mult...
tools/testing/dart/multitest.dart:182: '^(?:@\\w+\\s+)?' // Allow for a
meta-data annotation.
On 2014/02/17 17:59:08, Bill Hesse wrote:
> Do we only use annotations of this form here, and not
> @AConstantConstructor('args','here')?
> 

Indeed we allow more general annotations.

[sigurdm, 2014-02-21 12:41:55]

Committed patchset #3 manually as r32892 (presubmit successful).

[sigurdm, 2014-02-24 09:46:03]

ptal

[floitsch, 2014-02-24 09:56:28]

LGTM with spaces instead of multiline.

https://codereview.chromium.org/164333013/diff/220001/tools/testing/dart/test...
File tools/testing/dart/test_suite.dart (right):

https://codereview.chromium.org/164333013/diff/220001/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:32: RegExp multiTestRegExp = new
RegExp(r"^.*\S.*"
Let's avaoid the multiline, and use " " (space), instead of allowing new-lines.

[sigurdm, 2014-02-24 10:13:48]

Committed patchset #5 manually as r32956 (presubmit successful).