Replace broken language/async_star_await_pauses_test

Replace broken language/async_star_await_pauses_test

BUG=https://github.com/dart-lang/sdk/issues/23996
R=ahe@google.com, hausner@google.com

Committed: https://github.com/dart-lang/sdk/commit/daeb7cc3bb0725307057d3485c83a3c7a3f88bd5

[Bill Hesse, 2016-04-07 13:54:31]

Description was changed from

==========
Remove broken language/async_star_await_pauses_test

BUG=https://github.com/dart-lang/sdk/issues/23996
==========

to

==========
Replace broken language/async_star_await_pauses_test

BUG=https://github.com/dart-lang/sdk/issues/23996
==========

[Bill Hesse, 2016-04-07 13:54:31]

whesse@google.com changed reviewers:
+ kustermann@google.com

[Bill Hesse, 2016-04-07 15:26:45]

whesse@google.com changed reviewers:
+ hausner@google.com, iposva@google.com

[Bill Hesse, 2016-04-07 15:26:45]

Forgot to publish: PTAL.

[hausner, 2016-04-07 18:01:28]

DBC: I don't have the detailed understanding of streams needed to lgtm this. If
would be nice to have some comments (in addition to the descriptive identifiers
in the test code) that explain what aspect of the stream and async
implementation is being verified.

Thank you for fixing the test.

[Bill Hesse, 2016-04-08 11:20:18]

whesse@google.com changed reviewers:
+ ahe@google.com

[ahe, 2016-04-08 11:46:15]

Looking at the version with comments now.

https://codereview.chromium.org/1869583003/diff/20001/tests/language/async_st...
File tests/language/async_star_await_pauses_test.dart (right):

https://codereview.chromium.org/1869583003/diff/20001/tests/language/async_st...
tests/language/async_star_await_pauses_test.dart:1: // Copyright (c) 2016, the
Dart project authors.  Please see the AUTHORS file
The name of this test is odd. I think it should be renamed to
async_await_for_pauses_text.dart as it doesn't use "async*", and tests "await
for", not "await".

https://codereview.chromium.org/1869583003/diff/20001/tests/language/async_st...
tests/language/async_star_await_pauses_test.dart:19: send() async {
Since this is a language feature test of "await for", I think it would be best
to avoid using other async/await features. I know this is less convenient.

[ahe, 2016-04-08 12:00:01]

lgtm

https://codereview.chromium.org/1869583003/diff/40001/tests/language/async_st...
File tests/language/async_star_await_pauses_test.dart (right):

https://codereview.chromium.org/1869583003/diff/40001/tests/language/async_st...
tests/language/async_star_await_pauses_test.dart:43: await
pauseEventReceived.future;
Add a comment here, something along:

pauseEventReceived.future completes when controller.stream is paused by the
await-for loop below. What's specified is that await-for must pause immediately
on an "await", but instead the implementations agree on not pausing until
receiving the next event. For this reason, [timer] will call it's callback at
least once before we cancel it again.

https://codereview.chromium.org/1869583003/diff/40001/tests/language/async_st...
tests/language/async_star_await_pauses_test.dart:50: receive() async {
I suggest adding something like this:

if (entry == 'second') {
  gotSecondEntry = true;
}
if (entry == 'third') {
  gotThirdEntry = true;
}

And then check for that after "await f" below.

[Bill Hesse, 2016-04-08 12:16:47]

https://codereview.chromium.org/1869583003/diff/40001/tests/language/async_st...
File tests/language/async_star_await_pauses_test.dart (right):

https://codereview.chromium.org/1869583003/diff/40001/tests/language/async_st...
tests/language/async_star_await_pauses_test.dart:43: await
pauseEventReceived.future;
On 2016/04/08 12:00:01, ahe wrote:
> Add a comment here, something along:
> 
> pauseEventReceived.future completes when controller.stream is paused by the
> await-for loop below. What's specified is that await-for must pause
immediately
> on an "await", but instead the implementations agree on not pausing until
> receiving the next event. For this reason, [timer] will call it's callback at
> least once before we cancel it again.

Done.

https://codereview.chromium.org/1869583003/diff/40001/tests/language/async_st...
tests/language/async_star_await_pauses_test.dart:50: receive() async {
On 2016/04/08 12:00:01, ahe wrote:
> I suggest adding something like this:
> 
> if (entry == 'second') {
>   gotSecondEntry = true;
> }
> if (entry == 'third') {
>   gotThirdEntry = true;
> }
> 
> And then check for that after "await f" below.

There is no guarantee that we get a "second", since if
something implements the spec correctly, we won't get it.
I could check for it, with a comment that this check can be
removed, or just not check for it.

It seems pretty unlikely for the loop to exit without reading the "third" and
the close, but I'll add the check.

[ahe, 2016-04-08 12:20:17]

https://codereview.chromium.org/1869583003/diff/40001/tests/language/async_st...
File tests/language/async_star_await_pauses_test.dart (right):

https://codereview.chromium.org/1869583003/diff/40001/tests/language/async_st...
tests/language/async_star_await_pauses_test.dart:50: receive() async {
On 2016/04/08 12:16:47, Bill Hesse wrote:
> There is no guarantee that we get a "second", since if
> something implements the spec correctly, we won't get it.
> I could check for it, with a comment that this check can be
> removed, or just not check for it.

Good point. Hopefully, they'll work on changing the specification.

[Bill Hesse, 2016-04-08 12:21:15]

Description was changed from

==========
Replace broken language/async_star_await_pauses_test

BUG=https://github.com/dart-lang/sdk/issues/23996
==========

to

==========
Replace broken language/async_star_await_pauses_test

BUG=https://github.com/dart-lang/sdk/issues/23996
R=ahe@google.com, hausner@google.com

Committed:
https://github.com/dart-lang/sdk/commit/daeb7cc3bb0725307057d3485c83a3c7a3f88bd5
==========

[Bill Hesse, 2016-04-08 12:21:17]

Committed patchset #4 (id:60001) manually as
daeb7cc3bb0725307057d3485c83a3c7a3f88bd5 (presubmit successful).