Updated the early close test, to use a Socket that we can close at any time.

Updated the early close test, to use a Socket that we can close at any time.
Committed: https://code.google.com/p/dart/source/detail?r=6029

[Søren Gjesse, 2012-03-30 08:56:53]

lgtm, with comments

Didn't you want to create a new test HttpClientEarlyCloseTest.dart instead of
overwriting
HttpServerEarlyCloseTest.dart?

https://chromiumcodereview.appspot.com/9965008/diff/1/tests/standalone/src/io...
File tests/standalone/src/io/HttpServerEarlyCloseTest.dart (right):

https://chromiumcodereview.appspot.com/9965008/diff/1/tests/standalone/src/io...
tests/standalone/src/io/HttpServerEarlyCloseTest.dart:28: server.onRequest =
(HttpRequest request, HttpResponse response) {
Maybe check that each onRequest function are only called once.

https://chromiumcodereview.appspot.com/9965008/diff/1/tests/standalone/src/io...
tests/standalone/src/io/HttpServerEarlyCloseTest.dart:32: server.onError =
(Exception error) {
Maybe set onRequest to a failure function here, just to check that there are no
unexpected requests.

https://chromiumcodereview.appspot.com/9965008/diff/1/tests/standalone/src/io...
tests/standalone/src/io/HttpServerEarlyCloseTest.dart:42: sendData(d,
server.port).then((_) {
We don't want timed tests, as they are  bound to be flaky. If onError is never
called the test will just time out.

[Anders Johnsen, 2012-03-30 09:29:27]

This is doing exactly what the old test was doing, except it does it a lot
better and covers a way bigger range. :)

But I agree that it's the client that performs the early close, it's just the
http server we tests. Maybe rename to HttpServerEarlyClientCloseTest.dart?

https://chromiumcodereview.appspot.com/9965008/diff/1/tests/standalone/src/io...
File tests/standalone/src/io/HttpServerEarlyCloseTest.dart (right):

https://chromiumcodereview.appspot.com/9965008/diff/1/tests/standalone/src/io...
tests/standalone/src/io/HttpServerEarlyCloseTest.dart:28: server.onRequest =
(HttpRequest request, HttpResponse response) {
On 2012/03/30 08:56:53, Søren Gjesse wrote:
> Maybe check that each onRequest function are only called once.

Done.

https://chromiumcodereview.appspot.com/9965008/diff/1/tests/standalone/src/io...
tests/standalone/src/io/HttpServerEarlyCloseTest.dart:32: server.onError =
(Exception error) {
On 2012/03/30 08:56:53, Søren Gjesse wrote:
> Maybe set onRequest to a failure function here, just to check that there are
no
> unexpected requests.

Ahh yes, very nice catch!

https://chromiumcodereview.appspot.com/9965008/diff/1/tests/standalone/src/io...
tests/standalone/src/io/HttpServerEarlyCloseTest.dart:42: sendData(d,
server.port).then((_) {
On 2012/03/30 08:56:53, Søren Gjesse wrote:
> We don't want timed tests, as they are  bound to be flaky. If onError is never
> called the test will just time out.

Done.

[Anders Johnsen, 2012-03-30 10:13:39]

How about this? :)

[Søren Gjesse, 2012-03-30 10:14:23]

lgtm

[ahe, 2012-03-30 13:29:51]

DBC

Timeout doesn't work that well, so it is better to skip tests that time out. 

The reason timeout doesn't work well is that we have test suites that complete
in a few seconds, but now you have to wait a minute for this test to time out.

Cheers,
Peter

[ahe, 2012-03-30 13:29:54]

DBC

Timeout doesn't work that well, so it is better to skip tests that time out. 

The reason timeout doesn't work well is that we have test suites that complete
in a few seconds, but now you have to wait a minute for this test to time out.

Cheers,
Peter