search

keymanapp / keyman

Keyman cross platform input methods system running on Android, iOS, Linux, macOS, Windows and mobile and desktop web
https://keyman.com/
Other
403 stars 112 forks source link

bug(developer): kmcmplib does not run a full keyboard repository test #12623

Closed markcsinclair closed 1 week ago

markcsinclair commented 2 weeks ago

Describe the bug

If a full keyboard repository test is requested in kmcmplib with, say, ./build.sh --full-test test:x86 no extra keyboards are tested compared to running without the --full-test flag

Keyman apps

Keyman version

No response

markcsinclair commented 2 weeks ago

It would seem that there is a mismatch between the build system and meson, such that the build system passes through excess parameters such as --full-test, but that meson invocations, such as meson test, can't take these additional parameters. I have yet to identify a way that this can be achieved, although several possibities are discussed on forums, but I have not found one that works in practice.

In addition, the actual option identified in kmcmplib/meson_options.txt is full_test instead of full-test (see also line 106 of kmcmplib/tests/meson.build).

If the full_test option is defaulted to true (in kmcmplib/meson_options.txt), sadly, 65 of 702 tests fail, although many of these seem to be memory allocation problems.

mcdurdin commented 2 weeks ago

If the full_test option is defaulted to true (in kmcmplib/meson_options.txt), sadly, 65 of 702 tests fail, although many of these seem to be memory allocation problems.

This is curious, because we are building all the keyboards regularly with the release build of kmc/kmcmplib, whenever we receive a new PR in the keyboards repository (most days). Looks like we need to get this resolved so we can track this.

mcdurdin commented 2 weeks ago

If the full_test option is defaulted to true (in kmcmplib/meson_options.txt), sadly, 65 of 702 tests fail, although many of these seem to be memory allocation problems.

Turns out they are not memory allocation problems -- there is some misdirection in the reporting due to meson. The test was returning error 88 which means that the binary file mismatches with the fixture.

I found a regression in #12107 which causes 64 of the 65 fails:

https://github.com/keymanapp/keyman/pull/12107/files#diff-966fa269ec2e2dd3b5c40550dbe4df652e4f0f4253ae75552838852a0a6df629L11-R12

The N was accidentally replaced with M. Any keyboards which included a nomatch rule would fail with this. New PR on its way to address this (and --full-test will come as a separate PR too)

Still investigating the last failure.

mcdurdin commented 2 weeks ago

The final failure, for masaram_gondi, is because the keyboard source had non-BMP characters in the key part of the rule on line 215, which has been picked up by the 18.0 compiler in #11806: 

$keymanonly: if(opt1 = 'gondi') any(Vyanjana)"𑵄" + any(Vyanjana)    >   index(Vyanjana,2) U+11D45

The trick here is that the compiled fixture developer/src/kmcmplib/tests/fixtures/keyboards-repo/masaram_gondi.kmx is based on a broken version of the keyboard source in the keyboards repo (fixed version), and updating to the fixed version will also trigger updates to a bunch of other keyboards. I think the simplest resolution is probably to skip masaram_gondi for 18.0.

Once I got this building on mac and linux as well, discovered a few cross-platform issues, minor but varied. See #12631 for details.

Three PRs incoming: