Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various fixes #60

Merged
merged 6 commits into from
Aug 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ node_modules
npm-debug.log
.env
coverage
.vscode
package-lock.json
10 changes: 9 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ Request.prototype.abort = function () {
};

// expose request methods from RequestRetry
['end', 'on', 'emit', 'once', 'setMaxListeners', 'start', 'removeListener', 'pipe', 'write'].forEach(function (requestMethod) {
['end', 'on', 'emit', 'once', 'setMaxListeners', 'start', 'removeListener', 'pipe', 'write', 'auth'].forEach(function (requestMethod) {
Request.prototype[requestMethod] = function exposedRequestMethod () {
return this._req[requestMethod].apply(this._req, arguments);
};
Expand Down Expand Up @@ -216,6 +216,10 @@ function defaults(defaultOptions, defaultF) {
});
factory.del = factory['delete'];

['jar', 'cookie'].forEach(function (method) {
factory[method] = factory.Request.request[method];
});

return factory;
}

Expand All @@ -230,3 +234,7 @@ Factory.RetryStrategies = RetryStrategies;
makeHelper(Factory, verb);
});
Factory.del = Factory['delete'];

['jar', 'cookie'].forEach(function (method) {
Factory[method] = Factory.Request.request[method];
});
18 changes: 18 additions & 0 deletions test/auth.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
'use strict';

var request = require('../');
var t = require('chai').assert;

describe('Auth', function () {
it('should work with Basic Authentication', function (done) {
var r = request.defaults({
json: true
});
r('http://httpbin.org/basic-auth/user/passwd', function (err, response, body) {
t.strictEqual(response.statusCode, 200);
t.strictEqual(body.authenticated, true);
t.strictEqual(body.user, 'user');
done();
}).auth('user', 'passwd', false);
});
});
23 changes: 23 additions & 0 deletions test/cookie.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';

var request = require('../');
var t = require('chai').assert;

describe('Cookie', function () {
it('should work with custom cookie jar', function (done) {
const j = request.jar();
var url = 'http://httpbin.org/cookies/set?k2=v2&k1=v1';
request({
url,
jar: j,
json: true
}, function (err, response, body) {
t.strictEqual(response.statusCode, 200);
var cookies = j.getCookies(url);
t.strictEqual(cookies.length, 2);
t.strictEqual(cookies.filter(c => c.key === 'k1')[0].value, 'v1');
t.strictEqual(cookies.filter(c => c.key === 'k2')[0].value, 'v2');
done();
});
});
});
12 changes: 6 additions & 6 deletions test/defaults.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ describe('Defaults', function () {
});
r('http://www.filltext.com/?rows=1', function (err, response, body) {
t.strictEqual(response.statusCode, 200);
t.strictEqual(body[0].d, 1);
t.isNumber(body[0].d);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, why not. I prefer to ensure that the response is as it should be (because it's free here to do so), why would we want to prefer isNumber here?

Copy link
Contributor Author

@dgkanatsios dgkanatsios Jul 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in PR comment, filltext.com does not return 1 but a random integer, e.g. try running

curl http://www.filltext.com/?rows=1&d=index

For me, it returns 17120, so the strictEqual tests were failing. I replaced them with isNumber so that they pass.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

done();
});
});

it('should set a default function', function (done) {
var r = request.defaults({}, function (err, response, body) {
t.strictEqual(response.statusCode, 200);
t.strictEqual(body[0].d, 1);
t.isNumber(body[0].d);
done();
});
r({ url: 'http://www.filltext.com/?rows=1&d={index}', json: true });
Expand All @@ -32,7 +32,7 @@ describe('Defaults', function () {
});
r.get({ url: 'http://www.filltext.com/?rows=1', qs: { d: "{index}" } }, function (err, response, body) {
t.strictEqual(response.statusCode, 200);
t.strictEqual(body[0].d, 1);
t.isNumber(body[0].d);
done();
});
});
Expand All @@ -49,7 +49,7 @@ describe('Defaults', function () {
fullResponse: false
});
level2.get('/?rows=1').then(function (body) {
t.strictEqual(body[0].d, 1);
t.isNumber(body[0].d);
done();
});
});
Expand All @@ -61,10 +61,10 @@ describe('Defaults', function () {
});
r({ url: 'http://www.filltext.com/?rows=1', qs: { x: "test" } }, function (err, response, body) {
t.strictEqual(response.statusCode, 200);
t.strictEqual(body[0].d, 1);
t.isNumber(body[0].d);
t.strictEqual(body[0].x, "test");
done();
});
});

});
});
2 changes: 1 addition & 1 deletion test/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,4 @@ describe('Helpers', function () {
}
});

});
});