Support an @OnPlatform annotation.

Support an @OnPlatform annotation.

Closes #75

R=kevmoo@google.com

Committed: https://github.com/dart-lang/test/commit/3d733df80f5308e3643317c1c0dedc60bc25c173

[kevmoo, 2015-04-21 03:11:40]

On 2015/04/21 03:02:52, nweiz wrote:

Well first:

~/source/github/test.dart/ (cl1092153003)> dartanalyzer
lib/src/runner/parse_metadata.dart
Analyzing [lib/src/runner/parse_metadata.dart]...
[warning] The getter 'constructorName' is not defined for the class 'Expression'
(/Users/kevmoo/source/github/test.dart/lib/src/runner/parse_metadata.dart, line
316, col 20)
[warning] The getter 'constructorName' is not defined for the class 'Expression'
(/Users/kevmoo/source/github/test.dart/lib/src/runner/parse_metadata.dart, line
317, col 20)
2 warnings found.

[nweiz, 2015-04-21 03:12:31]

On 2015/04/21 03:11:40, kevmoo wrote:
> On 2015/04/21 03:02:52, nweiz wrote:
> 
> Well first:
> 
> ~/source/github/test.dart/ (cl1092153003)> dartanalyzer
> lib/src/runner/parse_metadata.dart
> Analyzing [lib/src/runner/parse_metadata.dart]...
> [warning] The getter 'constructorName' is not defined for the class
'Expression'
> (/Users/kevmoo/source/github/test.dart/lib/src/runner/parse_metadata.dart,
line
> 316, col 20)
> [warning] The getter 'constructorName' is not defined for the class
'Expression'
> (/Users/kevmoo/source/github/test.dart/lib/src/runner/parse_metadata.dart,
line
> 317, col 20)
> 2 warnings found.

I'll fix that later; please continue the review.

[kevmoo, 2015-04-21 03:16:38]

pub run test test/runner/browser/runner_test.dart -N 'with an @OnPlat'
00:13 +1 -1: with an @OnPlatform annotation ignores non-matching Skips
  Expected: contains '+1: All tests passed!'
    Actual: 'Compiling test.dart...\n'
    'test.dart:1:2:\n'
    'Error: Cannot resolve \'OnPlatform\'.\n'
    '@OnPlatform(const {"vm": const Skip()})\n'
    ' ^^^^^^^^^^\n'
    'test.dart:1:2:\n'
    'Error: Not a compile-time constant.\n'
    '@OnPlatform(const {"vm": const Skip()})\n'
    ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n'
    'test.dart:1:2:\n'
    'Error: Not a compile-time constant.\n'
    '@OnPlatform(const {"vm": const Skip()})\n'
    ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n'
    'Error: Compilation failed.\n'
    '\r00:00 +0: load error                                                     
                          \r00:00 +0 -1: load error                             
                                               \n'
    '  Failed to load "test.dart": dart2js failed.\n'
    '\r00:00 +0 -1: Some tests failed.                                          
                          \n'
    ''

  package:test                                expect
  test/runner/browser/runner_test.dart 643:7  main.<fn>.<fn>

00:25 +1 -2: with an @OnPlatform annotation respects matching Timeouts
  Expected: contains 'Test timed out after 0 seconds.'
    Actual: 'Compiling test.dart...\n'
    'test.dart:1:2:\n'
    'Error: Cannot resolve \'OnPlatform\'.\n'
    '@OnPlatform(const {\n'
    ' ^^^^^^^^^^\n'
    'test.dart:1:2:\n'
    'Error: Not a compile-time constant.\n'
    '@OnPlatform(const {\n'
    ' ^^^^^^^^^^^^^^^^^^^\n'
    'test.dart:1:2:\n'
    'Error: Not a compile-time constant.\n'
    '@OnPlatform(const {\n'
    ' ^^^^^^^^^^^^^^^^^^^\n'
    'Error: Compilation failed.\n'
    '\r00:00 +0: load error                                                     
                          \r00:00 +0 -1: load error                             
                                               \n'
    '  Failed to load "test.dart": dart2js failed.\n'
    '\r00:00 +0 -1: Some tests failed.                                          
                          \n'
    ''

  package:test                                expect
  test/runner/browser/runner_test.dart 662:7  main.<fn>.<fn>

00:38 +1 -3: with an @OnPlatform annotation ignores non-matching Timeouts
  Expected: contains '+1: All tests passed!'
    Actual: 'Compiling test.dart...\n'
    'test.dart:1:2:\n'
    'Error: Cannot resolve \'OnPlatform\'.\n'
    '@OnPlatform(const {\n'
    ' ^^^^^^^^^^\n'
    'test.dart:1:2:\n'
    'Error: Not a compile-time constant.\n'
    '@OnPlatform(const {\n'
    ' ^^^^^^^^^^^^^^^^^^^\n'
    'test.dart:1:2:\n'
    'Error: Not a compile-time constant.\n'
    '@OnPlatform(const {\n'
    ' ^^^^^^^^^^^^^^^^^^^\n'
    'Error: Compilation failed.\n'
    '\r00:00 +0: load error                                                     
                          \r00:00 +0 -1: load error                             
                                               \n'
    '  Failed to load "test.dart": dart2js failed.\n'
    '\r00:00 +0 -1: Some tests failed.                                          
                          \n'
    ''

  package:test                                expect
  test/runner/browser/runner_test.dart 682:7  main.<fn>.<fn>

