First check in for pub package manager.

First check in for pub package manager.

It doesn't do anything more than show usage and version, but the test
infrastructure is there.

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

[Bob Nystrom, 2012-03-29 23:47:15]

This is more or less a "shell" patch. Pub doesn't do anything interesting yet,
but the basic harness for running integration tests is there.

[nweiz, 2012-03-30 00:38:37]

https://chromiumcodereview.appspot.com/9950005/diff/1/tools/testing/dart/test...
File tools/testing/dart/test_suite.dart (right):

https://chromiumcodereview.appspot.com/9950005/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:988: * A standard test suite whose file
organization matches an expected structure.
Woo!

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/pub/pub.dart
File utils/pub/pub.dart (right):

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/pub/pub.dart#newc...
utils/pub/pub.dart:12: List<PubCommand> commands;
This should probably be a hash.

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/pub/pub.dart#newc...
utils/pub/pub.dart:18: new PubCommand('version', 'print Pub version',
showVersion)
Not something worth changing in this CL, but at some point it would be nice to
support --version, --help, and -h as well. Those are flags that I feel should be
supported by every command-line executable, regardless of its normal argument
conventions, because they're going to be used by users who haven't familiarized
themselves with the conventions yet.

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/pub/pub.dart#newc...
utils/pub/pub.dart:58: print('  ${padRight(command.name, length)} 
${command.description}');
I'm not sure two spaces between the command name and the description is enough.
I found the example output a little tough to read. Git uses three, for
reference.

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/tests/pub/pub_tes...
File utils/tests/pub/pub_tests.dart (right):

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/tests/pub/pub_tes...
utils/tests/pub/pub_tests.dart:45: void testPub(String description, List<String>
arguments, String expectedOutput) {
This name doesn't match that being used in main(), and it doesn't have an
optional exitCode argument.

Also, line length.

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/tests/pub/pub_tes...
utils/tests/pub/pub_tests.dart:47: Expect.fail('args = ${new
Options().script}');
Debugging line?

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/tests/pub/pub_tes...
utils/tests/pub/pub_tests.dart:69: }
How does this async stuff integrate into the testing infrastructure? Don't you
have to notify something when the test is finished?

[Bob Nystrom, 2012-03-30 01:06:17]

Thanks!

https://chromiumcodereview.appspot.com/9950005/diff/1/tools/testing/dart/test...
File tools/testing/dart/test_suite.dart (right):

https://chromiumcodereview.appspot.com/9950005/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:988: * A standard test suite whose file
organization matches an expected structure.
On 2012/03/30 00:38:37, nweiz wrote:
> Woo!

I need to rebase, this is actually in another patch. But, yeah, this is nice. :)

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/pub/pub.dart
File utils/pub/pub.dart (right):

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/pub/pub.dart#newc...
utils/pub/pub.dart:12: List<PubCommand> commands;
On 2012/03/30 00:38:37, nweiz wrote:
> This should probably be a hash.

Done.

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/pub/pub.dart#newc...
utils/pub/pub.dart:18: new PubCommand('version', 'print Pub version',
showVersion)
On 2012/03/30 00:38:37, nweiz wrote:
> Not something worth changing in this CL, but at some point it would be nice to
> support --version, --help, and -h as well. Those are flags that I feel should
be
> supported by every command-line executable, regardless of its normal argument
> conventions, because they're going to be used by users who haven't
familiarized
> themselves with the conventions yet.

