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

proxy request fix #1765

Merged
merged 3 commits into from
Apr 6, 2022
Merged

proxy request fix #1765

merged 3 commits into from
Apr 6, 2022

Conversation

AntonDyukarev
Copy link
Contributor

fix request entity setting for any request methods

fix request entity setting for any request methods
@AntonDyukarev AntonDyukarev changed the title remove addBodyIfPostPutOrPatch method proxy request fix Jan 11, 2022
@kermek
Copy link

kermek commented Feb 9, 2022

@tomakehurst could you please review this PR, this issue affects our tests.

@tomakehurst
Copy link
Member

@AntonDyukarev please could you add a test case?

@AntonDyukarev
Copy link
Contributor Author

@tomakehurst i'll try=)
@kermek can you list some broken tests please

@tomakehurst
Copy link
Member

I think we just need some tests for HTTP methods other than PUT, POST, PATCH - I'd suggest going for GET and DELETE.

@AntonDyukarev
Copy link
Contributor Author

AntonDyukarev commented Feb 13, 2022

@tomakehurst hi! I was added tests for get, post, put, delete and patch methods. But locally i has problem with keystorePath. The problem is X509CertificateVersion enum wich always is NULL, and i don't know why. Because of that locally tests failed.

But without keystorePath, all tests passed (look at second picture). I don't think that it affects of my changes about proxy.

Also for compare i ran same tests on master branch, and got fails with get and delete methods tests(look at third picture).

image
image
image

@kermek
Copy link

kermek commented Mar 16, 2022

@tomakehurst @AntonDyukarev any news on this one?

@AntonDyukarev
Copy link
Contributor Author

@tomakehurst @AntonDyukarev any news on this one?

Hi! I waiting for your answers about my tests

@tomakehurst
Copy link
Member

@AntonDyukarev can you push your tests so I can take a look at the issue you encountered?

@AntonDyukarev
Copy link
Contributor Author

@tomakehurst They already pushed

@kermek
Copy link

kermek commented Mar 25, 2022

@tomakehurst any news on this one?

@kermek
Copy link

kermek commented Apr 1, 2022

@tomakehurst could you please look at this PR?

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

3 participants