Skip to content

Commit

Permalink
feat(web): improve errors and tests
Browse files Browse the repository at this point in the history
Addresses code review comments.
  • Loading branch information
ermshiperete committed Sep 16, 2024
1 parent 1dd6989 commit 94b06b7
Show file tree
Hide file tree
Showing 14 changed files with 244 additions and 136 deletions.
4 changes: 4 additions & 0 deletions web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@
"./engine/osk/internals": {
"types": "./build/engine/osk/obj/test-index.d.ts",
"import": "./build/engine/osk/obj/test-index.js"
},
"./tools/testing/test-utils": {
"types": "./build/tools/testing/test-utils/obj/index.d.ts",
"import": "./build/tools/testing/test-utils/obj/index.js"
}
},
"imports": {
Expand Down
9 changes: 7 additions & 2 deletions web/src/engine/keyboard/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function do_configure() {

BUILD_DIR="${KEYMAN_ROOT}/web/build/${SUBPROJECT_NAME}"

function do_build() {
do_build() {
tsc --build "${THIS_SCRIPT_PATH}/tsconfig.all.json"

# Base product - the main keyboard processor
Expand Down Expand Up @@ -73,7 +73,12 @@ function do_build() {
tsc --emitDeclarationOnly --outFile "${BUILD_DIR}/lib/node-keyboard-loader.d.ts" -p src/keyboards/loaders/tsconfig.node.json
}

do_test() {
test-headless "${SUBPROJECT_NAME}" ""
test-headless-typescript "${SUBPROJECT_NAME}"
}

builder_run_action configure do_configure
builder_run_action clean rm -rf "${BUILD_DIR}"
builder_run_action build do_build
builder_run_action test test-headless "${SUBPROJECT_NAME}" ""
builder_run_action test do_test
2 changes: 1 addition & 1 deletion web/src/engine/keyboard/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export { default as Keyboard } from "./keyboards/keyboard.js";
export * from "./keyboards/keyboard.js";
export { KeyboardHarness, KeyboardKeymanGlobal, MinimalCodesInterface, MinimalKeymanGlobal } from "./keyboards/keyboardHarness.js";
export { KeyboardLoaderBase } from "./keyboards/keyboardLoaderBase.js";
export { KeyboardLoadErrorBuilder, KeyboardMissingError, KeyboardScriptError, KeyboardDownloadError } from './keyboards/keyboardLoadError.js'
export { KeyboardLoadErrorBuilder, KeyboardMissingError, KeyboardScriptError, KeyboardDownloadError, InvalidKeyboardError } from './keyboards/keyboardLoadError.js'
export {
CloudKeyboardFont,
internalizeFont,
Expand Down
146 changes: 80 additions & 66 deletions web/src/engine/keyboard/src/keyboards/keyboardLoadError.ts
Original file line number Diff line number Diff line change
@@ -1,90 +1,104 @@
import { type KeyboardStub } from './keyboardLoaderBase.js';

export interface KeyboardLoadErrorBuilder {
scriptError(err?: Error): void;
missingError(err: Error): void;
missingKeyboardError(msg: string, err: Error): void;
keyboardDownloadError(msg: string, err: Error): void;
scriptError(err?: Error): void;
missingError(err: Error): void;
keyboardDownloadError(err: Error): void;

invalidKeyboard(err: Error): void;
}

export class KeyboardScriptError extends Error {
public readonly cause;
public readonly cause;

constructor(msg: string, cause?: Error) {
super(msg);
this.cause = cause;
}
constructor(msg: string, cause?: Error) {
super(msg);
this.cause = cause;
}
}

export class KeyboardMissingError extends Error {
public readonly cause;
public readonly cause;

constructor(msg: string, cause?: Error) {
super(msg);
this.cause = cause;
}
constructor(msg: string, cause?: Error) {
super(msg);
this.cause = cause;
}
}

export class KeyboardDownloadError extends Error {
public readonly cause;
public readonly cause;

constructor(message: string, cause?: Error) {
super(message);
this.cause = cause;
}
constructor(message: string, cause?: Error) {
super(message);
this.cause = cause;
}
}

export class UriBasedErrorBuilder implements KeyboardLoadErrorBuilder {
readonly uri: string;

constructor(uri: string) {
this.uri = uri;
}

missingError(err: Error) {
const msg = `Cannot find the keyboard at ${this.uri}.`;
return new KeyboardMissingError(msg, err);
}
export class InvalidKeyboardError extends Error {
public readonly cause;

missingKeyboardError(msg: string, err: Error) {
return new KeyboardMissingError(msg, err);
}

scriptError(err: Error) {
const msg = `Error registering the keyboard script at ${this.uri}; it may contain an error.`;
return new KeyboardScriptError(msg, err);
}
constructor(message: string, cause?: Error) {
super(message);
this.cause = cause;
}
}

keyboardDownloadError(msg: string, err: Error) {
return new KeyboardDownloadError(msg, err);
}
export class UriBasedErrorBuilder implements KeyboardLoadErrorBuilder {
readonly uri: string;

constructor(uri: string) {
this.uri = uri;
}

missingError(err: Error) {
const msg = `Cannot find the keyboard at ${this.uri}.`;
return new KeyboardMissingError(msg, err);
}

scriptError(err: Error) {
const msg = `Error registering the keyboard script at ${this.uri}; it may contain an error.`;
return new KeyboardScriptError(msg, err);
}

keyboardDownloadError(err: Error) {
const msg = `Unable to download keyboard at ${this.uri}`;
return new KeyboardDownloadError(msg, err);
}

invalidKeyboard(err: Error) {
const msg = `${this.uri} is not a valid keyboard file`;
return new InvalidKeyboardError(msg, err);
}
}

export class StubBasedErrorBuilder implements KeyboardLoadErrorBuilder {
readonly stub: KeyboardStub;

constructor(stub: KeyboardStub) {
this.stub = stub;
}

missingError(err: Error) {
const stub = this.stub;
const msg = `Cannot find the ${stub.name} keyboard for ${stub.langName} at ${stub.filename}.`;
return new KeyboardMissingError(msg, err);
}

missingKeyboardError(msg: string, err: Error) {
return new KeyboardMissingError(msg, err);
}

scriptError(err: Error) {
const stub = this.stub;
const msg = `Error registering the ${stub.name} keyboard for ${stub.langName}; keyboard script at ${stub.filename} may contain an error.`;
return new KeyboardScriptError(msg, err);
}

keyboardDownloadError(msg: string, err: Error) {
return new KeyboardDownloadError(msg, err);
}
readonly stub: KeyboardStub;

constructor(stub: KeyboardStub) {
this.stub = stub;
}

missingError(err: Error) {
const stub = this.stub;
const msg = `Cannot find the ${stub.name} keyboard for ${stub.langName} at ${stub.filename}.`;
return new KeyboardMissingError(msg, err);
}

scriptError(err: Error) {
const stub = this.stub;
const msg = `Error registering the ${stub.name} keyboard for ${stub.langName}; keyboard script at ${stub.filename} may contain an error.`;
return new KeyboardScriptError(msg, err);
}

keyboardDownloadError(err: Error) {
const msg = `Unable to download ${this.stub.name} keyboard for ${this.stub.langName}`;
return new KeyboardDownloadError(msg, err);
}

invalidKeyboard(err: Error) {
const msg = `${this.stub.name} is not a valid keyboard`;
return new InvalidKeyboardError(msg, err);
}
}

6 changes: 3 additions & 3 deletions web/src/engine/keyboard/src/keyboards/keyboardLoaderBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export abstract class KeyboardLoaderBase {
* @param uri The URI of the keyboard to load.
* @returns A Promise that resolves to the loaded keyboard.
*/
public loadKeyboardFromPath(uri: string): Promise<Keyboard> {
public async loadKeyboardFromPath(uri: string): Promise<Keyboard> {
this.harness.install();
return this.loadKeyboardInternal(uri, new UriBasedErrorBuilder(uri));
}
Expand All @@ -33,7 +33,7 @@ export abstract class KeyboardLoaderBase {
* @param stub The stub of the keyboard to load.
* @returns A Promise that resolves to the loaded keyboard.
*/
public loadKeyboardFromStub(stub: KeyboardStub) {
public async loadKeyboardFromStub(stub: KeyboardStub): Promise<Keyboard> {
this.harness.install();
return this.loadKeyboardInternal(stub.filename, new StubBasedErrorBuilder(stub));
}
Expand All @@ -45,7 +45,7 @@ export abstract class KeyboardLoaderBase {
try {
script = await blob.text();
} catch (e) {
throw errorBuilder.missingKeyboardError('The keyboard has an invalid encoding.', e);
throw errorBuilder.invalidKeyboard(e);
}

if (script.startsWith('KXTS', 0)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,17 @@ export class DOMKeyboardLoader extends KeyboardLoaderBase {
try {
response = await fetch(uri);
} catch (e) {
throw errorBuilder.keyboardDownloadError(`Unable to fetch fetch keyboard at ${uri}`, e);
throw errorBuilder.keyboardDownloadError(e);
}

if (!response.ok) {
throw errorBuilder.missingError(new Error(`HTTP ${response.status} ${response.statusText}`));
throw errorBuilder.keyboardDownloadError(new Error(`HTTP ${response.status} ${response.statusText}`));
}

try {
return await response.blob();
} catch (e) {
throw errorBuilder.keyboardDownloadError(`Unable to retrieve blob from keyboard at ${uri}`, e);
throw errorBuilder.invalidKeyboard(e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export class NodeKeyboardLoader extends KeyboardLoaderBase {
try {
buffer = await readFile(uri);
} catch (err) {
throw errorBuilder.keyboardDownloadError(`Unable to read keyboard file at ${uri}`, err);
throw errorBuilder.keyboardDownloadError(err);
}
return new Blob([buffer]);
}
Expand All @@ -46,7 +46,7 @@ export class NodeKeyboardLoader extends KeyboardLoaderBase {
try {
script = new vm.Script(scriptSrc);
} catch (err) {
throw errorBuilder.missingError(err);
throw errorBuilder.invalidKeyboard(err);
}
try {
script.runInContext(this.harness._jsGlobal);
Expand Down
21 changes: 20 additions & 1 deletion web/src/test/auto/dom/cases/keyboard/domKeyboardLoader.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { assert } from 'chai';

import { DOMKeyboardLoader } from 'keyman/engine/keyboard/dom-keyboard-loader';
import { extendString, KeyboardHarness, Keyboard, MinimalKeymanGlobal, DeviceSpec, KeyboardKeymanGlobal } from 'keyman/engine/keyboard';
import { extendString, KeyboardHarness, Keyboard, MinimalKeymanGlobal, DeviceSpec, KeyboardKeymanGlobal, KeyboardDownloadError, KeyboardScriptError } from 'keyman/engine/keyboard';
import { KeyboardInterface, Mock } from 'keyman/engine/js-processor';
import { assertThrowsAsync } from 'keyman/tools/testing/test-utils';

declare let window: typeof globalThis;
// KeymanEngine from the web/ folder... when available.
Expand All @@ -27,6 +28,24 @@ describe('Keyboard loading in DOM', function() {
}
})

it('throws error when keyboard does not exist', async () => {
const harness = new KeyboardInterface(window, MinimalKeymanGlobal);
const keyboardLoader = new DOMKeyboardLoader(harness);
const nonExisting = '/does/not/exist.js';

await assertThrowsAsync(async () => await keyboardLoader.loadKeyboardFromPath(nonExisting),
KeyboardDownloadError, `Unable to download keyboard at ${nonExisting}`);
});

it('throws error when keyboard is invalid', async () => {
const harness = new KeyboardInterface(window, MinimalKeymanGlobal);
const keyboardLoader = new DOMKeyboardLoader(harness);
const nonKeyboardPath = '/common/test/resources/index.mjs';

await assertThrowsAsync(async () => await keyboardLoader.loadKeyboardFromPath(nonKeyboardPath),
KeyboardScriptError, `Error registering the keyboard script at ${nonKeyboardPath}; it may contain an error.`);
});

it('`window`, disabled rule processing', async () => {
const harness = new KeyboardHarness(window, MinimalKeymanGlobal);
let keyboardLoader = new DOMKeyboardLoader(harness);
Expand Down
34 changes: 34 additions & 0 deletions web/src/test/auto/headless/engine/keyboard/keyboard.tests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { assert } from 'chai';

import { createRequire } from 'module';
const require = createRequire(import.meta.url);

import { KeyboardHarness, MinimalKeymanGlobal, DeviceSpec } from 'keyman/engine/keyboard';
import { NodeKeyboardLoader } from 'keyman/engine/keyboard/node-keyboard-loader';


describe('Keyboard tests', function () {
const khmerPath = require.resolve('@keymanapp/common-test-resources/keyboards/khmer_angkor.js');

it('accurately determines layout properties', async () => {
// -- START: Standard Recorder-based unit test loading boilerplate --
const harness = new KeyboardHarness({}, MinimalKeymanGlobal);
const keyboardLoader = new NodeKeyboardLoader(harness);
const km_keyboard = await keyboardLoader.loadKeyboardFromPath(khmerPath);
// -- END: Standard Recorder-based unit test loading boilerplate --

// `khmer_angkor` - supports longpresses, but not flicks or multitaps.

// Phone supports longpress if the keyboard supports it.
const mobileLayout = km_keyboard.layout(DeviceSpec.FormFactor.Phone);
assert.isTrue(mobileLayout.hasLongpresses);
assert.isFalse(mobileLayout.hasFlicks);
assert.isFalse(mobileLayout.hasMultitaps);

// Desktop doesn't support longpress even if the keyboard supports it.
const desktopLayout = km_keyboard.layout(DeviceSpec.FormFactor.Desktop);
assert.isFalse(desktopLayout.hasLongpresses);
assert.isFalse(desktopLayout.hasFlicks);
assert.isFalse(desktopLayout.hasMultitaps);
});
});
Loading

0 comments on commit 94b06b7

Please sign in to comment.