Skip to content
This repository was archived by the owner on Nov 8, 2024. It is now read-only.

Support setting Request encoding for response #836

Closed
wants to merge 2 commits into from
Closed

Support setting Request encoding for response #836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fad213d
feat: Set transaction.encoding for Request
bb010g Aug 3, 2017
3056e4d
fix: Switch to prepare from prepublish
bb010g Aug 3, 2017
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
1 change: 1 addition & 0 deletions docs/data-structures.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Transaction object is passed as a first argument to [hook functions](hooks.md) a
- headers (object) - keys are HTTP header names, values are HTTP header contents
- uri: `/message` (string) - request URI as it was written in API description
- method: `POST` (string)
- encoding: `utf8` (object) - response encoding
- expected (object) - the HTTP response Dredd expects to get from the tested server
- statusCode: `200` (string)
- headers (object) - keys are HTTP header names, values are HTTP header contents
Expand Down
4 changes: 3 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
"pretest": "npm run build",
"test": "mocha \"test/**/*-test.coffee\"",
"test:coverage": "scripts/coverage.sh",
"prepublish": "npm run build",
"prepare": "npm run build",
"prepublish": "check-node-version --npm \">=4\" || npm run prepare",
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is necessary. Dredd is being published on CI exclusively, by Semantic Release. Also, the engines field clearly states "node": ">= 4".

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I did not notice the commit message during review:

See https://iamakulov.com/notes/npm-4-prepublish/ , npm/npm#16685 , and https://stackoverflow.com/a/44136814/1098906

Could you send this change in a separate PR, please? At the same time, I'm still not completely sure about it, because Dredd is being published on CI exclusively, so we could just make sure that package.json contents match the npm version which publishes the package. That basically means just pinning npm to a specific version in the Travis CI config and then having the package.json specific to that npm version.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I can separate that out. I ended up sticking it in here because of the need to depend on my Git branch with Yarn, and yarnpkg/yarn#2875 / yarnpkg/yarn#3553 requiring an explicit prepare for that. The benefit of using this slightly more defensive syntax is that, if you need to do source compilation for whatever reason, you can still work with an older npm. If we don't care about npm 3 compatibility though, the prepublish script can be dropped entirely.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm makes sense. Let's do this, then!

"coveralls": "scripts/coveralls.sh",
"semantic-release": "scripts/semantic-release.sh"
},
Expand Down Expand Up @@ -55,6 +56,7 @@
},
"devDependencies": {
"body-parser": "^1.17.1",
"check-node-version": "^2.1.0",
"coffee-coverage": "^2.0.1",
"coffeelint": "^1.15.7",
"conventional-changelog-lint": "^1.1.9",
Expand Down
1 change: 1 addition & 0 deletions src/transaction-runner.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,7 @@ class TransactionRunner
options.method = transaction.request.method
options.headers = transaction.request.headers
options.body = transaction.request.body
options.encoding = transaction.request.encoding
options.proxy = false
options.followRedirect = false
return options
Expand Down