00:38 +1 -3: Some tests failed.

[nweiz, 2015-04-21 03:25:18]

On 2015/04/21 03:16:38, kevmoo wrote:
> pub run test test/runner/browser/runner_test.dart -N 'with an @OnPlat'
> 00:13 +1 -1: with an @OnPlatform annotation ignores non-matching Skips
>   Expected: contains '+1: All tests passed!'
>     Actual: 'Compiling test.dart...\n'
>     'test.dart:1:2:\n'
>     'Error: Cannot resolve \'OnPlatform\'.\n'
>     '@OnPlatform(const {"vm": const Skip()})\n'
>     ' ^^^^^^^^^^\n'
>     'test.dart:1:2:\n'
>     'Error: Not a compile-time constant.\n'
>     '@OnPlatform(const {"vm": const Skip()})\n'
>     ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n'
>     'test.dart:1:2:\n'
>     'Error: Not a compile-time constant.\n'
>     '@OnPlatform(const {"vm": const Skip()})\n'
>     ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n'
>     'Error: Compilation failed.\n'
>     '\r00:00 +0: load error                                                   
 
>                           \r00:00 +0 -1: load error                           
 
>                                                \n'
>     '  Failed to load "test.dart": dart2js failed.\n'
>     '\r00:00 +0 -1: Some tests failed.                                        
 
>                           \n'
>     ''
> 
>   package:test                                expect
>   test/runner/browser/runner_test.dart 643:7  main.<fn>.<fn>
> 
> 00:25 +1 -2: with an @OnPlatform annotation respects matching Timeouts
>   Expected: contains 'Test timed out after 0 seconds.'
>     Actual: 'Compiling test.dart...\n'
>     'test.dart:1:2:\n'
>     'Error: Cannot resolve \'OnPlatform\'.\n'
>     '@OnPlatform(const {\n'
>     ' ^^^^^^^^^^\n'
>     'test.dart:1:2:\n'
>     'Error: Not a compile-time constant.\n'
>     '@OnPlatform(const {\n'
>     ' ^^^^^^^^^^^^^^^^^^^\n'
>     'test.dart:1:2:\n'
>     'Error: Not a compile-time constant.\n'
>     '@OnPlatform(const {\n'
>     ' ^^^^^^^^^^^^^^^^^^^\n'
>     'Error: Compilation failed.\n'
>     '\r00:00 +0: load error                                                   
 
>                           \r00:00 +0 -1: load error                           
 
>                                                \n'
>     '  Failed to load "test.dart": dart2js failed.\n'
>     '\r00:00 +0 -1: Some tests failed.                                        
 
>                           \n'
>     ''
> 
>   package:test                                expect
>   test/runner/browser/runner_test.dart 662:7  main.<fn>.<fn>
> 
> 00:38 +1 -3: with an @OnPlatform annotation ignores non-matching Timeouts
>   Expected: contains '+1: All tests passed!'
>     Actual: 'Compiling test.dart...\n'
>     'test.dart:1:2:\n'
>     'Error: Cannot resolve \'OnPlatform\'.\n'
>     '@OnPlatform(const {\n'
>     ' ^^^^^^^^^^\n'
>     'test.dart:1:2:\n'
>     'Error: Not a compile-time constant.\n'
>     '@OnPlatform(const {\n'
>     ' ^^^^^^^^^^^^^^^^^^^\n'
>     'test.dart:1:2:\n'
>     'Error: Not a compile-time constant.\n'
>     '@OnPlatform(const {\n'
>     ' ^^^^^^^^^^^^^^^^^^^\n'
>     'Error: Compilation failed.\n'
>     '\r00:00 +0: load error                                                   
 
>                           \r00:00 +0 -1: load error                           
 
>                                                \n'
>     '  Failed to load "test.dart": dart2js failed.\n'
>     '\r00:00 +0 -1: Some tests failed.                                        
 
>                           \n'
>     ''
> 
>   package:test                                expect
>   test/runner/browser/runner_test.dart 682:7  main.<fn>.<fn>
> 
> 00:38 +1 -3: Some tests failed.

What version of Dart is that?

[kevmoo, 2015-04-21 03:27:58]

On 2015/04/21 03:25:18, nweiz wrote:
> On 2015/04/21 03:16:38, kevmoo wrote:
> > pub run test test/runner/browser/runner_test.dart -N 'with an @OnPlat'
> > 00:13 +1 -1: with an @OnPlatform annotation ignores non-matching Skips
> >   Expected: contains '+1: All tests passed!'
> >     Actual: 'Compiling test.dart...\n'
> >     'test.dart:1:2:\n'
> >     'Error: Cannot resolve \'OnPlatform\'.\n'
> >     '@OnPlatform(const {"vm": const Skip()})\n'
> >     ' ^^^^^^^^^^\n'
> >     'test.dart:1:2:\n'
> >     'Error: Not a compile-time constant.\n'
> >     '@OnPlatform(const {"vm": const Skip()})\n'
> >     ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n'
> >     'test.dart:1:2:\n'
> >     'Error: Not a compile-time constant.\n'
> >     '@OnPlatform(const {"vm": const Skip()})\n'
> >     ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n'
> >     'Error: Compilation failed.\n'
> >     '\r00:00 +0: load error                                                 
 
