Refactor the pub tests.

Refactor the pub tests.

The goal of this refactoring is to make it easier to flexibly set up the
filesystem before Pub runs and validate it afterwards. To do so, the pub tests
are made more modular by moving creation and validation out of the monolithic
testPub method. The declarative philosophy of testPub is maintained.

This CL also includes several changes to the unit test library. Most notably, it
causes stack traces to be displayed for errors that are thrown during tests.

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

[nweiz, 2012-05-09 23:37:40]

Including Bob mostly for the Pub test changes and Siggi mostly for unittest
changes.

[Bob Nystrom, 2012-05-10 20:15:10]

LGTM with a better name for "guard". I would wait for feedback from Siggy too on
unittest.

I worry a bit that the scheduleCreate() and scheduleValidate() methods will be
error-prone. Especially with validate, it is easy to have a test do dir(...),
forget the trailing validate call, and then silently fail to actually test
anything.

But I don't have any great solutions in mind.

https://chromiumcodereview.appspot.com/10382094/diff/1/lib/unittest/unittest....
File lib/unittest/unittest.dart (right):

https://chromiumcodereview.appspot.com/10382094/diff/1/lib/unittest/unittest....
lib/unittest/unittest.dart:471: guard(tryBody, [finallyBody]) {
If we're going to make this public, we should give it a more distinct name. We
expect/hope lots of people will use unittest so we try to be careful about what
names it steps on.

Maybe testGuard()?

https://chromiumcodereview.appspot.com/10382094/diff/1/utils/tests/pub/test_p...
File utils/tests/pub/test_pub.dart (right):

https://chromiumcodereview.appspot.com/10382094/diff/1/utils/tests/pub/test_p...
utils/tests/pub/test_pub.dart:79: _scheduledAfterPub = [];
I would set these to null instead of an empty list here since schedule() checks
for that case and that's what they'll be on the first test.

https://chromiumcodereview.appspot.com/10382094/diff/1/utils/tests/pub/test_p...
utils/tests/pub/test_pub.dart:89: Future runScheduled(List<_ScheduledEvent>
scheduled) {
I would break this out into a (private) top-level function and pass in the
sandboxDir. The containing function is pretty big as it is.

[nweiz, 2012-05-10 21:01:07]

I could have Descriptors assert that they're either created or validated after
the test is done, so that isolated dir() calls don't get dropped silently.

https://chromiumcodereview.appspot.com/10382094/diff/1/lib/unittest/unittest....
File lib/unittest/unittest.dart (right):

https://chromiumcodereview.appspot.com/10382094/diff/1/lib/unittest/unittest....
lib/unittest/unittest.dart:471: guard(tryBody, [finallyBody]) {
On 2012/05/10 20:15:10, Bob Nystrom wrote:
> If we're going to make this public, we should give it a more distinct name. We
> expect/hope lots of people will use unittest so we try to be careful about
what
> names it steps on.
> 
> Maybe testGuard()?

I don't have any great ideas for this name. Maybe Siggi will.

https://chromiumcodereview.appspot.com/10382094/diff/1/utils/tests/pub/test_p...
File utils/tests/pub/test_pub.dart (right):

https://chromiumcodereview.appspot.com/10382094/diff/1/utils/tests/pub/test_p...
utils/tests/pub/test_pub.dart:79: _scheduledAfterPub = [];
On 2012/05/10 20:15:10, Bob Nystrom wrote:
> I would set these to null instead of an empty list here since schedule()
checks
> for that case and that's what they'll be on the first test.

Done.

https://chromiumcodereview.appspot.com/10382094/diff/1/utils/tests/pub/test_p...
utils/tests/pub/test_pub.dart:89: Future runScheduled(List<_ScheduledEvent>
scheduled) {
On 2012/05/10 20:15:10, Bob Nystrom wrote:
> I would break this out into a (private) top-level function and pass in the
> sandboxDir. The containing function is pretty big as it is.

Done.

[Siggi Cherem (dart-lang), 2012-05-11 01:03:21]

LGTM,

as for guard - maybe guardAsync (it's the name we started using in the promise
of a better API in the documentation on the top.)

[Bob Nystrom, 2012-05-11 17:41:11]

On 2012/05/11 01:03:21, sigmund wrote:
> LGTM,
> 
> as for guard - maybe guardAsync (it's the name we started using in the promise
> of a better API in the documentation on the top.)

+1 for guardAsync.