-
-
Notifications
You must be signed in to change notification settings - Fork 610
[IntegrationTest] Use 127.0.0.1
as target address and a random port
#756
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,3 +5,9 @@ scm | |
Threadless | ||
threadless | ||
youtube | ||
socio | ||
sexualized | ||
https | ||
www | ||
html | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will probably be unnecessary if you fix the document to have an explicit link by wrapping that link with angled brackets. |
||
faq |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,9 +5,13 @@ | |
|
||
import pytest | ||
|
||
from proxy.common.utils import get_available_port | ||
from proxy.common._compat import IS_WINDOWS # noqa: WPS436 | ||
|
||
|
||
PROXY_PY_PORT = get_available_port() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's best to use an ephemeral port. Otherwise, it's still possible to hit race conditions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep I would have, but situation here is a little different. Proxy is not in running in embedded mode but started using command line. So we must then either parse port from logs or let proxy write the port somewhere or use pid file to infer the port :). I went for the easy solution for now. |
||
|
||
|
||
# FIXME: Ignore is necessary for as long as pytest hasn't figured out | ||
# FIXME: typing for their fixtures. | ||
# Refs: | ||
|
@@ -22,6 +26,7 @@ def _proxy_py_instance() -> Generator[None, None, None]: | |
proxy_cmd = ( | ||
'proxy', | ||
'--hostname', '127.0.0.1', | ||
'--port', str(PROXY_PY_PORT), | ||
'--enable-web-server', | ||
) | ||
proxy_proc = Popen(proxy_cmd) | ||
|
@@ -49,4 +54,4 @@ def test_curl() -> None: | |
this_test_module = Path(__file__) | ||
shell_script_test = this_test_module.with_suffix('.sh') | ||
|
||
check_output(str(shell_script_test)) | ||
check_output([str(shell_script_test), str(PROXY_PY_PORT)]) |
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.
Maybe add these to the document and not globally?
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.
Totally, I added these because CI was failing and making me nervous, haha. Let me address them properly as a follow up PR. I have a few bits more related to doc. Will pull in as part of that.