Good call. Added a TODO.

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/pub/pub.dart#newc...
utils/pub/pub.dart:58: print('  ${padRight(command.name, length)} 
${command.description}');
On 2012/03/30 00:38:37, nweiz wrote:
> I'm not sure two spaces between the command name and the description is
enough.
> I found the example output a little tough to read. Git uses three, for
> reference.

Done.

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/tests/pub/pub_tes...
File utils/tests/pub/pub_tests.dart (right):

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/tests/pub/pub_tes...
utils/tests/pub/pub_tests.dart:45: void testPub(String description, List<String>
arguments, String expectedOutput) {
On 2012/03/30 00:38:37, nweiz wrote:
> This name doesn't match that being used in main(), and it doesn't have an
> optional exitCode argument.
> 
> Also, line length.

Oops, old code. Deleted.

https://chromiumcodereview.appspot.com/9950005/diff/1/utils/tests/pub/pub_tes...
utils/tests/pub/pub_tests.dart:69: }
On 2012/03/30 00:38:37, nweiz wrote:
> How does this async stuff integrate into the testing infrastructure? Don't you
> have to notify something when the test is finished?

Yeah, this is dead code. The real code is in test_pub.dart.

[nweiz, 2012-03-30 01:25:06]

lgtm

https://chromiumcodereview.appspot.com/9950005/diff/8001/utils/tests/pub/test...
File utils/tests/pub/test_pub.dart (right):

https://chromiumcodereview.appspot.com/9950005/diff/8001/utils/tests/pub/test...
utils/tests/pub/test_pub.dart:71: * leading and trailing whitespaces differences
and tries to report the
s/whitespaces/whitespace/

https://chromiumcodereview.appspot.com/9950005/diff/8001/utils/tests/pub/test...
utils/tests/pub/test_pub.dart:80: Expect.fail('Output line ${i + 1} was:
${actual[i]}\nexpected: ${expected[i]}');
Line length; use """?

[kasperl, 2012-04-11 07:17:03]

LGTM. Nits:

https://chromiumcodereview.appspot.com/9950005/diff/5007/utils/pub/pub.dart
File utils/pub/pub.dart (right):

https://chromiumcodereview.appspot.com/9950005/diff/5007/utils/pub/pub.dart#n...
utils/pub/pub.dart:34: exit(64); // see
http://www.freebsd.org/cgi/man.cgi?query=sysexits.
see -> See

https://chromiumcodereview.appspot.com/9950005/diff/5007/utils/pub/pub.dart#n...
utils/pub/pub.dart:44: print('Pub is a package manager for Dart.');
Wouldn't this be nicer in a multiline string constant somewhere?

https://chromiumcodereview.appspot.com/9950005/diff/5007/utils/tests/pub/test...
File utils/tests/pub/test_pub.dart (right):

https://chromiumcodereview.appspot.com/9950005/diff/5007/utils/tests/pub/test...
utils/tests/pub/test_pub.dart:26: '../../../tools/testing/bin/$platform/dart');
You sometimes indent these with 4 spaces and sometimes 2.

[Bob Nystrom, 2012-04-11 16:50:22]

Thanks!

https://chromiumcodereview.appspot.com/9950005/diff/8001/utils/tests/pub/test...
File utils/tests/pub/test_pub.dart (right):

https://chromiumcodereview.appspot.com/9950005/diff/8001/utils/tests/pub/test...
utils/tests/pub/test_pub.dart:71: * leading and trailing whitespaces differences
and tries to report the
On 2012/03/30 01:25:06, nweiz wrote:
> s/whitespaces/whitespace/

Done.

https://chromiumcodereview.appspot.com/9950005/diff/8001/utils/tests/pub/test...
utils/tests/pub/test_pub.dart:80: Expect.fail('Output line ${i + 1} was:
${actual[i]}\nexpected: ${expected[i]}');
On 2012/03/30 01:25:06, nweiz wrote:
> Line length; use """?

Done. ''' actually wouldn't work here because then you'd get the newline in your
string. Fortunately, in this case, the whole string fits on one line if you move
it down.

https://chromiumcodereview.appspot.com/9950005/diff/5007/utils/pub/pub.dart
File utils/pub/pub.dart (right):

https://chromiumcodereview.appspot.com/9950005/diff/5007/utils/pub/pub.dart#n...
utils/pub/pub.dart:34: exit(64); // see
http://www.freebsd.org/cgi/man.cgi?query=sysexits.
On 2012/04/11 07:17:03, kasperl wrote:
> see -> See

Done (in a forthcoming patch).

https://chromiumcodereview.appspot.com/9950005/diff/5007/utils/pub/pub.dart#n...
utils/pub/pub.dart:44: print('Pub is a package manager for Dart.');
On 2012/04/11 07:17:03, kasperl wrote:
> Wouldn't this be nicer in a multiline string constant somewhere?

I did that at first, but the problem is multiline strings don't handle
indentation well. I don't want to have to break the indentation of the file for
the string, so this seemed like the cleanest solution.

https://chromiumcodereview.appspot.com/9950005/diff/5007/utils/tests/pub/test...
File utils/tests/pub/test_pub.dart (right):

https://chromiumcodereview.appspot.com/9950005/diff/5007/utils/tests/pub/test...
utils/tests/pub/test_pub.dart:26: '../../../tools/testing/bin/$platform/dart');
On 2012/04/11 07:17:03, kasperl wrote:
> You sometimes indent these with 4 spaces and sometimes 2.

Normal indentation should be 2 spaces, and line continuations should be 4 (or
more, to align with stuff). I think that's more or less Google standard style,
though you're right that there are probably mistakes where I'm not following it
right.