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

Tentative fix for #77 and #154 #17

Merged
merged 2 commits into from Jul 4, 2013
Merged

Tentative fix for #77 and #154 #17

merged 2 commits into from Jul 4, 2013

Conversation

Zlika
Copy link

@Zlika Zlika commented Jul 2, 2013

Tentative fix for #77 and #154. The default behaviour ("Accept-Encoding: gzip,deflate") is kept to avoid breaking backward compatibility.
The only problem I see with this fix is that it uses an internal type (com.jayway.restassured.internal.http.ContentEncoding) in the public API of RequestSpecification, but this problem already exists for ObjectMapperType. Maybe this type could be moved outside the internal package ? (I would like to avoid creating a new public "ContentEncoding" that would be a mere copy-paste of the original one)

…is that it uses an internal type (com.jayway.restassured.internal.http.ContentEncoding) in the public API : could this type be moved outside the internal package ?
@johanhaleby
Copy link
Collaborator

Thanks for helping out! I have some comments though at the google code
website at issue 154.

On Tue, Jul 2, 2013 at 3:17 PM, Zlika notifications@github.com wrote:

Tentative fix for #77 and #154. The default behaviour ("Accept-Encoding:
gzip,deflate") is kept to avoid breaking backward compatibility.
The only problem I see with this fix is that it uses an internal type
(com.jayway.restassured.internal.http.ContentEncoding) in the public API of
RequestSpecification, but this problem already exists for ObjectMapperType.
Maybe this type could be moved outside the internal package ? (I would like
to avoid creating a new public "ContentEncoding" that would be a mere

copy-paste of the original one)

You can merge this Pull Request by running

git pull https://github.com/Zlika/rest-assured master

Or view, comment on, or merge it at:

#17
Commit Summary

File Changes

Patch Links:

… This configuration can be done globally (with RestAssuredConfig.newConfig().acceptEncodingConfig(...)) or locally (given().acceptEncoding(AcceptEncodingConfig.XXX)...).

Default value is "gzip,deflate".
@buildhive
Copy link

Jayway » rest-assured #79 SUCCESS
This pull request looks good
(what's this?)

@Zlika
Copy link
Author

Zlika commented Jul 3, 2013

By the way, I saw that there were very few unit tests on rest-assured, and nothing is setup in the Maven build to allow "real world" REST tests. On one of my projects I use Arquillian to automate REST tests (with rest-assured). It could be easily feasible to write a small JEE REST application and use Arquillian to automate rest-assured tests. If you are interested I can propose a pull request on the subject when I have enough time.

johanhaleby added a commit that referenced this pull request Jul 4, 2013
@johanhaleby johanhaleby merged commit 0cfa770 into rest-assured:master Jul 4, 2013
@johanhaleby
Copy link
Collaborator

Thanks a lot for helping out! I've accepted that patch but I'll probably update it a little bit.

There are actually quite a few integration tests for Rest Assured, they're located in the examples/rest-assured-itest-java project! :)

@Zlika
Copy link
Author

Zlika commented Jul 4, 2013

Oups, if I had seen these IT tests I would have added others for this new feature instead of using the test suite of my project at work.
Feel free to update the patch, I will not be upset :-)

@karthik008
Copy link

Hi I am still getting error stating that "java.util.zip.ZipException: incorrect header check",Iam using Rest assured 3.0

Response r = io.restassured.RestAssured.given().headers("content-type","application/json").headers("Accept","application/json").contentType(ContentType.JSON).body(updateBody).when().post("http://Common.domain.com/api/api/TestItem/678566/3.3.90.57/test/").then().extract().response();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants