Improve request tests. #48

maier49 · 2015-07-07T23:18:53Z

Refs #34

Now provides an echo server on port 9001 that proxies most
calls to the intern server, except for requests to /__echo/.

Calls to /__echo/ return a JSON response that contains the
parsed query string, headers, and payload from the request.

If a responseType query parameter is specified, a response
in the format requested will be returned if available.
Currently, 'html', 'document', 'xml', 'json', and 'gif'
are acceptable values for responseType. If any format other
than JSON is requested the response body will not contain
the payload, query or headers from the request.

The X-Requested-With header is no longer being used for
testing custom headers because IE 11 was modifying the
value before sending to the server.

Added tests for different content types and to improve
coverage.

maier49 · 2015-07-08T13:29:26Z

Currently ~~two~~ three of these tests fail because request/xhr is not handling the query property from XhrRequestOptions. Once that is fixed those tests should start passing.

kfranqueiro · 2015-07-10T15:08:57Z

We're reassigning this to @maier49 so that he can test #49 when addressing that too.

* Add `request/util` module with `generateRequestUrl` method. * Append query string to end of request URL. * Update `RequesOptions#query` to accept either a string or `ParamList` from `UrlSearchParams`.

Refs dojo#34 Now provides an echo server on port 9001 that proxies most calls to the intern server, except for requests to /__echo/. Calls to /__echo/ return a JSON response that contains the parsed query string, headers, and payload from the request. If a responseType query parameter is specified, a response in the format requested will be returned if available. Currently, 'html', 'document', 'xml', 'json', and 'gif' are acceptable values for responseType. If any format other than JSON is requested the response body will not contain the payload, query or headers from the request. The X-Requested-With header is no longer being used for testing custom headers because IE 11 was modifying the value before sending to the server. Added tests for different content types and to improve coverage.

maier49 · 2015-07-16T19:18:48Z

This now includes @mwistrand 's code for handling the query and cacheBust options, as well as test cases for those changes, so it should resolve #34 and #49.

bryanforbes · 2015-07-20T22:24:41Z

src/request/util.ts

+		query += query ? '&' + cacheBust : cacheBust;
+	}
+
+	const separator = url.indexOf('?') > -1 ? '&' : '?';


Is there a reason we're doing all of this logic here and on line 19 when we have it in UrlSearchParams?

I removed the redundant logic at the beginning of the function, but I wasn't sure how line 19 could also be removed, since UrlSearchParams doesn't deal with the presence or absence of '?'.

Currently, URLSearchParams will take a full URL and return a full URL. This is the wrong behavior according to the spec (which I just checked), so I thought we could use that to our advantage. Leave this line. Nice work on removing the redundant logic :).

Aaaaaaaaaand ignore me on UrlSearchParams being wrong. I'm on a roll today.

* Used deepEqual instead of multiple assertions * Removed redundant logic and delegated to UrlSearchParams * Added timeout so that the second timestamp in cacheBust tests will alwasy be different from the first.

Changing more comparisons to use deepEqual.

jason0x43 · 2015-07-24T21:01:06Z

tests/typings/tsd.d.ts

@@ -1,3 +1,6 @@
 /// <reference path="intern/intern.d.ts" />
 /// <reference path="benchmark/benchmark.d.ts" />
 /// <reference path="sinon/sinon.d.ts" />
+/// <reference path="formidable/formidable.d.ts" />
+/// <reference path="http-proxy/http-proxy.d.ts" />
+/// <reference path="services/echo.d.ts" />


Are we still doing typings in a tsd, or should they just be defined in the tsconfig.json?

Good catch, I removed the tsd file and everything is still working.

jason0x43 · 2015-07-27T12:32:20Z

src/request/xhr.ts

@@ -67,8 +69,15 @@ export default function xhr<T>(url: string, options: XhrRequestOptions = {}): Re

 				response.statusCode = request.status;
 				response.statusText = request.statusText;
-
-				resolve(response);
+				if (response.statusCode >= 200 && response.statusCode < 400) {


Would this be better as just response.statusCode < 400 since the 100 codes aren't errors?

This ties into your comment on the 'timeout' test that's failing. When the request times out, prior to the ontimeout callback getting fired, this code gets fired with a status code of 0. I looked into it and apparently 0 is a valid value for XMLHttpRequest's statusCode property, indicating an error. I could change this to response.statusCode < 400 && response.statusCode !== 0, but I'm still not sure why this is getting called with a statusCode of 0, and how we want to handle that with regards to the 'timeout' test.

jason0x43 · 2015-07-27T13:12:50Z

tests/unit/request/xhr.ts

-			const dfd = this.async();
-			xhrRequest(getRequestUrl('foo.json'))
+		'default'() {
+			if (!echoServerAvailable) {


This test isn't using the echo server.

* Placed single callbacks on the same line as then. * Made all tests in xhr.ts use the echo server. * Used more appropriate propertyVal assertion. * Called server.close in a callback function to preserve context. * Added a leading slash to '/runner/end' in intern.ts * Passed server to resolve in the callback for the listening event in echo.ts.

maier49 · 2015-07-27T15:58:32Z

With the exception of the issue regarding status codes and the timeout test I mentioned in a comment above, all the issues @jason0x43 commented on should be resolved now.

jdonaghue · 2015-07-28T19:00:24Z

tests/services/echo.ts

+				const parser = new formidable.IncomingForm();
+				parser.parse(request, function (err: any, fields: formidable.Fields, files: formidable.Files) {
+					if (err) {
+						reject(err);


this should probably return, currently it will flow through

Added a return after reject.

kfranqueiro mentioned this pull request Jul 8, 2015

request should append query/cache-bust strings to the end of the URL. #49

Closed

eheasley assigned maier49 Jul 10, 2015

eheasley added this to the Milestone 2 milestone Jul 10, 2015

Matt Wistrand and others added 2 commits July 16, 2015 15:13

Add cache busting and query string handling to request.

efec0ac

* Add `request/util` module with `generateRequestUrl` method. * Append query string to end of request URL. * Update `RequesOptions#query` to accept either a string or `ParamList` from `UrlSearchParams`.

maier49 force-pushed the improve-request-tests branch from a3eadc6 to 7cbfaa7 Compare July 16, 2015 19:15

Removing reference to jsgi from comment.

b76d72f

bryanforbes assigned bryanforbes and unassigned maier49 Jul 20, 2015

bryanforbes reviewed Jul 20, 2015
View reviewed changes

maier49 added 6 commits July 21, 2015 15:19

PR fixes

0a94701

* Used deepEqual instead of multiple assertions * Removed redundant logic and delegated to UrlSearchParams * Added timeout so that the second timestamp in cacheBust tests will alwasy be different from the first.

PR fixes

193f3ef

Changing more comparisons to use deepEqual.

PR fixes

4a66c8d

Abort if the request resolves when it should timeout.

2c64da1

Remove unused url query parameter.

39a6259

Handle the case where XMLHttpRequest.statusCode is 0

460face

jason0x43 reviewed Jul 24, 2015
View reviewed changes

Remove unnecessary tsd.d.ts file.

323dd65

jason0x43 reviewed Jul 27, 2015
View reviewed changes

jdonaghue reviewed Jul 28, 2015
View reviewed changes

Return after rejecting in echo.ts

6c3a807

bryanforbes closed this in 9349b9e Aug 10, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve request tests. #48

Improve request tests. #48

maier49 commented Jul 7, 2015

maier49 commented Jul 8, 2015

kfranqueiro commented Jul 10, 2015

maier49 commented Jul 16, 2015

bryanforbes Jul 20, 2015

maier49 Jul 21, 2015

bryanforbes Jul 21, 2015

bryanforbes Jul 21, 2015

jason0x43 Jul 24, 2015

maier49 Jul 24, 2015

jason0x43 Jul 27, 2015

maier49 Jul 27, 2015

jason0x43 Jul 27, 2015

maier49 commented Jul 27, 2015

jdonaghue Jul 28, 2015

maier49 Jul 29, 2015

Improve request tests. #48

Improve request tests. #48

Conversation

maier49 commented Jul 7, 2015

maier49 commented Jul 8, 2015

kfranqueiro commented Jul 10, 2015

maier49 commented Jul 16, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maier49 commented Jul 27, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment