Summary
While reviewing #11 I ran the JVM unit tests (:app:testOfflineDebugUnitTest) on both main (origin/main @ c63c688f) and the PR branch. 3 tests already fail on main, independent of #11. They go unnoticed because CI never runs the test suite.
Pre-existing failures on main
| Test |
Failure |
InputLogicTest.tapOnlyCombiningWordDoesNotShowAutospaceIndicatorWhenGestureGateEnabled |
Mockito ArgumentsAreDifferent (verifies setCombiningMode(false, …) called and setCombiningMode(true, …) never) |
InputLogicTest.insertLetterIntoWordHangulFails |
org.junit.ComparisonFailure |
InputLogicTest."revert autocorrect on delete" |
org.junit.ComparisonFailure |
Run on JDK 21 / Windows: 79 tests completed, 3 failed on main; the exact same 3 fail on #11's branch (87 tests completed, 3 failed). So #11 adds 8 passing tests and introduces zero regressions — these are old.
Why they're invisible
.github/workflows/build-test-auto.yml runs only compileOfflineRunTestsKotlin (compile-only). Tests compile but are never executed, so failures don't break CI.
Likely nature (needs triage)
- The two
ComparisonFailures are classic LatinIME tests (Hangul insertion, autocorrect-revert) that may depend on dictionary/locale data not available in the JVM unit environment — possibly environment-sensitive rather than genuine product bugs.
- The Mockito failure is a LeanType two-thumb combining-mode expectation; could be a brittle verification or a real logic drift.
Proposed
- Add a CI step that actually runs
:app:testOfflineDebugUnitTest (at least non-blocking at first) so regressions surface.
- Triage the 3 failures: fix, or quarantine/
@Ignore with a tracking note, so the suite goes green and future regressions are detectable.
Minor side note (from #11 review, not a blocker)
The opt-in live-converge path (tryLiveConvergeTap) makes a synchronous, GET_SUGGESTED_WORDS_TIMEOUT-bounded recognizer call per tap-after-swipe. It mirrors the existing performUpdateSuggestionStripSync pattern and is gated behind a default-off setting, so it's acceptable — but if per-tap latency ever shows up, making it async is the follow-up.
Summary
While reviewing #11 I ran the JVM unit tests (
:app:testOfflineDebugUnitTest) on bothmain(origin/main@c63c688f) and the PR branch. 3 tests already fail onmain, independent of #11. They go unnoticed because CI never runs the test suite.Pre-existing failures on
mainInputLogicTest.tapOnlyCombiningWordDoesNotShowAutospaceIndicatorWhenGestureGateEnabledArgumentsAreDifferent(verifiessetCombiningMode(false, …)called andsetCombiningMode(true, …)never)InputLogicTest.insertLetterIntoWordHangulFailsorg.junit.ComparisonFailureInputLogicTest."revert autocorrect on delete"org.junit.ComparisonFailureRun on JDK 21 / Windows:
79 tests completed, 3 failedonmain; the exact same 3 fail on #11's branch (87 tests completed, 3 failed). So #11 adds 8 passing tests and introduces zero regressions — these are old.Why they're invisible
.github/workflows/build-test-auto.ymlruns onlycompileOfflineRunTestsKotlin(compile-only). Tests compile but are never executed, so failures don't break CI.Likely nature (needs triage)
ComparisonFailures are classic LatinIME tests (Hangul insertion, autocorrect-revert) that may depend on dictionary/locale data not available in the JVM unit environment — possibly environment-sensitive rather than genuine product bugs.Proposed
:app:testOfflineDebugUnitTest(at least non-blocking at first) so regressions surface.@Ignorewith a tracking note, so the suite goes green and future regressions are detectable.Minor side note (from #11 review, not a blocker)
The opt-in live-converge path (
tryLiveConvergeTap) makes a synchronous,GET_SUGGESTED_WORDS_TIMEOUT-bounded recognizer call per tap-after-swipe. It mirrors the existingperformUpdateSuggestionStripSyncpattern and is gated behind a default-off setting, so it's acceptable — but if per-tap latency ever shows up, making it async is the follow-up.