Add flag to disable capturing stack traces.

Add flag to disable capturing stack traces.

BUG=http://dartbug.com/9697

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

[Bob Nystrom, 2013-04-09 20:18:54]

It defaults to true in the package. Pub leaves it true on the bots but disables
it by default on local runs so the tests run fast.

On my machine, this takes the pub tests from 1:30 to ~0:24

[nweiz, 2013-04-09 21:25:58]

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/lib/schedu...
File pkg/scheduled_test/lib/scheduled_test.dart (right):

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/lib/schedu...
pkg/scheduled_test/lib/scheduled_test.dart:210: bool captureStackTraces = true;
I'd rather see this on the Schedule class. That's where other scheduled test
configuration has gone.

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/lib/src/sc...
File pkg/scheduled_test/lib/src/schedule_error.dart (right):

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/lib/src/sc...
pkg/scheduled_test/lib/src/schedule_error.dart:58: stackTrace =
thrownStackTrace;
While you're at it, change this to use [Trace.capture].

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/lib/src/ta...
File pkg/scheduled_test/lib/src/task.dart (right):

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/lib/src/ta...
pkg/scheduled_test/lib/src/task.dart:125: var result = "$this";
I think I prefer `this.toString()` here.

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/test/sched...
File pkg/scheduled_test/test/scheduled_test/capture_stack_traces_test.dart
(right):

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/test/sched...
pkg/scheduled_test/test/scheduled_test/capture_stack_traces_test.dart:20: error
= currentSchedule.errors.first.toString();
To be consistent with other tests, don't convert this to a string until test 2.
Same for the test below.

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/test/sched...
pkg/scheduled_test/test/scheduled_test/capture_stack_traces_test.dart:30:
expect(numStackTraces, equals(2));
expect(error.toString(), stringContainsInOrder('Stack trace:', 'Stack trace:'))

Also, for future reference, a RegExp isn't necessary here; you could just use
[String.allMatches].

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/test/sched...
pkg/scheduled_test/test/scheduled_test/capture_stack_traces_test.dart:49:
expect(numStackTraces, equals(1));
expect(error.toString(), isNot(contains('Stack trace:')));

https://codereview.chromium.org/13839023/diff/1/utils/tests/pub/test_pub.dart
File utils/tests/pub/test_pub.dart (right):

https://codereview.chromium.org/13839023/diff/1/utils/tests/pub/test_pub.dart...
utils/tests/pub/test_pub.dart:53: captureStackTraces = false;
I'd rather put this in [integration], especially once captureStackTraces is a
property on Schedule.

Also, please make this configurable via the command line somehow. Maybe an
environment variable?

[Bob Nystrom, 2013-04-10 21:52:00]

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/lib/schedu...
File pkg/scheduled_test/lib/scheduled_test.dart (right):

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/lib/schedu...
pkg/scheduled_test/lib/scheduled_test.dart:210: bool captureStackTraces = true;
On 2013/04/09 21:25:58, nweiz wrote:
> I'd rather see this on the Schedule class. That's where other scheduled test
> configuration has gone.

Done.

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/lib/src/sc...
File pkg/scheduled_test/lib/src/schedule_error.dart (right):

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/lib/src/sc...
pkg/scheduled_test/lib/src/schedule_error.dart:58: stackTrace =
thrownStackTrace;
On 2013/04/09 21:25:58, nweiz wrote:
> While you're at it, change this to use [Trace.capture].

Trace.current()? Done.

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/lib/src/ta...
File pkg/scheduled_test/lib/src/task.dart (right):

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/lib/src/ta...
pkg/scheduled_test/lib/src/task.dart:125: var result = "$this";
On 2013/04/09 21:25:58, nweiz wrote:
> I think I prefer `this.toString()` here.

Done.

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/test/sched...
File pkg/scheduled_test/test/scheduled_test/capture_stack_traces_test.dart
(right):

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/test/sched...
pkg/scheduled_test/test/scheduled_test/capture_stack_traces_test.dart:20: error
= currentSchedule.errors.first.toString();
On 2013/04/09 21:25:58, nweiz wrote:
> To be consistent with other tests, don't convert this to a string until test
2.
> Same for the test below.

Done.

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/test/sched...
pkg/scheduled_test/test/scheduled_test/capture_stack_traces_test.dart:30:
expect(numStackTraces, equals(2));
On 2013/04/09 21:25:58, nweiz wrote:
> expect(error.toString(), stringContainsInOrder('Stack trace:', 'Stack
trace:'))
> 
> Also, for future reference, a RegExp isn't necessary here; you could just use
> [String.allMatches].

Done.

https://codereview.chromium.org/13839023/diff/1/pkg/scheduled_test/test/sched...
pkg/scheduled_test/test/scheduled_test/capture_stack_traces_test.dart:49:
expect(numStackTraces, equals(1));
On 2013/04/09 21:25:58, nweiz wrote:
> expect(error.toString(), isNot(contains('Stack trace:')));

It will have one instance of 'Stack trace' in it. See the above comment.

[Bob Nystrom, 2013-04-10 22:40:34]

https://codereview.chromium.org/13839023/diff/1/utils/tests/pub/test_pub.dart
File utils/tests/pub/test_pub.dart (right):

https://codereview.chromium.org/13839023/diff/1/utils/tests/pub/test_pub.dart...
utils/tests/pub/test_pub.dart:53: captureStackTraces = false;
On 2013/04/09 21:25:58, nweiz wrote:
> I'd rather put this in [integration], especially once captureStackTraces is a
> property on Schedule.

Done.

> Also, please make this configurable via the command line somehow. Maybe an
> environment variable?

Done. Add "--trace" to the command line.

[nweiz, 2013-04-10 22:57:07]

lgtm

[Bob Nystrom, 2013-04-10 23:21:07]

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