Added new style of tests for parser.

pkg/args/test/parse_all_test.dart

Index: pkg/args/test/parse_all_test.dart
diff --git a/pkg/args/test/parse_all_test.dart b/pkg/args/test/parse_all_test.dart
new file mode 100644
index 0000000000000000000000000000000000000000..ee23343362ddda13d170d427edc5de3d8195f831
--- /dev/null
+++ b/pkg/args/test/parse_all_test.dart
@@ -0,0 +1,90 @@
+// Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+library parse_all_test;
+
+import 'package:unittest/unittest.dart';
+import 'package:args/args.dart';
+
+main() {
+  group('ArgParser.parse()', () {
+    var parser;
+    var args;
+
+    setUp(() {
+      parser = new ArgParser();
+      args = <String>[];
+    });
+
+    /// Tests that parsing beginning with an argument turns everything following
+    /// into an argument.
+    group('unknown argument', () {

[Bob Nystrom, 2013/06/19 00:23:18]

"Unknown" is confusing here. "non-option"?

How about:

group("starting with a non-option", () {...})

and then ditch the comment.

[Andrei Mouravski, 2013/06/19 19:50:56]

Done.

+      // args: 'A --flag'

[Bob Nystrom, 2013/06/19 00:23:18]

Remove this and just make the arg list a list literal. :)

[Andrei Mouravski, 2013/06/19 19:50:56]

Done.

+      test('followed by flag', () {
+        args.add('A');
+        parser.addFlag('flag');
+        args.add('--flag');
+
+        var results = parser.parse(args);

[Bob Nystrom, 2013/06/19 00:23:18]

Just inline the args here:

parser.parse(['A', '--flag']);

[Andrei Mouravski, 2013/06/19 19:50:56]

I'd rather keep it as a var so I can check against it.

[Bob Nystrom, 2013/06/19 19:53:04]

OK, well at least create it using a literal. The .add() seems needlessly
verbose.

[Andrei Mouravski, 2013/06/19 20:00:35]

I did. You're looking at the wrong patch. :]

+        expect(results['flag'], isFalse);
+        expect(results.rest, orderedEquals(args));
+      });
+
+      // args: 'A --opt'
+      test('followed by option', () {
+        args.add('A');
+        parser.addOption('opt');
+
+        args.add('--opt');
+
+        var results = parser.parse(args);
+        expect(results['opt'], isNull);
+        expect(results.rest, orderedEquals(args));
+      });
+
+      // args: 'A --opt V'
+      test('followed by option and value', () {
+        args.add('A');
+        parser.addOption('opt');
+        args.add('--opt');
+        args.add('V');
+
+        var results = parser.parse(args);
+        expect(results['opt'], isNull);
+        expect(results.rest, orderedEquals(args));
+      });
+
+      // args: 'A --xflag'
+      test('followed by unknown flag', () {
+        args.add('A');
+        args.add('--xflag');
+        var results = parser.parse(args);
+        expect(results.rest, orderedEquals(args));
+      });
+
+      // args: 'A --xopt V'
+      test('followed by unknown option and value', () {
+        args.add('A');
+        args.add('--xopt');
+        args.add('V');
+        var results = parser.parse(args);
+        expect(results.rest, orderedEquals(args));
+      });
+
+      // args: 'A com'
+      test('followed by command', () {
+        args.add('A');
+        parser.addCommand('com');
+        args.add('com');
+
+        var results = parser.parse(args);
+        expect(results.command, isNull);
+        expect(results.rest, orderedEquals(args));
+      });

[Bob Nystrom, 2013/06/19 00:23:18]

I think you could accomplish as much by adding a test or two to the parser suite
like:

      test('ignores options following a non-option-like arg', () {
        var parser = new ArgParser();
        parser.addFlag('woof');
        parser.addOption('meow');

        var results = parser.parse(['arg', '--woof', '--meow']);
        expect(results['woof'], isNull);
        expect(results['meow'], isNull);
        expect(results.rest, orderedEquals(['arg', '--woof', '--meow']));
      });

[Andrei Mouravski, 2013/06/19 19:50:56]

I still think it's valuable to be spartan because it'll make the contrast with
parseAll make more sense.

[Bob Nystrom, 2013/06/19 19:53:04]

I don't understand this. Why do you want to contrast with the other tests.
Wouldn't it be better to be consistent?

[Andrei Mouravski, 2013/06/19 20:00:35]

I'm not contrasting with the other tests. I'm saying that once my next change
goes in, there will be contrast within these tests for how parseAll=true is
different from parseAll=false.

+    });
+  });
+}
+
+expectThrows(ArgParser parser, List<String> args) =>
+  expect(() => parser.parse(args), throws);

[Bob Nystrom, 2013/06/19 00:23:18]

Unused? Remove.

[Andrei Mouravski, 2013/06/19 19:50:56]

Done.