[scripts] Add smoke tests and other changes

Error in readme has been fixed, python now has more type hints and the run_test script has been completely reworked. It is currently unknown if this test script works on windows. The github deploy workflow has been updated to take advantage of these tests.

.github/workflows/deploy.yml

@@ -24,7 +24,6 @@ env:
   PACKWIZ_COMMIT: 0bb89a4872d8dc2c45af251345ee780cab7ab9ad
   PACKWIZ_DIR: /tmp/packwiz_artifact
   PACKWIZ: /tmp/packwiz_artifact/packwiz
-  PACKWIZ_BOOTSTRAP_VERSION: "v0.0.3"
 
 jobs:
   build_test_deploy:
@@ -43,7 +42,7 @@ jobs:
 
       - name: Cache Packwiz
         id: cache-packwiz
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: ${{ env.PACKWIZ_DIR }}
           key: packwiz-${{ env.PACKWIZ_COMMIT }}
@@ -65,18 +64,33 @@ jobs:
       - name: Build pack
         run: python scripts/assemble_packwiz.py
 
-      # Test
-      # - name: Cache Packwiz Bootstrap
-      #   uses: actions/cache@v3
-      #   with:
-      #     path: run/packwiz-installer
-      #     key: packwiz-bootstrap-${{ env.PACKWIZ_BOOTSTRAP_VERSION }}
-      # - uses: actions/setup-java@v4
-      #   with:
-      #     distribution: 'temurin' # See 'Supported distributions' for available options
-      #     java-version: '21'
-      # - name: Test pack
-      #   run: python scripts/run_test.py
+      # Setup test-runner cache
+      - name: Generate cache key
+        run: python scripts/run_test.py
+        env:
+          GENERATE_DESIRED_CACHE_STATE_AND_EXIT: true
+      - name: Cache test-runner (static cache)
+        uses: actions/cache@v4
+        with:
+          path: |
+            run/cache-dynamic
+          key: test-run-static-cache-${{ hashFiles('run/desired_cache_state_for_static_cache.json') }} # Try and get the desired state
+          restore-keys: test-run-static-cache- # But fall back if needed
+      - name: Cache test-runner (dynamic cache)
+        uses: actions/cache@v4
+        with:
+          path: |
+            run/cache-dynamic
+          key: test-run-dynamic-cache-${{ hashFiles('generated/pack/pack.toml') }} # pack.toml contains hashes for all other files
+          restore-keys: test-run-dynamic-cache-
+
+      # Run tests
+      - uses: actions/setup-java@v4
+        with:
+          distribution: 'temurin' # See 'Supported distributions' for available options
+          java-version: '21'
+      - name: Test pack
+        run: python scripts/run_test.py
 
       # Deploy
 

.github/workflows/pull_platform.yml

@@ -28,7 +28,7 @@ jobs:
 
       - name: Cache Packwiz
         id: cache-packwiz
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: ${{ env.PACKWIZ_DIR }}
           key: packwiz-${{ env.PACKWIZ_COMMIT }}

README.md

@@ -2,8 +2,7 @@
 
 ## Repository setup
 Please create an (empty) `pack/index.toml`.
