-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Fix and re-enable newer mitmproxy usage in tests. #6201
Conversation
@@ -46,7 +47,7 @@ def start(self): | |||
stdout=PIPE, | |||
) | |||
line = self.proc.stdout.readline().decode("utf-8") | |||
host_port = re.search(r"listening at http://([^:]+:\d+)", line).group(1) | |||
host_port = re.search(r"listening at (?:http://)?([^:]+:\d+)", line).group(1) |
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.
The output format has changed in some newer version but we can support both.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #6201 +/- ##
==========================================
- Coverage 88.63% 88.61% -0.02%
==========================================
Files 160 160
Lines 11615 11615
Branches 1890 1890
==========================================
- Hits 10295 10293 -2
- Misses 994 995 +1
- Partials 326 327 +1 |
@@ -31,6 +31,7 @@ def start(self): | |||
self.proc = Popen( | |||
[ | |||
sys.executable, | |||
"-u", |
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.
mitmproxy changed the code that prints log lines and we now need to disable stdout buffering to get it via Popen.
#mitmproxy >= 5.3.0; python_version >= '3.9' and implementation_name != 'pypy' | ||
# The tests hang with mitmproxy 8.0.0: https://github.com/scrapy/scrapy/issues/5454 | ||
mitmproxy >= 4.0.4, < 8; python_version < '3.9' and implementation_name != 'pypy' | ||
mitmproxy; implementation_name != 'pypy' |
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.
is there a minimal mitmproxy version, like >8
?
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.
The code should work with any mitmproxy version.
As usual, a two-character fix.
Fixes #5454.