Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(developer): reconnect --full-test in kmcmplib build and enable for CI #12631

Open
wants to merge 7 commits into
base: chore/developer/12623-skip-masaram-gondi
Choose a base branch
from

Conversation

mcdurdin
Copy link
Member

@mcdurdin mcdurdin commented Nov 6, 2024

The --full-test parameter in kmcmplib build.sh has not been working. This PR reconnects the parameter and also enables it by default for CI test builds (not release builds).

Fixes: #12623

@keymanapp-test-bot skip

…r CI

The --full-test parameter in kmcmplib build.sh has not been working.
This PR reconnects the parameter and also enables it by default for CI
test builds (not release builds).

Fixes: #12623
@keymanapp-test-bot keymanapp-test-bot bot added this to the A18S14 milestone Nov 6, 2024
@mcdurdin mcdurdin changed the title fix(developer): reconnect --full-test in kmcmplib build and enable for CI fix(developer): reconnect --full-test in kmcmplib build and enable for CI Nov 6, 2024
@mcdurdin mcdurdin linked an issue Nov 6, 2024 that may be closed by this pull request
8 tasks
@mcdurdin
Copy link
Member Author

mcdurdin commented Nov 6, 2024

Linux wasm build failed with:

 223/701 anii                                    FAIL            0.08s   exit status 108
 636/701 sil_kmhmu                               FAIL            0.08s   exit status 108
 685/701 naijatype                               FAIL            0.08s   exit status 108

mac wasm build failed with:

 59/701 naijatype                               FAIL            0.08s   exit status 108
  • anii error is due to mismatch in case in bitmap filename. Filename is anii.ico; reference is Anii.ico.
  • sil_kmhmu error is due to mismatch in case in bitmap filename. Filename is sIL_kmhmu.ico; reference is sil_kmhmu.ico.
  • naijatype error is due to backslashes in bitmap filename. Resolved in 3c680e3

mac arch build failed with:

121/701 fv_statimcets    FAIL    exit status 108
> fv_statimcets.kmn(213): SevError | 0x00a
128/701 fv_nuucaanul     FAIL    exit status 108
> fv_nuucaanul.kmn(147): SevError | 0x00a

0x00a is Invalid Token. Turns out that those lines both have a U+2002 at the end, which in WASM and VC++ builds is trimmed by u16trim() and friends (via iswspace()), but it seems in mac arch build, is not.

# mainline branch test, or as a release build
#
builder_is_ci_build() {
if builder_is_ci_release_build || builder_is_ci_test_build; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do some build configs use a --ci flag?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They do at present. We'll move away from that in the future, because it should be unnecessary. Similarly, web and iOS have a ci.sh which is just different to everything else (there's quite a lot of stuff in the web build scripts which has diverged from the other platforms -- we should be able to simplify). (web scripts also do other tests for CI environments which should be updated to use these functions in the future)

Those changes can wait until 19.0, however.

While kmc did this already in its callbacks, the unit test callbacks
did not, which meant that some keyboard tests failed on Linux and macOS.

Relates-to: #12623
…hitespace management

On mac arch build, kmcmplib is failing to trim U+2002, unlike all other
platforms. Using UTF-8 locale should treat the ISO 30112 POSIX space
characters, which includes U+2002, as whitespace.
anii.kmn and sil_kmhmu.kmn both have mismatching case in filename
references for icons. For now, we will disable tests for these two
keyboards, so that tests pass on Linux (which has case-sensitive
filesystem).

Note: the filename case was already addressed in the keyboards repo, so
when we realign to a more recent commit for the test fixtures, we should
be able to include these tests again.

Relates-to: #12623
@mcdurdin
Copy link
Member Author

mcdurdin commented Nov 6, 2024

Follow-up disabled tests in 19.0 in #12634.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

bug(developer): kmcmplib does not run a full keyboard repository test
2 participants