WebHost: Fix generate argparse with --config-override + add autogen unit tests so we can test that (#5541)

* Fix webhost argparse with extra args

* accidentally added line

* WebHost: fix some typing

B64 url conversion is used in test/hosting,
so it felt appropriate to include this here.

* Test: Hosting: also test autogen

* Test: Hosting: simplify stop_* and leave a note about Windows compat

* Test: Hosting: fix formatting error

* Test: Hosting: add limitted Windows support

There are actually some differences with MP on Windows
that make it impossible to run this in CI.

---------

Co-authored-by: black-sliver <59490463+black-sliver@users.noreply.github.com>

Generate.py

@@ -23,7 +23,7 @@ from BaseClasses import seeddigits, get_seed, PlandoOptions
 from Utils import parse_yamls, version_tuple, __version__, tuplize_version
 
 
-def mystery_argparse():
+def mystery_argparse(argv: list[str] | None = None):
     from settings import get_settings
     settings = get_settings()
     defaults = settings.generator
@@ -57,7 +57,7 @@ def mystery_argparse():
     parser.add_argument("--spoiler_only", action="store_true",
                         help="Skips generation assertion and multidata, outputting only a spoiler log. "
                              "Intended for debugging and testing purposes.")
-    args = parser.parse_args()
+    args = parser.parse_args(argv)
 
     if args.skip_output and args.spoiler_only:
         parser.error("Cannot mix --skip_output and --spoiler_only")

WebHostLib/__init__.py

@@ -1,6 +1,7 @@
 import base64
 import os
 import socket
+import typing
 import uuid
 
 from flask import Flask
@@ -61,20 +62,21 @@ cache = Cache()
 Compress(app)
 
 
-def to_python(value):
+def to_python(value: str) -> uuid.UUID:
     return uuid.UUID(bytes=base64.urlsafe_b64decode(value + '=='))
 
 
-def to_url(value):
+def to_url(value: uuid.UUID) -> str:
     return base64.urlsafe_b64encode(value.bytes).rstrip(b'=').decode('ascii')
 
 
 class B64UUIDConverter(BaseConverter):
 
-    def to_python(self, value):
+    def to_python(self, value: str) -> uuid.UUID:
         return to_python(value)
 
-    def to_url(self, value):
+    def to_url(self, value: typing.Any) -> str:
+        assert isinstance(value, uuid.UUID)
         return to_url(value)
 
 
@@ -84,7 +86,7 @@ app.jinja_env.filters["suuid"] = to_url
 app.jinja_env.filters["title_sorted"] = title_sorted
 
 
-def register():
+def register() -> None:
     """Import submodules, triggering their registering on flask routing.
     Note: initializes worlds subsystem."""
     import importlib

WebHostLib/autolauncher.py

@@ -17,7 +17,7 @@ from .locker import Locker, AlreadyRunningException
 _stop_event = Event()
 
 
-def stop():
+def stop() -> None:
     """Stops previously launched threads"""
     global _stop_event
     stop_event = _stop_event

WebHostLib/generate.py

@@ -137,7 +137,7 @@ def gen_game(gen_options: dict, meta: dict[str, Any] | None = None, owner=None,
 
         seedname = "W" + (f"{random.randint(0, pow(10, seeddigits) - 1)}".zfill(seeddigits))
 
-        args = mystery_argparse()
+        args = mystery_argparse([])  # Just to set up the Namespace with defaults
         args.multi = playercount
         args.seed = seed
         args.name = {x: "" for x in range(1, playercount + 1)}  # only so it can be overwritten in mystery

test/hosting/__main__.py

@@ -3,6 +3,7 @@
 # Run with `python test/hosting` instead,
 import logging
 import traceback
+from pathlib import Path
 from tempfile import TemporaryDirectory
 from time import sleep
 from typing import Any
@@ -11,7 +12,7 @@ from test.hosting.client import Client
 from test.hosting.generate import generate_local
 from test.hosting.serve import ServeGame, LocalServeGame, WebHostServeGame
 from test.hosting.webhost import (create_room, get_app, get_multidata_for_room, set_multidata_for_room, start_room,
-                                  stop_autohost, upload_multidata)
+                                  stop_autogen, stop_autohost, upload_multidata, generate_remote)
 from test.hosting.world import copy as copy_world, delete as delete_world
 
 failure = False
@@ -56,35 +57,62 @@ else:
 
 
 if __name__ == "__main__":
+    import sys
     import warnings
+
     warnings.simplefilter("ignore", ResourceWarning)
     warnings.simplefilter("ignore", UserWarning)
+    warnings.simplefilter("ignore", DeprecationWarning)
 
     spacer = '=' * 80
 
     with TemporaryDirectory() as tempdir:
+        empty_file = str(Path(tempdir) / "empty")
+        open(empty_file, "w").close()
+        sys.argv += ["--config_override", empty_file]  # tests #5541
         multis = [["VVVVVV"], ["Temp World"], ["VVVVVV", "Temp World"]]
-        p1_games = []
+        p1_games: list[str] = []
-        data_paths = []
+        data_paths: list[Path | None] = []
-        rooms = []
+        rooms: list[str] = []
+        multidata: Path | None
 
         copy_world("VVVVVV", "Temp World")
         try:
             for n, games in enumerate(multis, 1):
-                print(f"Generating [{n}] {', '.join(games)}")
+                print(f"Generating [{n}] {', '.join(games)} offline")
                 multidata = generate_local(games, tempdir)
                 print(f"Generated [{n}] {', '.join(games)} as {multidata}\n")
-                p1_games.append(games[0])
                 data_paths.append(multidata)
+                p1_games.append(games[0])
         finally:
             delete_world("Temp World")
 
         webapp = get_app(tempdir)
         webhost_client = webapp.test_client()
+
         for n, multidata in enumerate(data_paths, 1):
+            assert multidata
             seed = upload_multidata(webhost_client, multidata)
+            print(f"Uploaded [{n}] {multidata} as {seed}\n")
             room = create_room(webhost_client, seed)
-            print(f"Uploaded [{n}] {multidata} as {room}\n")
+            print(f"Started [{n}] {seed} as {room}\n")
+            rooms.append(room)
+
+        # Generate 1 extra game on WebHost
+        from WebHostLib.autolauncher import autogen
+        for n, games in enumerate(multis[:1], len(multis) + 1):
+            multis.append(games)
+            try:
+                print(f"Generating [{n}] {', '.join(games)} online")
+                autogen(webapp.config)
+                sleep(5)  # until we have lazy loading of worlds, wait here for the process to start up
+                seed = generate_remote(webhost_client, games)
+                print(f"Generated [{n}] {', '.join(games)} as {seed}\n")
+            finally:
+                stop_autogen()
+            data_paths.append(None)  # WebHost-only
+            room = create_room(webhost_client, seed)
+            print(f"Started [{n}] {seed} as {room}\n")
             rooms.append(room)
 
         print("Starting autohost")
@@ -96,31 +124,10 @@ if __name__ == "__main__":
             for n, (multidata, room, game, multi_games) in enumerate(zip(data_paths, rooms, p1_games, multis), 1):
                 involved_games = {"Archipelago"} | set(multi_games)
                 for collected_items in range(3):
-                    print(f"\nTesting [{n}] {game} in {multidata} on MultiServer with {collected_items} items collected")
-                    with LocalServeGame(multidata) as host:
-                        with Client(host.address, game, "Player1") as client:
-                            local_data_packages = client.games_packages
-                            local_collected_items = len(client.checked_locations)
-                            if collected_items < 2:  # Don't collect anything on the last iteration
-                                client.collect_any()
-                            # TODO: Ctrl+C test here as well
-
-                    for game_name in sorted(involved_games):
-                        expect_true(game_name in local_data_packages,
-                                    f"{game_name} missing from MultiServer datap ackage")
-                        expect_true("item_name_groups" not in local_data_packages.get(game_name, {}),
-                                    f"item_name_groups are not supposed to be in MultiServer data for {game_name}")
-                        expect_true("location_name_groups" not in local_data_packages.get(game_name, {}),
-                                    f"location_name_groups are not supposed to be in MultiServer data for {game_name}")
-                    for game_name in local_data_packages:
-                        expect_true(game_name in involved_games,
-                                    f"Received unexpected extra data package for {game_name} from MultiServer")
-                    assert_equal(local_collected_items, collected_items,
-                                 "MultiServer did not load or save correctly")
-
                     print(f"\nTesting [{n}] {game} in {multidata} on customserver with {collected_items} items collected")
                     prev_host_adr: str
                     with WebHostServeGame(webhost_client, room) as host:
+                        sleep(.1)  # wait for the server to fully start before doing anything
                         prev_host_adr = host.address
                         with Client(host.address, game, "Player1") as client:
                             web_data_packages = client.games_packages
@@ -134,6 +141,7 @@ if __name__ == "__main__":
                                 autohost(webapp.config)  # this will spin the room right up again
                                 sleep(1)  # make log less annoying
                                 # if saving failed, the next iteration will fail below
+                        sleep(2)  # work around issue #5571
 
                     # verify server shut down
                     try:
@@ -156,6 +164,31 @@ if __name__ == "__main__":
                                  "customserver did not load or save correctly during/after "
                                  + ("Ctrl+C" if collected_items == 2 else "/exit"))
 
