-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
gh-131178: Add tests for http.server command-line interface #132540
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
base: main
Are you sure you want to change the base?
Conversation
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
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.
I haven't read everything carefully yet, but this is what catches my eye first
Misc/NEWS.d/next/Library/2025-04-15-12-47-09.gh-issue-131178.Td8j5x.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Semyon Moroz <[email protected]>
Co-authored-by: Semyon Moroz <[email protected]>
Co-authored-by: Semyon Moroz <[email protected]>
Co-authored-by: Semyon Moroz <[email protected]>
Co-authored-by: Semyon Moroz <[email protected]>
Co-authored-by: Semyon Moroz <[email protected]>
Co-authored-by: Semyon Moroz <[email protected]>
Co-authored-by: Semyon Moroz <[email protected]>
Co-authored-by: Semyon Moroz <[email protected]>
Should we remove the |
I don't think we should remove it until PR #133811 is merged. |
You can remove them. I don't think it's worth for a test to stay if I'm going to remove it just after. However, you shouldn't remove it from the runtime (namely, don't change the current behavior, you just don't need to bother with creating the tests). This will be part of my PR. We didn't backport the additional tests, so in 3.15, there shouldn't be code using CGI. |
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
proc = spawn_python('-u', '-m', 'http.server', str(port), '-b', bind, | ||
'--tls-cert', self.tls_cert, | ||
'--tls-key', self.tls_key, | ||
'--tls-password-file', self.tls_password_file, | ||
bufsize=1, text=True) | ||
self.assertTrue(self.wait_for_server(proc, 'https', port, bind)) | ||
res = self.fetch_file(f'https://{bind}:{port}/{self.random_file_name}') | ||
self.assertEqual(res, self.random_data) | ||
proc.terminate() | ||
kill_python(proc) |
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.
proc = spawn_python('-u', '-m', 'http.server', str(port), '-b', bind, | |
'--tls-cert', self.tls_cert, | |
'--tls-key', self.tls_key, | |
'--tls-password-file', self.tls_password_file, | |
bufsize=1, text=True) | |
self.assertTrue(self.wait_for_server(proc, 'https', port, bind)) | |
res = self.fetch_file(f'https://{bind}:{port}/{self.random_file_name}') | |
self.assertEqual(res, self.random_data) | |
proc.terminate() | |
kill_python(proc) | |
proc = spawn_python('-u', '-m', 'http.server', str(port), '-b', bind, | |
'--tls-cert', self.tls_cert, | |
'--tls-key', self.tls_key, | |
'--tls-password-file', self.tls_password_file, | |
bufsize=1, text=True) | |
self.addCleanup(proc.terminate) | |
self.addCleanup(kill_python, proc) | |
self.assertTrue(self.wait_for_server(proc, 'https', port, bind)) | |
res = self.fetch_file(f'https://{bind}:{port}/{self.random_file_name}') | |
self.assertEqual(res, self.random_data) |
Hopefully, it will first call proc.terminate before calling kill_python(proc).
res = self.fetch_file(f'http://{bind}:{port}/{self.random_file_name}') | ||
self.assertEqual(res, self.random_data) | ||
proc.terminate() | ||
kill_python(proc) |
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.
Ditto
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
with self.assertRaises(SystemExit): | ||
_ = self.invoke_httpd(option) | ||
|
||
@mock.patch('http.server.test') | ||
def test_unknown_flag(self, _): | ||
with self.assertRaises(SystemExit): | ||
_ = self.invoke_httpd('--unknown-flag') | ||
|
||
class CommandLineRunTimeTestCase(unittest.TestCase): |
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.
Let's capture the output outside invoke_httpd
as well:
def invoke_httpd(self, *args, stdout=None, stderr=None):
stdout = StringIO() if stdout is None else stdout
stderr = StringIO() if stderr is None else stderr
with contextlib.redirect_stdout(stdout), \
contextlib.redirect_stderr(stderr):
server._main(args)
return {'stdout': stdout.getvalue(), 'stderr': stderr.getvalue()}
Then do:
stdout, stderr = StringIO(), StringIO()
with self.assertRaises(SystemExit):
_ = self.invoke_httpd('--unknown-flag', stdout=stdout, stderr=stderr)
self.assertEqual(stdout.getvalue(), "")
self.assertIn("error: ", stderr.getvalue())
#131178