>  
> >                           \r00:00 +0 -1: load error                         
 
>  
> >                                                \n'
> >     '  Failed to load "test.dart": dart2js failed.\n'
> >     '\r00:00 +0 -1: Some tests failed.                                      
 
>  
> >                           \n'
> >     ''
> > 
> >   package:test                                expect
> >   test/runner/browser/runner_test.dart 643:7  main.<fn>.<fn>
> > 
> > 00:25 +1 -2: with an @OnPlatform annotation respects matching Timeouts
> >   Expected: contains 'Test timed out after 0 seconds.'
> >     Actual: 'Compiling test.dart...\n'
> >     'test.dart:1:2:\n'
> >     'Error: Cannot resolve \'OnPlatform\'.\n'
> >     '@OnPlatform(const {\n'
> >     ' ^^^^^^^^^^\n'
> >     'test.dart:1:2:\n'
> >     'Error: Not a compile-time constant.\n'
> >     '@OnPlatform(const {\n'
> >     ' ^^^^^^^^^^^^^^^^^^^\n'
> >     'test.dart:1:2:\n'
> >     'Error: Not a compile-time constant.\n'
> >     '@OnPlatform(const {\n'
> >     ' ^^^^^^^^^^^^^^^^^^^\n'
> >     'Error: Compilation failed.\n'
> >     '\r00:00 +0: load error                                                 
 
>  
> >                           \r00:00 +0 -1: load error                         
 
>  
> >                                                \n'
> >     '  Failed to load "test.dart": dart2js failed.\n'
> >     '\r00:00 +0 -1: Some tests failed.                                      
 
>  
> >                           \n'
> >     ''
> > 
> >   package:test                                expect
> >   test/runner/browser/runner_test.dart 662:7  main.<fn>.<fn>
> > 
> > 00:38 +1 -3: with an @OnPlatform annotation ignores non-matching Timeouts
> >   Expected: contains '+1: All tests passed!'
> >     Actual: 'Compiling test.dart...\n'
> >     'test.dart:1:2:\n'
> >     'Error: Cannot resolve \'OnPlatform\'.\n'
> >     '@OnPlatform(const {\n'
> >     ' ^^^^^^^^^^\n'
> >     'test.dart:1:2:\n'
> >     'Error: Not a compile-time constant.\n'
> >     '@OnPlatform(const {\n'
> >     ' ^^^^^^^^^^^^^^^^^^^\n'
> >     'test.dart:1:2:\n'
> >     'Error: Not a compile-time constant.\n'
> >     '@OnPlatform(const {\n'
> >     ' ^^^^^^^^^^^^^^^^^^^\n'
> >     'Error: Compilation failed.\n'
> >     '\r00:00 +0: load error                                                 
 
>  
> >                           \r00:00 +0 -1: load error                         
 
>  
> >                                                \n'
> >     '  Failed to load "test.dart": dart2js failed.\n'
> >     '\r00:00 +0 -1: Some tests failed.                                      
 
>  
> >                           \n'
> >     ''
> > 
> >   package:test                                expect
> >   test/runner/browser/runner_test.dart 682:7  main.<fn>.<fn>
> > 
> > 00:38 +1 -3: Some tests failed.
> 
> What version of Dart is that?

1.10.0-dev.1.5

Give me a sec to build bleeding edge...

[kevmoo, 2015-04-21 03:48:38]

LGTM with tiny comments

https://codereview.chromium.org/1092153003/diff/1/lib/src/utils.dart
File lib/src/utils.dart (right):

https://codereview.chromium.org/1092153003/diff/1/lib/src/utils.dart#newcode56
lib/src/utils.dart:56: E first;
both could be final?

https://codereview.chromium.org/1092153003/diff/1/lib/src/utils.dart#newcode63
lib/src/utils.dart:63: bool operator==(other) {
I'm a big fan of other is Pair && other.first == first && other.last == last.

...but whatevs...

[nweiz, 2015-04-21 19:51:00]

Code review changes

[nweiz, 2015-04-21 20:01:04]

https://codereview.chromium.org/1092153003/diff/1/lib/src/utils.dart
File lib/src/utils.dart (right):

https://codereview.chromium.org/1092153003/diff/1/lib/src/utils.dart#newcode56
lib/src/utils.dart:56: E first;
On 2015/04/21 03:48:38, kevmoo wrote:
> both could be final?

I'd rather keep this identical to the implementation in pub and other places.
Also, in general data structures in Dart are mutable.

[nweiz, 2015-04-21 20:01:17]

Committed patchset #4 (id:60001) manually as
3d733df80f5308e3643317c1c0dedc60bc25c173 (presubmit successful).