+                    if not multidata:
+                        continue  # games rolled on WebHost can not be tested against MultiServer
+
+                    print(f"\nTesting [{n}] {game} in {multidata} on MultiServer with {collected_items} items collected")
+                    with LocalServeGame(multidata) as host:
+                        with Client(host.address, game, "Player1") as client:
+                            local_data_packages = client.games_packages
+                            local_collected_items = len(client.checked_locations)
+                            if collected_items < 2:  # Don't collect anything on the last iteration
+                                client.collect_any()
+                            # TODO: Ctrl+C test here as well
+
+                    for game_name in sorted(involved_games):
+                        expect_true(game_name in local_data_packages,
+                                    f"{game_name} missing from MultiServer datapackage")
+                        expect_true("item_name_groups" not in local_data_packages.get(game_name, {}),
+                                    f"item_name_groups are not supposed to be in MultiServer data for {game_name}")
+                        expect_true("location_name_groups" not in local_data_packages.get(game_name, {}),
+                                    f"location_name_groups are not supposed to be in MultiServer data for {game_name}")
+                    for game_name in local_data_packages:
+                        expect_true(game_name in involved_games,
+                                    f"Received unexpected extra data package for {game_name} from MultiServer")
+                    assert_equal(local_collected_items, collected_items,
+                                 "MultiServer did not load or save correctly")
+
                     # compare customserver to MultiServer
                     expect_equal(local_data_packages, web_data_packages,
                                  "customserver datapackage differs from MultiServer")
