test(developer): keyboard info compiler unit tests 2

[markcsinclair]

Tests for the keyboard info compiler

Continues earlier PR #11000

@keymanapp-test-bot skip

[keymanapp-test-bot]

User Test Results

Test specification and instructions

User tests are not required

[markcsinclair]

Additional tests of both supporting methods and the main run() method, including some multi-part (looped) tests. Some further tests are still needed to get full coverage, but the branch is a couple of weeks old now, so time for review.

[mcdurdin]

One question on the platform tests where it seems there is a mismatch between hasJS and web platform? Apart from that LGTM

[mcdurdin]

This doesn't seem right -- if there is no .js file, then it can't target web platforms?

[markcsinclair]

There is a supporting comment in keyboard-ifo-compiler.ts (lines 329-332) that says:

        // In this case, there was no .kmx metadata available. We need to
        // make an assumption that this keyboard is both desktop+mobile web,
        // and if the .js is in the package, that it is mobile native as well,
        // because the targets metadata is not available in the .js.

So, the test reflects the code intent. I will leave the test unchanged, but do you want me to open a fix and change the default assumption (and hence the test) if no platforms are specified? (What would you like the default to be?)

[mcdurdin]

The whole snippet shows:

keyman/developer/src/kmc-keyboard-info/src/keyboard-info-compiler.ts

Lines 327 to 337 in c1b7f00

	if(jsFile) {
	if(platforms.size == 0) {
	// In this case, there was no .kmx metadata available. We need to
	// make an assumption that this keyboard is both desktop+mobile web,
	// and if the .js is in the package, that it is mobile native as well,
	// because the targets metadata is not available in the .js.
	platforms.add('mobileWeb').add('desktopWeb');
	if(kmpJsonData.files.find(file => file.name.match(/\.js$/))) {
	platforms.add('android').add('ios');
	}
	}

So, that has jsFile == true?

[markcsinclair]

Hmm ... more precision needed in my reply! (line numbers from beta)

The test is focused on the setting of the platforms supported if there is a .js file that has been supplied to the compiler via the options.sources in init() (keyboard-info-compiler.ts ln 125, 151) and successfully loaded from disk (ln 179). The test explores several possibilities, including no platform metadata in the .kmx files (keyboard-info-compiler.ts ln 328-337), and whether the presence of various platforms implies mobile or desktop web too (ln 346-351).

The test uses the Khmer Angkor default fixtures, but stubs in two places to force the code down alternate paths over nine test cases. The two stubs are to remove the .js file from the .kps file list produced by the KmpCompiler (keyboard-info-compiler.ts ln 162) stubbed in test-keyboard-info-compiler.ts ln 365-368, 379 and to set the targets from the .kmx file (keyboard-info-compiler.ts ln 185-188) stubbed in test-keyboard-info-compiler.ts ln 369-374, 380.

The choice of element tag hasJsFile is misleading, as all test cases have a .js file, so hasJsFileInKps would be better. I will change this in #11255.

[keyman-server]

Changes in this pull request will be available for download in Keyman version 17.0.312-beta