-This will ensure packwiz commands work correctly in the `pack/` directory.
-Scripts require only a recent version of python to run (>=3.11). No dependencies are required except when running tests.
+This will ensure packwiz commands work correctly in the `pack/` directory. `tomli_w` is the only dependency needed to run the scripts.
 
 The `pack/` directory contains the bulk of the pack. The files in here can be updated using the [packwiz](https://github.com/packwiz/packwiz) utility. The final pack will also include all submissions (and their dependencies), which are pulled from ModFest's platform api. The `pack/` directory will always take priority and can be used to override submitted mods. Submissions can be excluded altogether by putting it in the `platform.ignore` file.
 

scripts/assemble_packwiz.py

@@ -1,12 +1,15 @@
 #!/usr/bin/env python3
-import shutil
 import json
-import common
 import os
-import subprocess
 import re
+import shutil
+import subprocess
+from typing import Any, TypeAlias, TypedDict
+
+import common
 import tomli_w
 
+
 def main():
     repo_root = common.get_repo_root()
     submission_lock_file = repo_root / "submissions-lock.json"
@@ -24,7 +27,7 @@ def main():
 
     exclusions = list(filter(lambda l : len(l) > 0, [re.sub("#.*", "", l.strip()) for l in common.read_file(exclude_file).split("\n")]))
 
-    locked_data = json.loads(common.read_file(submission_lock_file))
+    locked_data: SubmissionLockfileFormat = json.loads(common.read_file(submission_lock_file))
     for platformid, moddata in locked_data.items():
         if not "files" in moddata:
             raise RuntimeError(f"lock data for {platformid} is invalid. Does not contain file key")
@@ -49,4 +52,10 @@ def main():
     subprocess.run([packwiz, "refresh"])
 
 if __name__ == "__main__":
-    main()
+    main()
+
+# For type hints
+class SubmissionLockfileEntry(TypedDict):
+    url: str 
+    files: dict[str, Any]
+SubmissionLockfileFormat: TypeAlias = dict[str, SubmissionLockfileEntry]

scripts/assemble_unsup.py

@@ -1,14 +1,16 @@
 #!/usr/bin/env python3
-import sys
-import re
-from zipfile import ZipFile
-import zipfile
 import io
 import json
+import re
+import sys
 import urllib.request
+import zipfile
+from typing import Any
+from zipfile import ZipFile
 
-from common import Ansi
 import common
+from common import Ansi
+
 
 def main():
     repo_root = common.get_repo_root()
@@ -17,7 +19,7 @@ def main():
     generated_dir = common.get_generated_dir()
 
     url = common.env("URL")
-    if url == None:
+    if url is None:
         print(f"{Ansi.ERROR}Please set the URL environment variable to the public url for this pack{Ansi.RESET}")
         sys.exit(1)
     if not url.endswith("pack.toml"):
@@ -102,7 +104,7 @@ def create_unsup_patch(unsup_version):
 # Creates the mmc-pack.json file, which stores "dependency" information for prism/multimc
 # The most important thing is that it defines the minecraft version and launcher used
 def create_mmc_meta(packwiz_info, unsup_version):
-    meta = {}
+    meta: Any = {}
     meta["formatVersion"] = 1
 
     components = []
@@ -144,7 +146,7 @@ def create_instance_config(packwiz_info, icon_name):
 
 # Creates the unsup config file, which tells unsup where
 # to download mods from
-def create_unsup_ini(url, constants):
+def create_unsup_ini(url: str, constants):
     colour_entries = []
     for colour_key in unsup_colours:
         colour_value = common.get_colour(constants, "_unsup_"+colour_key)

scripts/common.py

@@ -1,12 +1,14 @@
+import hashlib
 import json
 import os
+import re
 import shutil
 import time
-from pathlib import Path
-import hashlib
 import tomllib
 from dataclasses import dataclass
-import re
+from pathlib import Path
+from typing import Any, Callable, TypedDict, TypeVar, Unpack, overload
+
 
 class Ansi:
     BOLD = '\033[1m'
@@ -17,42 +19,45 @@ class Ansi:
     ERROR = RED_FG+BOLD
     RESET = '\033[0m'
 
-def check_packwiz():
+def check_packwiz() -> Path:
+    """Get the current packwiz executable"""
     packwiz = env("PACKWIZ", default="packwiz")
     if p := shutil.which(packwiz):
-        return p
+        return Path(p)
     else:
         raise RuntimeError(f"!!! Couldn't find packwiz (looked for '{packwiz}'). Please put packwiz on your path or set the PACKWIZ environment variable to a packwiz executable")
 
-def check_java():
+def check_java() -> Path:
+    """Get the current java executable"""
     java = "java"
     if "JAVA_HOME" in os.environ:
-        java = Path(os.environ["JAVA_HOME"]) / "bin/java"
-        if not java.exists():
+        java_p = Path(os.environ["JAVA_HOME"]) / "bin/java"
+        if not java_p.exists():
             raise RuntimeError(f"!!! JAVA_HOME is invalid. {java} does not exist")
-        return java
+        return java_p
     else:
-        if java := shutil.which("java"):
-            return p
+        if resolved_java := shutil.which("java"):
+            return Path(resolved_java)
         else:
             raise RuntimeError(f"!!! Couldn't find java on path. Please add it or set JAVA_HOME")
 
-def get_repo_root():
+def get_repo_root() -> Path:
     # This file should be located in <repo_root>/scripts/common.py, so the root
     # is one directory up from this one
     return Path(os.path.join(os.path.dirname(__file__), '..'))
 
-def get_generated_dir():
+def get_generated_dir() -> Path:
     dir = env("OUTPUT_DIR", default=(get_repo_root() / "generated"))
+    dir = Path(dir)
     if not dir.exists():
         dir.mkdir(exist_ok=True, parents=True)
     return dir
 
-def read_file(path):
+def read_file(path: os.PathLike) -> str:
     with open(path, "r") as f:
         return f.read()
 
-def fix_packwiz_pack(pack_toml):
+def fix_packwiz_pack(pack_toml: Path):
     data = tomllib.loads(read_file(pack_toml))
     index = pack_toml.parent / data["index"]["file"]
     if not index.exists():
@@ -62,32 +67,42 @@ class JSONWithCommentsDecoder(json.JSONDecoder):
     def __init__(self, **kw):
         super().__init__(**kw)
 
-    def decode(self, s: str):
+    def decode(self, s, _w):
         s = '\n'.join(l if not l.lstrip().startswith('//') else '' for l in s.split('\n'))
-        return super().decode(s)
+        return super().decode(s, _w)
 
-def jsonc_at_home(input):
+def jsonc_at_home(input: str | bytes) -> Any:
     return json.loads(input, cls=JSONWithCommentsDecoder)
 
-def hash(values: list[str]):
+def hash(values: list[str]) -> str:
     hasher = hashlib.sha256()
     for value in values:
         hasher.update(value.encode("UTF-8"))
     return hasher.hexdigest()
 
-def env(env: str, **kwargs):
+# overloads for proper type checking
+T = TypeVar('T')
+@overload
+def env(env: str, *, default: None = None) -> None | str: ...
+@overload
+def env(env: str, *, default: T) -> T | str: ...
+
+def env(env: str, *, default: Any = None) -> Any | str:
     if env in os.environ:
         return os.environ[env]
     else:
-        return kwargs.get("default")
+        return default
+
+class Constants(TypedDict):
+    colours: dict[str, str]
 
-def get_colour(parsed_constants, key):
+def get_colour(parsed_constants: Constants, key: str) -> str:
     """Given a parsed constants.jsonc, retrieves a colour by key. Returns a value in the form of #FFFFFF"""
     if not key.startswith("_"):
         raise RuntimeError("Scripts should only depend on colour keys starting with an underscore")
     def get_inner(k):
         v = parsed_constants["colours"].get(k)
-        if v == None:
+        if v is None:
             return None
         elif v.startswith("."):
             return get_inner(v[1:])
@@ -98,16 +113,29 @@ def get_inner(k):
     return get_inner(key)
 
 class Ratelimiter:
-    def __init__(self, time):
+    def __init__(self, time: float):
         # Time is given in seconds, convert to nanoseconds
         self.wait_time = time
-        self.last_action = 0
+        self.last_action: float = 0
 
     def limit(self):
         time.sleep(max(0, self.wait_time - (time.time() - self.last_action)))
         self.last_action = time.time()
 
-def parse_packwiz(pack_toml_file):
+@dataclass
+class PackwizPackInfo:
+    name: str | None
+    author: str | None
+    pack_version: str | None
+    minecraft_version: str
+    loader: str
+    loader_version: str
+
+    def safe_name(self) -> str:
+        assert self.name is not None
+        return re.sub("[^a-zA-Z0-9]+", "-", self.name)
+
+def parse_packwiz(pack_toml_file: Any) -> PackwizPackInfo:
     pack_toml = tomllib.loads(read_file(pack_toml_file))
 
     version_data = pack_toml["versions"]
@@ -136,16 +164,4 @@ def parse_packwiz(pack_toml_file):
         version_data["minecraft"],
         loader,
         loader_version
-    )
-
-@dataclass
-class PackwizPackInfo:
-    name: str
-    author: str
-    pack_version: str
-    minecraft_version: str
-    loader: str
-    loader_version: str
-
-    def safe_name(self):
-        return re.sub("[^a-zA-Z0-9]+", "-", self.name)
+    )