Skip to content

Commit bfe2c97

Browse files
authored
Fix a broken test for redirects (#317)
This test added a `.catchError` callback with some expects, but the `Future` was completing normally so the callback and expects never ran. The reason the response came back successfully was that the IO client does not consider a `302` to be a redirect when the method is `'POST'`. https://github.com/dart-lang/sdk/blob/ed9e89ea388e4bc0142bf6e967d4ca11999bdfdc/sdk/lib/_http/http_impl.dart#L355-L365 - Refactor to async/await to clean up the noise of `.then`, `.whenComplete`, and `.catchError` and make the logic easier to follow. - Switch the request type to `'GET'` so that the exception occurs. - Move server starting and stopping to a `setUp` and `tearDown`. - Fix the expectation to look for the wrapped `ClientException` instead of the exception thrown by the IO client. We also lose the underlying exception and have only the message so we can't check the length of the `redirects` field. - Remove the now unused test utility to get a matcher on the old exception type.
1 parent 506d251 commit bfe2c97

File tree

2 files changed

+37
-58
lines changed

2 files changed

+37
-58
lines changed

test/io/request_test.dart

Lines changed: 37 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -10,61 +10,48 @@ import 'package:test/test.dart';
1010
import 'utils.dart';
1111

1212
void main() {
13-
test('.send', () {
14-
expect(
15-
startServer().then((_) {
16-
var request = http.Request('POST', serverUrl)
17-
..body = 'hello'
18-
..headers['User-Agent'] = 'Dart';
13+
setUp(startServer);
1914

20-
expect(
21-
request.send().then((response) {
22-
expect(response.statusCode, equals(200));
23-
return response.stream.bytesToString();
24-
}).whenComplete(stopServer),
25-
completion(parse(equals({
26-
'method': 'POST',
27-
'path': '/',
28-
'headers': {
29-
'content-type': ['text/plain; charset=utf-8'],
30-
'accept-encoding': ['gzip'],
31-
'user-agent': ['Dart'],
32-
'content-length': ['5']
33-
},
34-
'body': 'hello'
35-
}))));
36-
}),
37-
completes);
38-
});
15+
tearDown(stopServer);
16+
17+
test('send happy case', () async {
18+
final request = http.Request('GET', serverUrl)
19+
..body = 'hello'
20+
..headers['User-Agent'] = 'Dart';
21+
22+
final response = await request.send();
3923

40-
test('#followRedirects', () {
24+
expect(response.statusCode, equals(200));
25+
final bytesString = await response.stream.bytesToString();
4126
expect(
42-
startServer().then((_) {
43-
var request = http.Request('POST', serverUrl.resolve('/redirect'))
44-
..followRedirects = false;
45-
var future = request.send().then((response) {
46-
expect(response.statusCode, equals(302));
47-
});
48-
expect(
49-
future.catchError((_) {}).then((_) => stopServer()), completes);
50-
expect(future, completes);
51-
}),
52-
completes);
27+
bytesString,
28+
parse(equals({
29+
'method': 'GET',
30+
'path': '/',
31+
'headers': {
32+
'content-type': ['text/plain; charset=utf-8'],
33+
'accept-encoding': ['gzip'],
34+
'user-agent': ['Dart'],
35+
'content-length': ['5']
36+
},
37+
'body': 'hello',
38+
})));
39+
});
40+
41+
test('without redirects', () async {
42+
final request = http.Request('GET', serverUrl.resolve('/redirect'))
43+
..followRedirects = false;
44+
final response = await request.send();
45+
46+
expect(response.statusCode, equals(302));
5347
});
5448

55-
test('#maxRedirects', () {
49+
test('exceeding max redirects', () async {
50+
final request = http.Request('GET', serverUrl.resolve('/loop?1'))
51+
..maxRedirects = 2;
5652
expect(
57-
startServer().then((_) {
58-
var request = http.Request('POST', serverUrl.resolve('/loop?1'))
59-
..maxRedirects = 2;
60-
var future = request.send().catchError((error) {
61-
expect(error, isRedirectLimitExceededException);
62-
expect(error.redirects.length, equals(2));
63-
});
64-
expect(
65-
future.catchError((_) {}).then((_) => stopServer()), completes);
66-
expect(future, completes);
67-
}),
68-
completes);
53+
request.send(),
54+
throwsA(isA<http.ClientException>()
55+
.having((e) => e.message, 'message', 'Redirect limit exceeded')));
6956
});
7057
}

test/io/utils.dart

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -119,14 +119,6 @@ void stopServer() {
119119
/// A matcher for functions that throw HttpException.
120120
Matcher get throwsClientException => throwsA(TypeMatcher<ClientException>());
121121

122-
/// A matcher for RedirectLimitExceededExceptions.
123-
final isRedirectLimitExceededException = const TypeMatcher<RedirectException>()
124-
.having((e) => e.message, 'message', 'Redirect limit exceeded');
125-
126-
/// A matcher for functions that throw RedirectLimitExceededException.
127-
final Matcher throwsRedirectLimitExceededException =
128-
throwsA(isRedirectLimitExceededException);
129-
130122
/// A matcher for functions that throw SocketException.
131123
final Matcher throwsSocketException =
132124
throwsA(const TypeMatcher<SocketException>());

0 commit comments

Comments
 (0)