Make package:scheduled_test throw ScheduledServerErrors, not strings.

Fix failures in pkg/scheduled_test scheduled_server_test.

The package and test were throwing strings, which do not have attached stack traces because they are primitives.


BUG=dartbug.com/9582
R=nweiz@google.com

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

[Bill Hesse, 2013-09-18 14:32:47]

This test is cluttering up the flaky logs, making it harder to see the real
flakes.  And it is easy to fix.

[nweiz, 2013-09-18 22:46:02]

https://codereview.chromium.org/23591061/diff/1/pkg/pkg.status
File pkg/pkg.status (right):

https://codereview.chromium.org/23591061/diff/1/pkg/pkg.status#newcode17
pkg/pkg.status:17: scheduled_test/test/scheduled_server_test: Pass, Slow # Issue
9231, 9582
These issue references can be removed.

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

https://codereview.chromium.org/23591061/diff/1/pkg/scheduled_test/test/sched...
pkg/scheduled_test/test/scheduled_server_test.dart:174: // The second is now a
copy of the first, rather than a copy of the third.
This comment shouldn't look like a conversation; it should describe the current
state of the system.

Why are there three exceptions now? If the exceptions have the correct stack
traces, two identical exceptions shouldn't register as different.

Same goes for the following tests.

[Bill Hesse, 2013-09-19 16:56:15]

I fixed the root cause of the new errors on the test, which is that primitives
no longer have attached stack traces when they are thrown.  Since
ScheduledServer threw strings when it encountered errors, they were being
double-reported by the ScheduledTest framework.

There is still one case in which an HttpException is being doubly reported, even
though the bug is marked as fixed.  I removed the code from all the other cases
though.  I don't know the system well enough to diagnose the remaining case.

ScheduledProcess and Descriptor still throw some errors as strings, and this
should be changed.

[nweiz, 2013-09-19 19:56:13]

Can you check whether the remaining test issue is in fact caused by
HttpException failing to have an attached stack trace? You can do so by adding
something like the following to [ScheduleError.from]:

    if (error is HttpException) {
      print("Stack trace for $error:\n${getAttachedStackTrace(error)}");
    }

Also, I think you missed my comment in pkg.status.

https://codereview.chromium.org/23591061/diff/9001/pkg/scheduled_test/lib/sch...
File pkg/scheduled_test/lib/scheduled_server.dart (right):

https://codereview.chromium.org/23591061/diff/9001/pkg/scheduled_test/lib/sch...
pkg/scheduled_test/lib/scheduled_server.dart:108: class ScheduledServerError
extends Error {
I'm not a fan of using a custom error class here. Since these errors are
intended to cause test failures, we can just use [fail] instead of "throw".

[Bill Hesse, 2013-09-20 13:30:20]

https://codereview.chromium.org/23591061/diff/9001/pkg/pkg.status
File pkg/pkg.status (right):

https://codereview.chromium.org/23591061/diff/9001/pkg/pkg.status#newcode17
pkg/pkg.status:17: scheduled_test/test/scheduled_server_test: Pass, Slow # Issue
9231, 9582
The issue 9231 is still why this test is marked Slow.  I think the (closed)
issue number should remain.  Removing 9582, but waiting to see if the test is
flaky before closing the issue.

https://codereview.chromium.org/23591061/diff/9001/pkg/scheduled_test/lib/sch...
File pkg/scheduled_test/lib/scheduled_server.dart (right):

https://codereview.chromium.org/23591061/diff/9001/pkg/scheduled_test/lib/sch...
pkg/scheduled_test/lib/scheduled_server.dart:108: class ScheduledServerError
extends Error {
On 2013/09/19 19:56:14, nweiz wrote:
> I'm not a fan of using a custom error class here. Since these errors are
> intended to cause test failures, we can just use [fail] instead of "throw".

Done.

[Bill Hesse, 2013-09-23 16:41:12]

I think all comments are addressed.  The note on issue 9151 points out that that
HttpException without a stack trace is not an error.  So we should leave in the
code to handle it, or make the scheduled_test error framework un-duplicate it in
some other way, I think.

I would like to get this in to see if there are any flaky failures or crashes,
which were masked by this issue.

[nweiz, 2013-09-23 20:06:39]

lgtm

[Bill Hesse, 2013-09-24 00:00:51]

Committed patchset #5 manually as r27790 (presubmit successful).