diff --git a/.github/workflows/build-test-auto.yml b/.github/workflows/build-test-auto.yml index 76a74d548..e48994cbb 100644 --- a/.github/workflows/build-test-auto.yml +++ b/.github/workflows/build-test-auto.yml @@ -39,8 +39,30 @@ jobs: - name: Grant execute permission for gradlew run: chmod +x gradlew + + - name: Record run start time + id: start + run: echo "ts=$(date +%s)" >> "$GITHUB_OUTPUT" + - name: Run unit tests - run: ./gradlew :app:testOfflineRunTestsUnitTest + continue-on-error: true + run: ./gradlew :app:testOfflineRunTestsUnitTest --continue + + # Authoritative gate. Strictly stronger than the Gradle exit code above: + # it also fails on results that are stale (task was UP-TO-DATE, so the + # report describes code that never ran) or self-inconsistent (enumerated + # testcases disagree with the totals the suites declare). Both of those + # have previously produced confident, wrong "it passes" conclusions. + - name: Verify test results and diff against baseline + run: | + python3 tools/check_test_results.py \ + --results-dir app/build/test-results/testOfflineRunTestsUnitTest \ + --baseline tools/test_baselines/runTests-linux.txt \ + --started-after ${{ steps.start.outputs.ts }} + + - name: Self-test the results checker + if: ${{ always() }} + run: python3 -m unittest discover -s tools/tests - name: Archive test reports uses: actions/upload-artifact@v4 diff --git a/.gitignore b/.gitignore index 06dd05602..e0f9f2f93 100755 --- a/.gitignore +++ b/.gitignore @@ -86,3 +86,7 @@ docs/superpowers/ # Temporary generated files docs/releasenote/release_notes_temp.md + +# Python bytecode from tools/ +__pycache__/ +*.pyc diff --git a/AGENTS.md b/AGENTS.md index 4bd59613d..d8e5ff327 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -89,6 +89,13 @@ $env:JAVA_HOME = "C:\Program Files\Eclipse Adoptium\jdk-21.0.11.10-hotspot" - **Key tests:** `InputLogicTest.kt` (typing/autocorrect/combining-mode/Hangul), `SuggestTest.kt`, `WordComposerTest.java`, `DictionaryGroupTest.kt` (reflection + Mockito on the package-internal `DictionaryGroup`), `SettingsContainerTest.kt` (settings wiring), `KeyboardParserTest.kt`, `ClipboardDaoTest.kt`. - **Conventions:** `@Test`; method names use camelCase or backtick form; obtain `Context` via Robolectric; package-internal classes are exercised via reflection (`Class.forName(...).declaredConstructors`). - **Known failures:** the full debug unit suite has ~11 pre-existing failures (in `KeyboardParserTest`, `XLinkTest`, `StringUtilsTest` emoji, and `InputLogicTest` Hangul/autocorrect-revert/autospace-indicator) that are environment/data-dependent and usually unrelated to a change. The `runTests` build type exists to skip these on CI. **Verify a change by diffing failures against an `origin/main` baseline run, not by absolute pass count.** +- **Don't read the test report by hand — use the gate.** `tools/check_test_results.py` parses the JUnit XML and refuses to answer when the results can't be trusted: it fails if any result file predates the run (Gradle served an UP-TO-DATE task, so the report describes code that never ran) and if its own enumeration disagrees with the totals the suites declare (an under-counting reader looks like good news). It then diffs failing test **names** against a checked-in baseline, and quarantines `net:`-marked tests that reach the network so they are never counted as a regression or as an attributable fix. It runs automatically in the Unit tests workflow and is the authoritative gate there. Locally: + ```bash + python tools/check_test_results.py \ + --results-dir app/build/test-results/testOfflineRunTestsUnitTest \ + --baseline tools/test_baselines/runTests-windows.txt + ``` + Baselines live in `tools/test_baselines/`. If it reports new failures, don't paste them into the baseline — establish they aren't yours first, then `--update-baseline` and say why in the PR. The checker has its own tests: `python -m unittest discover -s tools/tests`. - **Coverage gap:** gesture/glide recognition needs the native engine, so JVM unit tests exercise tap-based logic, not native gesture recognition (a trace/replay harness is planned — see `docs/IMPROVEMENT_PLAN.md`). - **Expectation:** new behavior MUST add/update unit tests; any settings change updates `SettingsContainerTest.kt`. Keep PRs single-responsibility (see `CONTRIBUTING.md`). diff --git a/tools/__pycache__/release.cpython-313.pyc b/tools/__pycache__/release.cpython-313.pyc deleted file mode 100644 index c77c38ac2..000000000 Binary files a/tools/__pycache__/release.cpython-313.pyc and /dev/null differ diff --git a/tools/check_test_results.py b/tools/check_test_results.py new file mode 100644 index 000000000..958816ca4 --- /dev/null +++ b/tools/check_test_results.py @@ -0,0 +1,256 @@ +#!/usr/bin/env python3 +"""Integrity + baseline gate for Gradle JUnit XML test results. + +This exists because three separate false conclusions were drawn from Gradle test +output during one session, none of which the test run itself flagged: + +1. Gradle served results from a previous run because the test task was + UP-TO-DATE, so a "passing" report described code that was never exercised. +2. A hand-rolled XML reader silently under-counted: it disagreed with the + totals the results themselves declared, and reported 4 failures where there + were 12. The exact mechanism is less important than the fact that nothing + flagged it -- under-reporting is the dangerous direction, because it looks + like good news. So this script never trusts its own enumeration: it counts + elements AND sums the tests=/failures= attributes the suites + declare, and refuses to report anything if the two disagree. +3. Failures were attributed to a code change when at least one of them reaches + the network and can flip with no code change at all. + +Each check below is one of those, made mechanical. Run it after a test task; +it exits non-zero rather than relying on anyone remembering. + +Usage: + python tools/check_test_results.py \ + --results-dir app/build/test-results/testOfflineRunTestsUnitTest \ + --baseline tools/test_baselines/runTests-linux.txt \ + --started-after 1723800000 + + # after deliberately changing which tests fail: + python tools/check_test_results.py --results-dir ... --baseline ... --update-baseline + +Exit codes: + 0 results are trustworthy and match the baseline + 1 baseline mismatch (new failures) + 2 integrity failure (stale, unparseable, or self-inconsistent results) +""" + +from __future__ import annotations + +import argparse +import os +import sys +import xml.etree.ElementTree as ET +from dataclasses import dataclass, field +from pathlib import Path + +NET_PREFIX = "net:" + + +@dataclass +class Results: + """Everything parsed out of one results directory.""" + + # test ids that failed or errored, as "ClassName > test name" + failed: set[str] = field(default_factory=set) + # declared totals, summed from attributes + declared_tests: int = 0 + declared_failures: int = 0 + # observed totals, counted from elements + observed_tests: int = 0 + observed_failures: int = 0 + suites: int = 0 + files: int = 0 + + +def parse_results(results_dir: Path) -> Results: + """Parse every in every XML file under results_dir. + + Deliberately iterates all testsuite elements, not just the document root: + a single file may hold more than one, and missing them silently + under-reports failures. + """ + res = Results() + xml_files = sorted(results_dir.glob("**/*.xml")) + if not xml_files: + raise SystemExit(f"[integrity] no result XML found under {results_dir}") + + for path in xml_files: + try: + tree = ET.parse(path) + except ET.ParseError as exc: + raise SystemExit(f"[integrity] cannot parse {path}: {exc}") + res.files += 1 + + root = tree.getroot() + suites = [root] if root.tag == "testsuite" else [] + suites.extend(root.iter("testsuite") if root.tag != "testsuite" else []) + # a root may itself nest further children + if root.tag == "testsuite": + suites.extend(root.findall("testsuite")) + + seen = set() + for suite in suites: + if id(suite) in seen: + continue + seen.add(id(suite)) + res.suites += 1 + res.declared_tests += int(suite.get("tests", 0)) + res.declared_failures += int(suite.get("failures", 0)) + int( + suite.get("errors", 0) + ) + suite_name = (suite.get("name") or path.stem).split(".")[-1] + + for case in suite.findall("testcase"): + res.observed_tests += 1 + if case.find("failure") is not None or case.find("error") is not None: + res.observed_failures += 1 + res.failed.add(f"{suite_name} > {case.get('name')}") + + return res + + +def check_freshness(results_dir: Path, started_after: float) -> list[str]: + """Every result file must post-date the run we think produced it.""" + problems = [] + for path in sorted(results_dir.glob("**/*.xml")): + mtime = path.stat().st_mtime + if mtime < started_after: + problems.append( + f" {path.name} last written {mtime:.0f}, before run start {started_after:.0f}" + ) + return problems + + +def check_self_consistency(res: Results) -> list[str]: + """Declared totals must equal what we actually enumerated. + + This is the check that catches an under-counting reader: whatever the + mechanism (a skipped suite, a results file read while still being written, + an unexpected root element), the enumerated counts come out lower than the + totals the suites declare, and that disagreement is mechanically visible. + """ + problems = [] + if res.declared_tests != res.observed_tests: + problems.append( + f" declared {res.declared_tests} tests but enumerated {res.observed_tests}" + ) + if res.declared_failures != res.observed_failures: + problems.append( + f" declared {res.declared_failures} failures but enumerated " + f"{res.observed_failures}" + ) + return problems + + +def load_baseline(path: Path) -> tuple[set[str], set[str]]: + """Return (expected_failures, network_dependent).""" + expected: set[str] = set() + networked: set[str] = set() + if not path.exists(): + return expected, networked + for raw in path.read_text(encoding="utf-8").splitlines(): + line = raw.strip() + if not line or line.startswith("#"): + continue + if line.startswith(NET_PREFIX): + networked.add(line[len(NET_PREFIX):].strip()) + else: + expected.add(line) + return expected, networked + + +def write_baseline(path: Path, failed: set[str], networked: set[str]) -> None: + lines = [ + "# Known-failing tests. Generated by tools/check_test_results.py.", + "# One test id per line, as reported: 'ClassName > test name'.", + "# Prefix a line with 'net:' if the test reaches the network -- those are", + "# reported but never treated as a regression or as an attributable fix,", + "# because they can flip without any code change.", + "", + ] + lines += sorted(failed - networked) + if networked: + lines.append("") + lines += [f"{NET_PREFIX}{name}" for name in sorted(networked)] + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("\n".join(lines) + "\n", encoding="utf-8") + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description=__doc__.split("\n")[0]) + ap.add_argument("--results-dir", required=True, type=Path) + ap.add_argument("--baseline", required=True, type=Path) + ap.add_argument( + "--started-after", + type=float, + default=None, + help="epoch seconds; every result file must be newer than this", + ) + ap.add_argument("--update-baseline", action="store_true") + args = ap.parse_args(argv) + + if not args.results_dir.is_dir(): + print(f"[integrity] results dir does not exist: {args.results_dir}") + return 2 + + # 1. staleness + if args.started_after is not None: + stale = check_freshness(args.results_dir, args.started_after) + if stale: + print("[integrity] STALE RESULTS -- the test task did not rerun:") + print("\n".join(stale)) + print(" re-run with --rerun-tasks, or pass the correct --started-after") + return 2 + + res = parse_results(args.results_dir) + + # 2. self-consistency + inconsistent = check_self_consistency(res) + if inconsistent: + print("[integrity] RESULTS DISAGREE WITH THEMSELVES:") + print("\n".join(inconsistent)) + print( + " the parser missed testcases -- most likely a file holding more than\n" + " one . Do not trust any count from this run." + ) + return 2 + + print( + f"[ok] {res.files} file(s), {res.suites} suite(s), " + f"{res.observed_tests} tests, {res.observed_failures} failed" + ) + + expected, networked = load_baseline(args.baseline) + + if args.update_baseline: + write_baseline(args.baseline, res.failed, networked & res.failed) + print(f"[ok] baseline written to {args.baseline} ({len(res.failed)} entries)") + return 0 + + # 3. baseline diff, by NAME, with network-dependent tests quarantined + new_failures = res.failed - expected - networked + fixed = expected - res.failed + net_failing = res.failed & networked + + if net_failing: + print("[note] network-dependent tests failing (not counted either way):") + for name in sorted(net_failing): + print(f" {name}") + + if fixed: + print("[note] no longer failing -- refresh the baseline if deliberate:") + for name in sorted(fixed): + print(f" {name}") + + if new_failures: + print(f"[FAIL] {len(new_failures)} test(s) failing that the baseline does not list:") + for name in sorted(new_failures): + print(f" {name}") + return 1 + + print("[ok] no new failures against baseline") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/test_baselines/runTests-linux.txt b/tools/test_baselines/runTests-linux.txt new file mode 100644 index 000000000..c7e7ad40d --- /dev/null +++ b/tools/test_baselines/runTests-linux.txt @@ -0,0 +1,15 @@ +# Known-failing tests. Generated by tools/check_test_results.py. +# One test id per line, as reported: 'ClassName > test name'. +# Prefix a line with 'net:' if the test reaches the network -- those are +# reported but never treated as a regression or as an attributable fix, +# because they can flip without any code change. +# +# This is the baseline for the `runTests` build type on Linux CI, which is the +# variant the Unit tests workflow gates on. It is intentionally empty: the +# `runTests` type self-skips the data/network-dependent tests, and the four +# ParserTest failures seen on Windows do not reproduce on Linux. +# +# If CI fails here listing test names, do not "fix" it by pasting them in. +# Work out whether the change caused them first; only then run +# python tools/check_test_results.py --results-dir --baseline --update-baseline +# and say in the PR why the baseline moved. diff --git a/tools/test_baselines/runTests-windows.txt b/tools/test_baselines/runTests-windows.txt new file mode 100644 index 000000000..34981573c --- /dev/null +++ b/tools/test_baselines/runTests-windows.txt @@ -0,0 +1,10 @@ +# Known-failing tests. Generated by tools/check_test_results.py. +# One test id per line, as reported: 'ClassName > test name'. +# Prefix a line with 'net:' if the test reaches the network -- those are +# reported but never treated as a regression or as an attributable fix, +# because they can flip without any code change. + +ParserTest > canLoadKeyboard +ParserTest > de_DE has extra keys +ParserTest > dvorak has 4 rows +ParserTest > popup key count does not depend on shift for (for simple layout) diff --git a/tools/tests/test_check_test_results.py b/tools/tests/test_check_test_results.py new file mode 100644 index 000000000..0a0947ceb --- /dev/null +++ b/tools/tests/test_check_test_results.py @@ -0,0 +1,181 @@ +#!/usr/bin/env python3 +"""Self-tests for tools/check_test_results.py. + +Each test corresponds to a false conclusion that was actually drawn from Gradle +output, so the gate is itself gated. Run with: + + python -m unittest discover -s tools/tests -v +""" + +from __future__ import annotations + +import os +import sys +import tempfile +import time +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +import check_test_results as gate # noqa: E402 + + +SINGLE_SUITE = """ + + + + stack + + +""" + +# Gradle writes several elements into one file for +# parameterised/nested classes. Reading only the first one silently drops the +# rest -- exactly the bug this gate exists to catch. +MULTI_SUITE = """ + + + + + + + stack + + + stack + + + + +""" + +# declares more tests than it lists -- what a parser miss looks like from outside +INCONSISTENT = """ + + + +""" + + +def write(dirpath: Path, name: str, content: str) -> Path: + p = dirpath / name + p.write_text(content, encoding="utf-8") + return p + + +class ParsingTests(unittest.TestCase): + def test_counts_every_testsuite_in_a_file(self): + """The regression this gate was built for: 2 of 3 suites must not vanish.""" + with tempfile.TemporaryDirectory() as td: + d = Path(td) + write(d, "TEST-multi.xml", MULTI_SUITE) + res = gate.parse_results(d) + self.assertEqual(res.observed_tests, 4, "should see all 4 testcases") + self.assertEqual(res.observed_failures, 2) + self.assertIn("ParserTest > canLoadKeyboard", res.failed) + self.assertIn("ParserTest > dvorak has 4 rows", res.failed) + + def test_aggregates_across_files(self): + with tempfile.TemporaryDirectory() as td: + d = Path(td) + write(d, "TEST-a.xml", SINGLE_SUITE) + write(d, "TEST-b.xml", MULTI_SUITE) + res = gate.parse_results(d) + self.assertEqual(res.files, 2) + self.assertEqual(res.observed_tests, 6) + self.assertEqual(res.observed_failures, 3) + + def test_empty_results_dir_is_an_error(self): + with tempfile.TemporaryDirectory() as td: + with self.assertRaises(SystemExit): + gate.parse_results(Path(td)) + + +class IntegrityTests(unittest.TestCase): + def test_self_inconsistency_is_detected(self): + with tempfile.TemporaryDirectory() as td: + d = Path(td) + write(d, "TEST-x.xml", INCONSISTENT) + res = gate.parse_results(d) + problems = gate.check_self_consistency(res) + self.assertTrue(problems, "declared 43 vs enumerated 1 must be flagged") + + def test_stale_results_are_detected(self): + with tempfile.TemporaryDirectory() as td: + d = Path(td) + p = write(d, "TEST-a.xml", SINGLE_SUITE) + old = time.time() - 3600 + os.utime(p, (old, old)) + self.assertTrue(gate.check_freshness(d, time.time() - 60)) + + def test_fresh_results_pass(self): + with tempfile.TemporaryDirectory() as td: + d = Path(td) + write(d, "TEST-a.xml", SINGLE_SUITE) + self.assertEqual(gate.check_freshness(d, time.time() - 60), []) + + +class BaselineTests(unittest.TestCase): + def _run(self, results_xml, baseline_text, extra=None): + with tempfile.TemporaryDirectory() as td: + d = Path(td) + rd = d / "results" + rd.mkdir() + write(rd, "TEST-a.xml", results_xml) + bl = d / "baseline.txt" + bl.write_text(baseline_text, encoding="utf-8") + argv = ["--results-dir", str(rd), "--baseline", str(bl)] + argv += extra or [] + return gate.main(argv) + + def test_known_failure_passes(self): + rc = self._run(SINGLE_SUITE, "InputLogicTest > breaks\n") + self.assertEqual(rc, 0) + + def test_new_failure_fails(self): + rc = self._run(SINGLE_SUITE, "# nothing known to fail\n") + self.assertEqual(rc, 1) + + def test_network_dependent_failure_is_not_a_regression(self): + """A test that reaches the network can flip with no code change.""" + rc = self._run(SINGLE_SUITE, "net:InputLogicTest > breaks\n") + self.assertEqual(rc, 0) + + def test_stale_results_fail_with_integrity_code(self): + with tempfile.TemporaryDirectory() as td: + d = Path(td) + rd = d / "results" + rd.mkdir() + p = write(rd, "TEST-a.xml", SINGLE_SUITE) + old = time.time() - 3600 + os.utime(p, (old, old)) + bl = d / "baseline.txt" + bl.write_text("InputLogicTest > breaks\n", encoding="utf-8") + rc = gate.main([ + "--results-dir", str(rd), + "--baseline", str(bl), + "--started-after", str(time.time() - 60), + ]) + self.assertEqual(rc, 2, "stale results must not be reported as a pass") + + def test_inconsistent_results_fail_with_integrity_code(self): + rc = self._run(INCONSISTENT, "") + self.assertEqual(rc, 2) + + def test_update_baseline_roundtrip(self): + with tempfile.TemporaryDirectory() as td: + d = Path(td) + rd = d / "results" + rd.mkdir() + write(rd, "TEST-a.xml", SINGLE_SUITE) + bl = d / "baseline.txt" + self.assertEqual( + gate.main(["--results-dir", str(rd), "--baseline", str(bl), + "--update-baseline"]), 0) + self.assertEqual( + gate.main(["--results-dir", str(rd), "--baseline", str(bl)]), 0) + + +if __name__ == "__main__": + unittest.main()