From fc142e0c9f1e6a911208088ece6b3e31917ff745 Mon Sep 17 00:00:00 2001 From: Hasan Zakeri Date: Thu, 16 Apr 2026 00:46:59 -0700 Subject: [PATCH 1/2] Add Tier 2 regression suite against external harfrust corpus Introduces an opt-in regression mode that shapes every test case in a local harfrust checkout through pyharfrust.shape and compares against the expected output embedded in harfrust's own tests. Enabled by setting HARFRUST_SOURCE; default-denied in pytest config so local dev and the main-branch CI stay fast. CI runs Tier 2 only on push/PR targeting the release branch, against harfrust pinned at commit efdae31 (0.5.2) to match the hr-shape dep. --- .github/workflows/ci.yml | 45 ++++++++++++++++++++++++--- pyproject.toml | 1 + tests/conftest.py | 66 ++++++++++++++++++++++++++++++++++++---- 3 files changed, 102 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fb7f113..941998d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,9 +2,9 @@ name: CI on: push: - branches: [main] + branches: [main, release] pull_request: - branches: [main] + branches: [main, release] workflow_dispatch: env: @@ -18,7 +18,7 @@ jobs: ci: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - uses: dtolnay/rust-toolchain@stable with: @@ -26,7 +26,7 @@ jobs: - uses: Swatinem/rust-cache@v2 - - uses: astral-sh/setup-uv@v4 + - uses: astral-sh/setup-uv@v6 with: enable-cache: true cache-dependency-glob: uv.lock @@ -54,3 +54,40 @@ jobs: - name: Python tests run: uv run pytest + + tier2: + # Tier 2: external regression against the full harfrust shaping corpus. + # Only runs on the release branch, where we care about version-drift signal. + if: github.ref == 'refs/heads/release' || github.base_ref == 'release' + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Check out harfrust at 0.5.2 + uses: actions/checkout@v6 + with: + repository: harfbuzz/harfrust + # Pinned to the 0.5.2 commit so the test corpus matches the hr-shape + # version declared in Cargo.toml. Bump this alongside the dep. + ref: efdae31 + path: harfrust-external + + - uses: dtolnay/rust-toolchain@stable + + - uses: Swatinem/rust-cache@v2 + + - uses: astral-sh/setup-uv@v6 + with: + enable-cache: true + cache-dependency-glob: uv.lock + + - name: Install Python deps + run: uv sync --locked + + - name: Build extension + run: uv run maturin develop + + - name: Tier 2 shaping regression + env: + HARFRUST_SOURCE: ${{ github.workspace }}/harfrust-external + run: uv run pytest -m external diff --git a/pyproject.toml b/pyproject.toml index 63e8451..a2a7e81 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,6 +35,7 @@ reportMissingModuleSource = false [tool.pytest.ini_options] testpaths = ["tests"] +addopts = "-m 'not external'" markers = [ "external: tests that require an external harfrust checkout via HARFRUST_SOURCE", ] diff --git a/tests/conftest.py b/tests/conftest.py index 07fab62..e5418be 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,5 @@ import os +import re import pytest @@ -6,6 +7,20 @@ BUNDLED_FONTS = os.path.join(os.path.dirname(__file__), "fonts") HARFRUST_SOURCE = os.environ.get("HARFRUST_SOURCE") +MIN_EXTERNAL_CASES = 5000 + +_RS_TEST_RE = re.compile( + r"shape\(\s*" + r'"((?:\\.|[^"\\])*)"\s*,\s*' + r'"((?:\\.|[^"\\])*)"\s*,\s*' + r'"((?:\\.|[^"\\])*)"\s*,?\s*' + r"\)\s*,\s*" + r'"((?:\\.|[^"\\])*)"', + re.DOTALL, +) +_RS_U_ESC = re.compile(r"\\u\{([0-9A-Fa-f]+)\}") +_RS_LINE_CONT = re.compile(r"\\\n\s*") + def parse_tests_file(path): """Parse a harfbuzz-format .tests file into test cases.""" @@ -32,6 +47,25 @@ def parse_tests_file(path): return cases +def parse_rs_file(path, font_root): + """Extract test cases from a harfrust-generated tests/shaping/*.rs file. + + Font paths inside the file are relative to the harfrust crate root + (e.g. "tests/fonts/in-house/X.ttf"); ``font_root`` is that root. + """ + cases = [] + with open(path) as f: + content = f.read() + for font_rel, text_lit, options, expected in _RS_TEST_RE.findall(content): + text = _RS_LINE_CONT.sub("", text_lit) + text = _RS_U_ESC.sub(lambda m: chr(int(m.group(1), 16)), text) + font_path = os.path.normpath(os.path.join(font_root, font_rel)) + if not os.path.exists(font_path): + continue + cases.append((font_path, text, options, expected)) + return cases + + def collect_tests_files(): """Yield (case, is_external) pairs for every bundled and external test case.""" cases = [] @@ -41,12 +75,32 @@ def collect_tests_files(): for case in parse_tests_file(os.path.join(BUNDLED_DATA, f)): cases.append((case, False)) if HARFRUST_SOURCE: - ext_tests = os.path.join(HARFRUST_SOURCE, "harfrust", "tests", "custom") - if os.path.isdir(ext_tests): - for f in sorted(os.listdir(ext_tests)): - if f.endswith(".tests"): - for case in parse_tests_file(os.path.join(ext_tests, f)): - cases.append((case, True)) + external_cases = _collect_external_cases(HARFRUST_SOURCE) + if len(external_cases) < MIN_EXTERNAL_CASES: + raise RuntimeError( + f"HARFRUST_SOURCE is set to {HARFRUST_SOURCE!r} but only " + f"{len(external_cases)} Tier 2 cases were discovered " + f"(expected at least {MIN_EXTERNAL_CASES}). " + "Either the checkout is missing tests/shaping/*.rs or the parser " + "is out of sync with harfrust's test generator." + ) + cases.extend((c, True) for c in external_cases) + return cases + + +def _collect_external_cases(harfrust_source: str): + cases = [] + harfrust_root = os.path.join(harfrust_source, "harfrust") + shaping_dir = os.path.join(harfrust_root, "tests", "shaping") + if os.path.isdir(shaping_dir): + for f in sorted(os.listdir(shaping_dir)): + if f.endswith(".rs") and f != "main.rs": + cases.extend(parse_rs_file(os.path.join(shaping_dir, f), harfrust_root)) + custom_dir = os.path.join(harfrust_root, "tests", "custom") + if os.path.isdir(custom_dir): + for f in sorted(os.listdir(custom_dir)): + if f.endswith(".tests"): + cases.extend(parse_tests_file(os.path.join(custom_dir, f))) return cases From bdaa43f1eceb2af18887ca11cad146c44172d5a6 Mon Sep 17 00:00:00 2001 From: Hasan Zakeri Date: Thu, 16 Apr 2026 10:14:27 -0700 Subject: [PATCH 2/2] fix: exclude .tests source files from external test collection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `_collect_external_cases()` function was incorrectly parsing `.tests` files from harfrust's `tests/custom/` directory. These files are source inputs for harfrust's test generator (`gen-shaping-tests.py`), not actual test cases to run. They contain expected values that may intentionally differ from harfrust's current behavior, as noted in the files themselves: "the expected values for the shaping process will be ignored and sometimes wrong." This caused false test failures, such as the `--language=pl` BigCaslon test which expects `cacute.polish` but harfrust outputs `cacute` (confirmed by running hr-shape CLI directly). Now we only parse the generated `.rs` files in `tests/shaping/`, which represent the actual tests that `cargo test` runs in harfrust. Test count: 6145 → 6139 external cases (removed 6 invalid cases from .tests) --- tests/conftest.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index e5418be..e48687a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -89,6 +89,14 @@ def collect_tests_files(): def _collect_external_cases(harfrust_source: str): + """Collect test cases from harfrust's generated .rs test files. + + Note: We only parse .rs files in tests/shaping/, NOT the .tests files in + tests/custom/. The .tests files are source files for harfrust's test + generator (gen-shaping-tests.py) and may contain tests that are + intentionally excluded from the generated .rs files (e.g., macOS-only + tests, tests with known different expected values, etc.). + """ cases = [] harfrust_root = os.path.join(harfrust_source, "harfrust") shaping_dir = os.path.join(harfrust_root, "tests", "shaping") @@ -96,11 +104,6 @@ def _collect_external_cases(harfrust_source: str): for f in sorted(os.listdir(shaping_dir)): if f.endswith(".rs") and f != "main.rs": cases.extend(parse_rs_file(os.path.join(shaping_dir, f), harfrust_root)) - custom_dir = os.path.join(harfrust_root, "tests", "custom") - if os.path.isdir(custom_dir): - for f in sorted(os.listdir(custom_dir)): - if f.endswith(".tests"): - cases.extend(parse_tests_file(os.path.join(custom_dir, f))) return cases