@@ -176,10 +209,12 @@ if __name__ == "__main__":
             print(f"Restoring multidata for {room}")
             set_multidata_for_room(webhost_client, room, old_data)
             with WebHostServeGame(webhost_client, room) as host:
+                sleep(.1)  # wait for the server to fully start before doing anything
                 with Client(host.address, game, "Player1") as client:
                     assert_equal(len(client.checked_locations), 2,
                                  "Save was destroyed during exception in customserver")
                     print("Save file is not busted 🥳")
+                sleep(2)  # work around issue #5571
 
         finally:
             print("Stopping autohost")

test/hosting/webhost.py

@@ -1,6 +1,10 @@
+import io
+import json
 import re
+import time
+import zipfile
 from pathlib import Path
-from typing import TYPE_CHECKING, Optional, cast
+from typing import TYPE_CHECKING, Iterable, Optional, cast
 
 from WebHostLib import to_python
 
@@ -10,6 +14,7 @@ if TYPE_CHECKING:
 
 __all__ = [
     "get_app",
+    "generate_remote",
     "upload_multidata",
     "create_room",
     "start_room",
@@ -17,6 +22,7 @@ __all__ = [
     "set_room_timeout",
     "get_multidata_for_room",
     "set_multidata_for_room",
+    "stop_autogen",
     "stop_autohost",
 ]
 
@@ -33,10 +39,43 @@ def get_app(tempdir: str) -> "Flask":
         "TESTING": True,
         "HOST_ADDRESS": "localhost",
         "HOSTERS": 1,
+        "GENERATORS": 1,
+        "JOB_THRESHOLD": 1,
     })
     return get_app()
 
 
