Open mcdurdin opened 1 year ago
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.
TODO: coverage for these files.
_Originally posted by @mcdurdin in https://github.com/keymanapp/keyman/pull/9048#discussion_r1236143014_