gh-131178: Add tests for http.server command-line interface #132540
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
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 |
ggqlq
changed the title
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 <donbarbos@proton.me>
Co-authored-by: Semyon Moroz <donbarbos@proton.me>
Co-authored-by: Semyon Moroz <donbarbos@proton.me>
Co-authored-by: Semyon Moroz <donbarbos@proton.me>
Co-authored-by: Semyon Moroz <donbarbos@proton.me>
Co-authored-by: Semyon Moroz <donbarbos@proton.me>
Co-authored-by: Semyon Moroz <donbarbos@proton.me>
Co-authored-by: Semyon Moroz <donbarbos@proton.me>
Co-authored-by: Semyon Moroz <donbarbos@proton.me>
Co-authored-by: Semyon Moroz <donbarbos@proton.me>
|
you don't have to request for a review, i still get notifications after your changes :) and it seems that News entry is not needed here. we have a devguide (very useful to read) and it also lists when it is worth writing: https://devguide.python.org/core-developers/committing/#updating-news-and-what-s-new-in-python |
|
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 |
| self.default_port = 8000 | ||
| self.default_bind = None | ||
| self.default_protocol = 'HTTP/1.0' | ||
| self.default_directory = os.getcwd() |
There was a problem hiding this comment.
unused var
| self.default_directory = os.getcwd() |
| def text_normalizer(self, string): | ||
| return textwrap.dedent(string).strip() | ||
|
|
||
| def invoke_httpd(self, args=[]): | ||
| output = StringIO() | ||
| with contextlib.redirect_stdout(output): | ||
| server._main(args) | ||
| return self.text_normalizer(output.getvalue()) |
There was a problem hiding this comment.
text_normalizerfunction is only used once, why not get rid of it- mutable variable as a default argument is bad practice
| def text_normalizer(self, string): | |
| return textwrap.dedent(string).strip() | |
| def invoke_httpd(self, args=[]): | |
| output = StringIO() | |
| with contextlib.redirect_stdout(output): | |
| server._main(args) | |
| return self.text_normalizer(output.getvalue()) | |
| def invoke_httpd(self, *args): | |
| output = StringIO() | |
| with contextlib.redirect_stdout(output): | |
| server._main(args) | |
| return textwrap.dedent(output.getvalue()).strip() |
| for option in options: | ||
| with self.assertRaises(SystemExit): | ||
| output = self.invoke_httpd([option]) | ||
| self.assertIn('usage:', output) |
There was a problem hiding this comment.
I'd prefer next method:
| self.assertIn('usage:', output) | |
| self.assertStartsWith(output, 'usage: ') |
|
|
||
| @unittest.skipIf(ssl is None, "requires ssl") | ||
| @mock.patch('http.server.test') | ||
| def test_tls_flag(self, mock_func): |
There was a problem hiding this comment.
the output of the result is directed outwards, need to catch it.
my actual output for python ./Lib/test/test_httpservers.py:
...............................................................usage: test_httpservers.py [-h] [--cgi] [-b ADDRESS] [-d DIRECTORY] [-p VERSION] [--tls-cert PATH] [--tls-key PATH]
[--tls-password-file PATH]
[port]
test_httpservers.py: error: --tls-key requires --tls-cert to be set
usage: test_httpservers.py [-h] [--cgi] [-b ADDRESS] [-d DIRECTORY] [-p VERSION] [--tls-cert PATH] [--tls-key PATH]
[--tls-password-file PATH]
[port]
test_httpservers.py: error: --tls-password-file requires --tls-cert to be set
usage: test_httpservers.py [-h] [--cgi] [-b ADDRESS] [-d DIRECTORY] [-p VERSION] [--tls-cert PATH] [--tls-key PATH]
[--tls-password-file PATH]
[port]
test_httpservers.py: error: Failed to read TLS password file: [Errno 2] No such file or directory: '/tmp/98555ec389358504f26d72f0915689fe'
.usage: test_httpservers.py [-h] [--cgi] [-b ADDRESS] [-d DIRECTORY] [-p VERSION] [--tls-cert PATH] [--tls-key PATH]
[--tls-password-file PATH]
[port]
test_httpservers.py: error: unrecognized arguments: --unknown-flag
........................
----------------------------------------------------------------------
Ran 88 tests in 2.865s
OK
| for port in ports: | ||
| with self.subTest(port=port): | ||
| self.invoke_httpd([str(port)]) | ||
| mock_func.assert_called_once_with(HandlerClass=self.default_handler, ServerClass=self.default_server, |
There was a problem hiding this comment.
Could you wrap similar lines to fit into 79 characters?
I didn't mark all such lines, but I think you can easily find them yourself :)
and if possible, try not to pass optional arguments
| ports = [8000, 65535,] | ||
| for port in ports: | ||
| with self.subTest(port=port): | ||
| self.invoke_httpd([str(port)]) |
There was a problem hiding this comment.
Could you tell me how to ensure that the server does not continue to work?
Sorry in advance, I just couldn't find such logic
#131178