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

httplib with OpenSSL mock support #215

Closed
wants to merge 11 commits into from

Conversation

andresriancho
Copy link
Contributor

Done:

  • Added httplib with OpenSSL mock support
  • Tested with test_openssl.py (all PASS)

Running all functional tests (at least on my workstation) fails with some "port already in use" errors. What's that about? Any ideas on how to fix that? (it was a problem even before this patch)

I'm sending this PR with two main objectives:
1- Get travis to run all tests (maybe there we don't get those errors)
2- Get an initial review of the code/changes

@andresriancho
Copy link
Contributor Author

Aha! Now these builds tell me something I didn't know :)

Will fix in a while

@andresriancho
Copy link
Contributor Author

@gabrielfalcao there are two things to notice about the builds:

@lithiumFlower
Copy link

Any chance this will get merged?

Conflicts:
	test-requirements.txt
@gabrielfalcao
Copy link
Owner

Hi!
I'm going through all the open PR's in HTTPretty and merging anything that fulfills these conditions:

  • Travis CI passing
  • Have test coverage

Your PR has test coverage but the travis build is breaking, I'd love to merge this as soon as the travis build is passing.

Thoughts?

@va3093
Copy link

va3093 commented Nov 25, 2016

@andresriancho This implementation won't work for python 3 since socket._fileobject is removed. This PR may be helpful in fixing that https://github.com/pallets/werkzeug/pull/565/files

@@ -468,7 +490,28 @@ def fake_wrap_socket(s, *args, **kw):
return s


def create_fake_connection(address, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, source_address=None):
def fake_fileobject(s, *args, **kwargs):
Copy link
Owner

Choose a reason for hiding this comment

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

this is nice

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.

4 participants