keymanapp
chore(common): additional coverage for common/web/types
TODO: coverage for these files.
Originally posted by @mcdurdin in https://github.com/keymanapp/keyman/pull/9048#discussion_r1236143014
"src/kmx/kmx-plus-builder/",
"src/kmx/kmx-plus.ts",
"src/kmx/kmx-builder.ts",
"src/kmx/element-string.ts",
"src/kmx/string-list.ts",
"src/ldml-keyboard/ldml-keyboard-testdata-xml.ts",
"src/ldml-keyboard/unicodeset-parser-api.ts",
"src/keyman-touch-layout/keyman-touch-layout-file-writer.ts",
"src/osk/osk.ts",
the challenge is that importing kmx-plus.ts in common/web/types tests drops coverage by 15%, under the threshold. So I can't add tests incrementally without introducing failures.
Copying a thread from Slack for posterity, with some edits.
Now that I have merged master into feature-kmc-kmw, both kmxplus and kmc-kmw projects have been adding to common/web/types, and now feature-kmc-kmw is failing the coverage test:
| File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s |
|---|---|---|---|---|---|
| All files | 69.77 | 72.01 | 52.08 | 69.77 | |
| src/keyman-touch-layout | 77.65 | 96.42 | 75 | 77.65 | |
| keyman-touch-layout-file-writer.ts | 41.9 | 80 | 66.66 | 41.9 | 42-43,46-104 |
| src/kmx | 42.41 | 34.32 | 22.64 | 42.41 | |
| element-string.ts | 24.05 | 50 | 0 | 24.05 | 13-23,28-66,68-77 |
| kmx-builder.ts | 2.64 | 0 | 0 | 2.64 | 7-227 |
| kmx-file-reader.ts | 82.85 | 66.66 | 85.71 | 82.85 | ...50,59-60,92-93 |
| kmx-plus.ts | 19.24 | 6.45 | 0 | 19.24 | ...39-483,505-847 |
| kmx.ts | 97.79 | 54.54 | 100 | 97.79 | ...19-420,450-451 |
| string-list.ts | 33.78 | 0 | 0 | 33.78 | ...54,56-70,72-73 |
| src/kmx/kmx-plus-builder | 40.93 | 0 | 0 | 40.93 | |
| build-disp.ts | 52.17 | 100 | 0 | 52.17 | 25-46 |
| build-elem.ts | 33.98 | 100 | 0 | 33.98 | ...2,45-91,94-103 |
| build-keys.ts | 40.57 | 100 | 0 | 40.57 | 72-175 |
| build-layr.ts | 41.55 | 100 | 0 | 41.55 | 65-154 |
| build-list.ts | 44.21 | 100 | 0 | 44.21 | 35-77,86-95 |
| build-loca.ts | 57.57 | 100 | 0 | 57.57 | 20-33 |
| build-meta.ts | 64.86 | 100 | 0 | 64.86 | 25-37 |
| build-name.ts | 51.35 | 100 | 0 | 51.35 | 20-37 |
| build-sect.ts | 70.96 | 100 | 0 | 70.96 | 23-31 |
| build-strs.ts | 46.55 | 100 | 0 | 46.55 | 26-46,49-58 |
| build-tran.ts | 42 | 100 | 0 | 42 | 43-100 |
| build-vars.ts | 37.7 | 100 | 0 | 37.7 | 24-61 |
| build-vkey.ts | 51.16 | 100 | 0 | 51.16 | 23-43 |
| kmx-plus-builder.ts | 21.62 | 0 | 0 | 21.62 | 41-185 |
| src/kpj | 77.17 | 60 | 66.66 | 77.17 | |
| keyman-developer-project.ts | 58.94 | 58.33 | 46.15 | 58.94 | ...42-145,148-151 |
| kpj-file-reader.ts | 91.66 | 60.71 | 100 | 91.66 | ...52-53,62,68-69 |
| src/kvk | 92.47 | 62.5 | 84.21 | 92.47 | |
| kvk-file-reader.ts | 94.11 | 66.66 | 100 | 94.11 | 11-12 |
| kvks-file-reader.ts | 89.63 | 65.62 | 100 | 89.63 | ...17-121,162-163 |
| kvks-file-writer.ts | 90.17 | 50 | 100 | 90.17 | ...,88-90,110-111 |
| visual-keyboard.ts | 77.77 | 25 | 25 | 77.77 | 17-23,26-30,37-42 |
| src/ldml-keyboard | 91.61 | 78.49 | 78.57 | 91.61 | |
| ldml-keyboard-xml-reader.ts | 89.69 | 77.9 | 94.73 | 89.69 | ...12-314,393-394 |
| pattern-parser.ts | 93.38 | 100 | 50 | 93.38 | ...10-111,119-120 |
| unicodeset-parser-api.ts | 48 | 0 | 0 | 48 | 13-25 |
| src/util | 88.17 | 95.45 | 47.82 | 88.17 | |
| common-events.ts | 94 | 100 | 66.66 | 94 | 38,43,47 |
| compiler-interfaces.ts | 88.53 | 100 | 16.66 | 88.53 | ...,55-63,227-228 |
| file-types.ts | 79.5 | 100 | 0 | 79.5 | ...02-107,117-122 |
| util.ts | 97.05 | 92.3 | 100 | 97.05 | 63-64 |
| test/helpers | 83.48 | 80 | 78.57 | 83.48 | |
| TestCompilerCallbacks.ts | 72.58 | 87.5 | 66.66 | 72.58 | ...29,38-39,53-54 |
| index.ts | 97.87 | 71.42 | 100 | 97.87 | 39 |
| --------------------------------------- | --------- | ---------- | --------- | --------- | ------------------- |
| ERROR: Coverage for lines (69.77%) does not meet global threshold (90%) |
marc: I'll go ahead and add some coverage tests for the bits I've added but we're going to need to do the same for the kmxplus compiler.
marc: We also should exclude test/helpers from coverage, not that that will help much
srl: build-disp.ts build-elem.ts build-keys.ts build-layr.ts build-list.ts build-loca.ts build-meta.ts build-name.ts build-sect.ts build-strs.ts build-tran.ts build-vars.ts build-vkey.ts
srl: well … the real exercisers for those are in kmc-ldml…
marc: perhaps that's where they belong?
marc: We could move the entire src/kmx/kmx-plus-builder into kmc-ldml
srl: common has the XML side, and the in-memory side, AND the binary side; kmc-ldml has the compiler
marc: I need to review the boundaries
srl: i’m trying to remember why it was in common before
srl: if you compiled basic.xml within common/web/types… that would hit a bunch
srl: src/kmx/kmx-plus-builder, wondering about excluding those from coverage for now
srl: added /* c8 ignore start */ to common/web/types/src/kmx/kmx-builder.ts common/web/types/src/kmx/kmx-plus-builder/build-disp.ts common/web/types/src/kmx/kmx-plus-builder/build-elem.ts common/web/types/src/kmx/kmx-plus-builder/build-keys.ts common/web/types/src/kmx/kmx-plus-builder/build-layr.ts common/web/types/src/kmx/kmx-plus-builder/build-list.ts common/web/types/src/kmx/kmx-plus-builder/build-loca.ts common/web/types/src/kmx/kmx-plus-builder/build-meta.ts common/web/types/src/kmx/kmx-plus-builder/build-name.ts common/web/types/src/kmx/kmx-plus-builder/build-sect.ts common/web/types/src/kmx/kmx-plus-builder/build-strs.ts common/web/types/src/kmx/kmx-plus-builder/build-tran.ts common/web/types/src/kmx/kmx-plus-builder/build-vars.ts common/web/types/src/kmx/kmx-plus-builder/build-vkey.ts common/web/types/src/kmx/kmx-plus-builder/kmx-plus-builder.ts common/web/types/src/kmx/kmx-plus.ts => 93% line cov
srl: but anyway moving to kmc-ldml probably makes sense
marc: common/web tests run in one CI configuration, and developer tests run in another, and I don't want to combine them.
- Another reason: I think it is important for common libraries to be able to build and be tested on their own in isolation (it's seriously not ideal that we have tests in Common (but run under Web's CI, yeuch, more chaos) that depend on Developer right now -- that's a nasty dependency that it'd be best to clean up; thinking here of LMLayer tests that depend on the lexical model compiler).
- More pragmatically, the common libraries can run on CI on multiple platforms while Developer still only runs on Windows for CI. So given what I've just said, I think (and I don't think this is new discussion):
- I want to move /common/predictive-text/ to /web/ -- just keeping /common/models/ where they are because those modules are shared between web and developer's lm compiler. (also /common/test/predictive-text/ should move over to /web/ with it)
- /common/web/input-processor, keyboard-processor, lm-worker, recorder, sentry-manager should move from /common/web/ to /web/. (They aren't common/web -- test this by asking which code imports them -- and it's only /web/ -- other users are importing /web/, not these modules directly)
- I want to move kmxbuilder from common to developer because it's really a compilation process. The raw readers and writers for the .kmx files belong in common/web/types.
- I know the kmxbuilder move is on a fuzzy boundary -- we've got the transforms from .kvks to .kvk in common/web/types as well, for example, and perhaps those should be refactored into Developer modules as well -- I'm open to that discussion.
- In fact, perhaps ideally all the source readers/writers should be in /developer/common/web/types/ and the binary readers/writers should be in /common/web/types/? None of our end user products should be importing source files so it would make sense to restructure common/web/types/ accordingly. Deck chairs on the Titanic perhaps?
- example of kicking this can down the road in https://github.com/keymanapp/keyman/pull/9374/commits/4aacfbf737762f4ae9095677d1a9227bb8df268c