Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Features
via an optional ``conf.py`` method ``TEMPLATE_ENGINE_FACTORY``.
* Switch to pyproject.toml
* Add path handler ``slug_source`` linking to source of post.
* Better error message in ``nikola serve`` when port is already in use

Bugfixes
--------
Expand Down
12 changes: 11 additions & 1 deletion nikola/plugins/command/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

"""Start test server."""
import atexit
import errno
import os
import sys
import re
Expand Down Expand Up @@ -144,7 +145,16 @@ def _execute(self, options, args):
handler_factory = OurHTTPRequestHandler
else:
handler_factory = _create_RequestHandler_removing_basepath(base_path)
self.httpd = OurHTTP((options['address'], options['port']), handler_factory)

try:
self.httpd = OurHTTP((options['address'], options['port']), handler_factory)
except OSError as e:
if e.errno == errno.EADDRINUSE:
self.logger.error(f"Port address {options['port']} already in use, please use "
"the `-p <port>` option to select a different one.")
return 3
else:
raise

sa = self.httpd.socket.getsockname()
if ipv6:
Expand Down
1 change: 1 addition & 0 deletions tests/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def __init__(self):
self.template_system = self
self.invariant = False
self.debug = True
self.show_tracebacks = True
self.config = {
"DISABLED_PLUGINS": [],
"EXTRA_PLUGINS": [],
Expand Down
62 changes: 62 additions & 0 deletions tests/integration/test_dev_server_serve.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
import logging
import re
import socket
import sys
from io import StringIO
from time import sleep
import requests
import pytest
Expand All @@ -8,11 +13,68 @@
from .dev_server_test_helper import MyFakeSite, SERVER_ADDRESS, find_unused_port, LOGGER, OUTPUT_FOLDER


def test_server_on_used_port(site_and_base_path: tuple[MyFakeSite, str]) -> None:
"""Check error if port for nikola serve is already being used.

`nikola serve` uses a default port and if that port is already in use it should print out a nice
error message that tells the user what happend and how to fix this.

To test the case where the port is already in use, we open a socket on the same port that we use
for `nikola serve` before starting the server.

The program should exit with a return code of 3 in this case and print out a message to the user.
"""

site, base_path = site_and_base_path
site.show_tracebacks = False
command_serve = serve.CommandServe()
command_serve.set_site(site)
command_serve.serve_pidfile = "there is no file with this name we hope"
command_serve.logger = logging.getLogger("dev_server_test")
catch_log = StringIO()
catch_log_handler = logging.StreamHandler(catch_log)
logging.getLogger().addHandler(catch_log_handler)
try:
s = socket.socket()
try:
ANY_PORT = 0
s.bind((SERVER_ADDRESS, ANY_PORT))
address, port = s.getsockname()
with ThreadPoolExecutor(max_workers=2) as executor:
options = {
"address": SERVER_ADDRESS,
"port": port,
"browser": False,
"detach": False,
"ipv6": False,
}
future_to_run_web_server = executor.submit(lambda: command_serve.execute(options=options))
command_serve.shutdown()
result = future_to_run_web_server.result()
assert 3 == result

# TODO: check if this works on windows
# for now we skip this assert on windows platforms.
if not sys.platform == 'win32':

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Imagine someone like you stumbles over this in a year or two. This line raises an eyebrow and leaves the puzzling question to be answered: "Why?"

Can you please add a comment here that'll help that someone?

(We need our Nikola community members and want to make life nice for them!)

@aknrdureegaesr aknrdureegaesr Mar 6, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We now see the original Windows exception. We would like to see the result being 3. I think we'll get that by setting show_tracebacks to False in MyFakeSite.

It should be True in general, which is what FakeSite now has, but we need False for this particular test, with MyFakeSite being the convenient place to get that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok, so I would set site.show_tracebacks=False inside the test (say right at the top of the test). Is this only needed for windows or for all tests?

I can add some comments later today and can also test this on linux and mac.

assert re.match(
r"Port address \d+ already in use, "
r"please use the `\-p \<port\>` option to select a different one\.",
catch_log.getvalue()
)
assert "OSError" not in catch_log.getvalue()
finally:
s.close()
finally:
logging.getLogger().removeHandler(catch_log_handler)


def test_serves_root_dir(
site_and_base_path: tuple[MyFakeSite, str], expected_text: str
) -> None:
site, base_path = site_and_base_path
command_serve = serve.CommandServe()
command_serve.serve_pidfile = "there is no file with this name we hope"
command_serve.logger = logging.getLogger("dev_server_test")
command_serve.set_site(site)
options = {
"address": SERVER_ADDRESS,
Expand Down
Loading