-
Notifications
You must be signed in to change notification settings - Fork 448
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
Improve non-root site handling of nikola auto. Fix 3715. #3727
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,9 +3,10 @@ | |
from nikola.utils import get_logger | ||
import pytest | ||
import pathlib | ||
import socket | ||
from typing import Optional | ||
import requests | ||
import socket | ||
import sys | ||
from typing import Optional, Tuple, Any, Dict | ||
|
||
from ..helper import FakeSite | ||
|
||
|
@@ -35,7 +36,21 @@ def find_unused_port() -> int: | |
s.close() | ||
|
||
|
||
def test_serves_root_dir(site, expected_text) -> None: | ||
class MyFakeSite(FakeSite): | ||
def __init__(self, config: Dict[str, Any], configuration_filename="conf.py"): | ||
self.configured = True | ||
self.debug = True | ||
self.THEMES = [] | ||
self._plugin_places = [] | ||
self.registered_auto_watched_folders = set() | ||
self.config = config | ||
self.configuration_filename = configuration_filename | ||
|
||
|
||
def test_serves_root_dir( | ||
site_and_base_path: Tuple[MyFakeSite, str], expected_text: str | ||
) -> None: | ||
site, base_path = site_and_base_path | ||
command_auto = auto.CommandAuto() | ||
command_auto.set_site(site) | ||
options = { | ||
|
@@ -55,75 +70,108 @@ def test_serves_root_dir(site, expected_text) -> None: | |
test_was_successful = False | ||
test_problem_description = "Async test setup apparently broken" | ||
test_inner_error: Optional[BaseException] = None | ||
loop_for_this_test = None | ||
|
||
async def grab_loop_and_run_test() -> None: | ||
loop = asyncio.get_running_loop() | ||
watchdog_handle = loop.call_later(TEST_MAX_DURATION, lambda: loop.stop()) | ||
nonlocal test_problem_description | ||
nonlocal test_problem_description, loop_for_this_test | ||
|
||
loop_for_this_test = asyncio.get_running_loop() | ||
watchdog_handle = loop_for_this_test.call_later(TEST_MAX_DURATION, lambda: loop_for_this_test.stop()) | ||
test_problem_description = f"Test did not complete within {TEST_MAX_DURATION} seconds." | ||
|
||
def run_test() -> None: | ||
nonlocal test_was_successful, test_problem_description, test_inner_error | ||
try: | ||
with requests.Session() as session: | ||
server_root_uri = f"http://{options['address']}:{options['port']}" | ||
|
||
# First subtest: Grab the document root index.html file: | ||
server_base_uri = f"http://{options['address']}:{options['port']}/" | ||
server_base_uri = f"{server_root_uri}{base_path}" | ||
LOGGER.info("Attempting to fetch HTML from %s", server_base_uri) | ||
res = session.get(server_base_uri) | ||
res.raise_for_status() | ||
assert "text/html; charset=utf-8" == res.headers['content-type'] | ||
assert expected_text in res.text | ||
|
||
# Second subtest: Does the dev server serve something for the livereload JS? | ||
res_js = session.get(f"{server_base_uri}livereload.js?snipver=1") | ||
js_uri = f"{server_root_uri}/livereload.js?snipver=1" | ||
LOGGER.info("Attempting to fetch JS from %s", js_uri) | ||
res_js = session.get(js_uri) | ||
res_js.raise_for_status() | ||
content_type_js = res_js.headers['content-type'] | ||
assert "javascript" in content_type_js | ||
|
||
test_was_successful = True | ||
test_problem_description = "No problem. All is well." | ||
except BaseException as be: | ||
LOGGER.error("Could not receive HTTP as expected.", exc_info=True) | ||
test_inner_error = be | ||
test_was_successful = False | ||
test_problem_description = "(see exception)" | ||
finally: | ||
LOGGER.info("Test completed, %s: %s", | ||
"successfully" if test_was_successful else "failing", | ||
test_problem_description) | ||
loop.call_soon_threadsafe(lambda: watchdog_handle.cancel()) | ||
if test_was_successful: | ||
LOGGER.info("Test completed successfully.") | ||
else: | ||
LOGGER.error("Test failed: %s", test_problem_description) | ||
loop_for_this_test.call_soon_threadsafe(lambda: watchdog_handle.cancel()) | ||
|
||
# We give the outer grab_loop_and_run_test a chance to complete | ||
# before burning the bridge: | ||
loop.call_soon_threadsafe(lambda: loop.call_later(0.05, lambda: loop.stop())) | ||
await loop.run_in_executor(None, run_test) | ||
loop_for_this_test.call_soon_threadsafe(lambda: loop_for_this_test.call_later(0.05, lambda: loop_for_this_test.stop())) | ||
|
||
await loop_for_this_test.run_in_executor(None, run_test) | ||
|
||
# We defeat the nikola site building functionality, so this does not actually get called. | ||
# But the setup code sets this up and needs a command list for that. | ||
# But the code setting up site building wants a command list: | ||
command_auto.nikola_cmd = ["echo"] | ||
|
||
# Defeat the site building functionality, and instead insert the test: | ||
command_auto.run_initial_rebuild = grab_loop_and_run_test | ||
|
||
# Start the development server | ||
# which under the hood runs our test when trying to build the site: | ||
command_auto.execute(options=options) | ||
|
||
# Verify the test succeeded: | ||
if test_inner_error is not None: | ||
raise test_inner_error | ||
assert test_was_successful, test_problem_description | ||
|
||
|
||
class MyFakeSite(FakeSite): | ||
def __init__(self, config, configuration_filename): | ||
self.configured = True | ||
self.debug = True | ||
self.THEMES = [] | ||
self._plugin_places = [] | ||
self.registered_auto_watched_folders = set() | ||
self.config = config | ||
self.configuration_filename = configuration_filename | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
def site() -> MyFakeSite: | ||
try: | ||
# Start the development server | ||
# which under the hood runs our test when trying to build the site: | ||
command_auto.execute(options=options) | ||
|
||
# Verify the test succeeded: | ||
if test_inner_error is not None: | ||
raise test_inner_error | ||
assert test_was_successful, test_problem_description | ||
finally: | ||
# Nikola is written with the assumption that it can | ||
# create the event loop at will without ever cleaning it up. | ||
# As this tests runs several times in succession, | ||
# that assumption becomes a problem. | ||
LOGGER.info("Cleaning up loop.") | ||
# Loop cleanup: | ||
assert loop_for_this_test is not None | ||
assert not loop_for_this_test.is_running() | ||
loop_for_this_test.close() | ||
asyncio.set_event_loop(None) | ||
# We would like to leave it at that, | ||
# but doing so causes the next test to fail. | ||
# | ||
# We did not find asyncio - API to reset the loop | ||
# to "back to square one, as if just freshly started". | ||
# | ||
# The following code does not feel right, it's a kludge, | ||
# but it apparently works for now: | ||
if sys.platform == 'win32': | ||
# For this case, the auto module has special code | ||
# (at module load time! 😟) which we reluctantly reproduce here: | ||
asyncio.set_event_loop(asyncio.ProactorEventLoop()) | ||
else: | ||
asyncio.set_event_loop(asyncio.new_event_loop()) | ||
|
||
|
||
@pytest.fixture(scope="module", | ||
params=["https://example.org", | ||
"https://example.org:1234/blog", | ||
"https://example.org:3456/blog/", | ||
"http://example.org/deep/down/a/rabbit/hole" | ||
]) | ||
def site_and_base_path(request) -> Tuple[MyFakeSite, str]: | ||
"""Return a fake site and the base_path (root) the dev server should be serving.""" | ||
assert OUTPUT_FOLDER.is_dir(), \ | ||
f"Could not find dev server test fixture {OUTPUT_FOLDER.as_posix()}" | ||
|
||
|
@@ -133,9 +181,10 @@ def site() -> MyFakeSite: | |
"GALLERY_FOLDERS": [], | ||
"LISTINGS_FOLDERS": [], | ||
"IMAGE_FOLDERS": [], | ||
"SITE_URL": request.param, | ||
"OUTPUT_FOLDER": OUTPUT_FOLDER.as_posix(), | ||
} | ||
return MyFakeSite(config, "conf.py") | ||
return (MyFakeSite(config), auto.base_path_from_siteuri(request.param)) | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
|
@@ -148,3 +197,27 @@ def expected_text(): | |
with open(OUTPUT_FOLDER / "index.html", encoding="utf-8") as html_file: | ||
all_html = html_file.read() | ||
return all_html[all_html.find("<body>"):] | ||
|
||
|
||
@pytest.mark.parametrize(("uri", "expected_basepath"), [ | ||
("http://localhost", ""), | ||
("http://local.host", ""), | ||
("http://localhost/", ""), | ||
("http://local.host/", ""), | ||
("http://localhost:123/", ""), | ||
("http://local.host:456/", ""), | ||
("https://localhost", ""), | ||
("https://local.host", ""), | ||
("https://localhost/", ""), | ||
("https://local.host/", ""), | ||
("https://localhost:123/", ""), | ||
("https://local.host:456/", ""), | ||
("http://example.org/blog", "/blog"), | ||
("https://lorem.ipsum/dolet/", "/dolet"), | ||
("http://example.org:124/blog", "/blog"), | ||
("http://example.org:124/Deep/Rab_bit/hol.e/", "/Deep/Rab_bit/hol.e"), | ||
# Would anybody in a sane mind actually do this? | ||
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. No, I doubt it would work in any reasonable way. |
||
("http://example.org:124/blog?lorem=ipsum&dol=et", "/blog"), | ||
]) | ||
def test_basepath(uri: str, expected_basepath: Optional[str]) -> None: | ||
assert expected_basepath == auto.base_path_from_siteuri(uri) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This was required to work around a bug in asyncio, IIRC it prevented shutting down the server with ^C. You may move it to some later point than loading the module.