+def generate_remote(app_client: "FlaskClient", games: Iterable[str]) -> str:
+    data = io.BytesIO()
+    with zipfile.ZipFile(data, "a", zipfile.ZIP_DEFLATED, False) as zip_file:
+        for n, game in enumerate(games, 1):
+            name = f"{n}.yaml"
+            zip_file.writestr(name, json.dumps({
+                "name": f"Player{n}",
+                "game": game,
+                game: {},
+                "description": f"generate_remote slot {n} ('Player{n}'): {game}",
+            }))
+    data.seek(0)
+    response = app_client.post("/generate", content_type="multipart/form-data", data={
+        "file": (data, "yamls.zip"),
+    })
+    assert response.status_code < 400, f"Starting gen failed: status {response.status_code}"
+    assert "Location" in response.headers, f"Starting gen failed: no redirect"
+    location = response.headers["Location"]
+    assert isinstance(location, str)
+    assert location.startswith("/wait/"), f"Starting WebHost gen failed: unexpected redirect to {location}"
+    for attempt in range(10):
+        response = app_client.get(location)
+        if "Location" in response.headers:
+            location = response.headers["Location"]
+            assert isinstance(location, str)
+            assert location.startswith("/seed/"), f"Finishing WebHost gen failed: unexpected redirect to {location}"
+            return location[6:]
+        time.sleep(1)
+    raise TimeoutError("WebHost gen did not finish")
+
+
 def upload_multidata(app_client: "FlaskClient", multidata: Path) -> str:
     response = app_client.post("/uploads", data={
         "file": multidata.open("rb"),
@@ -188,7 +227,7 @@ def set_multidata_for_room(webhost_client: "FlaskClient", room_id: str, data: by
         room.seed.multidata = data
 
 
-def stop_autohost(graceful: bool = True) -> None:
+def _stop_webhost_mp(name_filter: str, graceful: bool = True) -> None:
     import os
     import signal
 
@@ -198,13 +237,30 @@ def stop_autohost(graceful: bool = True) -> None:
 
     stop()
     proc: multiprocessing.process.BaseProcess
-    for proc in filter(lambda child: child.name.startswith("MultiHoster"), multiprocessing.active_children()):
+    for proc in filter(lambda child: child.name.startswith(name_filter), multiprocessing.active_children()):
+        # FIXME: graceful currently does not work on Windows because the signals are not properly emulated
+        #        and ungraceful may not save the game
+        if proc.pid == os.getpid():
+            continue
         if graceful and proc.pid:
             os.kill(proc.pid, getattr(signal, "CTRL_C_EVENT", signal.SIGINT))
         else:
             proc.kill()
         try:
+            try:
+                proc.join(30)
+            except TimeoutError:
+                raise
+            except KeyboardInterrupt:
+                # on Windows, the MP exception may be forwarded to the host, so ignore once and retry
                 proc.join(30)
         except TimeoutError:
             proc.kill()
             proc.join()
+
+def stop_autogen(graceful: bool = True) -> None:
+    # FIXME: this name filter is jank, but there seems to be no way to add a custom prefix for a Pool
+    _stop_webhost_mp("SpawnPoolWorker-", graceful)
+
+def stop_autohost(graceful: bool = True) -> None:
+    _stop_webhost_mp("MultiHoster", graceful)

test/hosting/world.py

@@ -11,7 +11,7 @@ _new_worlds: dict[str, str] = {}
 
 def copy(src: str, dst: str) -> None:
     from Utils import get_file_safe_name
-    from worlds import AutoWorldRegister
+    from worlds.AutoWorld import AutoWorldRegister
 
     assert dst not in _new_worlds, "World already created"
     if '"' in dst or "\\" in dst:  # easier to reject than to escape