-
-
Notifications
You must be signed in to change notification settings - Fork 916
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
Loosen unnecessary restrictions on the body
option
#521
Loosen unnecessary restrictions on the body
option
#521
Conversation
test/error.js
Outdated
body.push(chunk); | ||
}).on('end', () => { | ||
res.end(Buffer.concat(body).toString()); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use get-stream
here for simplicity
test/error.js
Outdated
@@ -54,17 +63,27 @@ test('dns message', async t => { | |||
|
|||
test('options.body error message', async t => { | |||
const err = await t.throws(got(s.url, {body: {}})); | |||
t.regex(err.message, /The `body` option must be a stream\.Readable, string or Buffer/); | |||
t.is(err.message, 'The `body` option must be a stream.Readable, string or Buffer'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should change all of the t.throws
assertions to the new format. Should not be done in this PR though. Just a note to myself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so after this gets merged I'll do another PR for that.
test/error.js
Outdated
this.a = 123; | ||
} | ||
|
||
const {body} = (await got(`${s.url}/body`, {body: new CustomObject(), json: true})); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need parens around await
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. Forgot because before I did const body = (...).body
then I used const { body } = ...
:)
Fixes #381