From 65b7b20dc8645a5e1810e00a807dfc740acc37f0 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Tue, 14 Nov 2023 06:24:28 +0700 Subject: [PATCH] chore(developer): handle project version checks cleanly in kmc Relates to #9948. While the schema validation does check the version already, it gives a fairly obtuse message, and we can do better. Found a secondary issue with a iterator type guard needed in kpj-file-reader when preparing unit test, which was probably masked in other tests by not verifying all messages generated, so updated all infrastructure message tests to verify the full set of messages generated. --- common/web/types/src/kpj/kpj-file-reader.ts | 10 +++-- .../src/messages/infrastructureMessages.ts | 6 ++- developer/src/kmc/src/util/projectLoader.ts | 4 ++ .../error_unsupported_project_version.kpj | 13 +++++++ .../kmc/test/test-infrastructureMessages.ts | 38 ++++++++++++++----- 5 files changed, 56 insertions(+), 15 deletions(-) create mode 100644 developer/src/kmc/test/fixtures/invalid-projects/error_unsupported_project_version.kpj diff --git a/common/web/types/src/kpj/kpj-file-reader.ts b/common/web/types/src/kpj/kpj-file-reader.ts index 72b6da3b0f5..29ffe8009d9 100644 --- a/common/web/types/src/kpj/kpj-file-reader.ts +++ b/common/web/types/src/kpj/kpj-file-reader.ts @@ -28,10 +28,12 @@ export class KPJFileReader { data = r as KPJFile; }); data = this.boxArrays(data); - for(let file of data.KeymanDeveloperProject?.Files?.File) { - // xml2js imports
as '' so we will just delete the empty string - if(typeof file.Details == 'string') { - delete file.Details; + if(data.KeymanDeveloperProject?.Files?.File?.length) { + for(let file of data.KeymanDeveloperProject?.Files?.File) { + // xml2js imports
as '' so we will just delete the empty string + if(typeof file.Details == 'string') { + delete file.Details; + } } } return data as KPJFile; diff --git a/developer/src/kmc/src/messages/infrastructureMessages.ts b/developer/src/kmc/src/messages/infrastructureMessages.ts index f291c5b4cf7..50334a6fd64 100644 --- a/developer/src/kmc/src/messages/infrastructureMessages.ts +++ b/developer/src/kmc/src/messages/infrastructureMessages.ts @@ -81,7 +81,11 @@ export class InfrastructureMessages { static ERROR_CannotCreateFolder = SevError | 0x0011; static Error_InvalidProjectFolder = (o:{folderName:string}) => m(this.ERROR_InvalidProjectFolder, - `The folder ${o.folderName} does not appear to be a Keyman Developer project`); + `The folder ${o.folderName} does not appear to be a Keyman Developer project.`); static ERROR_InvalidProjectFolder = SevError | 0x0012; + + static Error_UnsupportedProjectVersion = (o:{version:string}) => m(this.ERROR_UnsupportedProjectVersion, + `Project version ${o.version} is not supported by this version of Keyman Developer.`); + static ERROR_UnsupportedProjectVersion = SevError | 0x0013; } diff --git a/developer/src/kmc/src/util/projectLoader.ts b/developer/src/kmc/src/util/projectLoader.ts index 10cd58fc818..b26399ee395 100644 --- a/developer/src/kmc/src/util/projectLoader.ts +++ b/developer/src/kmc/src/util/projectLoader.ts @@ -47,6 +47,10 @@ function loadProjectFromFile(infile: string, callbacks: CompilerCallbacks): Keym let kpj = null; try { kpj = reader.read(kpjData); + if(kpj.KeymanDeveloperProject?.Options?.Version && kpj.KeymanDeveloperProject.Options.Version != "1.0" && kpj.KeymanDeveloperProject.Options.Version != "2.0") { + callbacks.reportMessage(InfrastructureMessages.Error_UnsupportedProjectVersion({version: kpj.KeymanDeveloperProject.Options.Version})); + return null; + } reader.validate(kpj); } catch(e) { callbacks.reportMessage(InfrastructureMessages.Error_InvalidProjectFile({message: (e??'').toString()})); diff --git a/developer/src/kmc/test/fixtures/invalid-projects/error_unsupported_project_version.kpj b/developer/src/kmc/test/fixtures/invalid-projects/error_unsupported_project_version.kpj new file mode 100644 index 00000000000..2c97588f0a7 --- /dev/null +++ b/developer/src/kmc/test/fixtures/invalid-projects/error_unsupported_project_version.kpj @@ -0,0 +1,13 @@ + + + + $PROJECTPATH\build + $PROJECTPATH\source + True + True + False + keyboard + + 3.0 + + diff --git a/developer/src/kmc/test/test-infrastructureMessages.ts b/developer/src/kmc/test/test-infrastructureMessages.ts index fde352f3dd3..6d543a5fd7c 100644 --- a/developer/src/kmc/test/test-infrastructureMessages.ts +++ b/developer/src/kmc/test/test-infrastructureMessages.ts @@ -4,7 +4,7 @@ import { InfrastructureMessages } from '../src/messages/infrastructureMessages.j import { verifyCompilerMessagesObject } from '@keymanapp/developer-test-helpers'; import { makePathToFixture } from './helpers/index.js'; import { NodeCompilerCallbacks } from '../src/util/NodeCompilerCallbacks.js'; -import { CompilerErrorNamespace } from '@keymanapp/common-types'; +import { CompilerErrorNamespace, CompilerEvent } from '@keymanapp/common-types'; import { unitTestEndpoints } from '../src/commands/build.js'; import { KmnCompilerMessages } from '@keymanapp/kmc-kmn'; @@ -21,12 +21,13 @@ describe('InfrastructureMessages', function () { it('should generate FATAL_UnexpectedException if an exception is raised', async function() { const ncb = new NodeCompilerCallbacks({logLevel: 'silent'}); + const expectedMessages = [InfrastructureMessages.FATAL_UnexpectedException]; + process.env.SENTRY_CLIENT_TEST_BUILD_EXCEPTION = '1'; await unitTestEndpoints.build(null, ncb, {}); delete process.env.SENTRY_CLIENT_TEST_BUILD_EXCEPTION; - assert.isTrue(ncb.hasMessage(InfrastructureMessages.FATAL_UnexpectedException), - `FATAL_UnexpectedException not generated, instead got: `+JSON.stringify(ncb.messages,null,2)); - assert.lengthOf(ncb.messages, 1); + + assertMessagesEqual(ncb.messages, expectedMessages); assert.instanceOf(ncb.messages[0].exceptionVar, Error); }); @@ -62,15 +63,14 @@ describe('InfrastructureMessages', function () { // This message is generated by NodeCompilerCallbacks, because that's where the filesystem is visible, // so we can't use our usual testForMessage pattern. const ncb = new NodeCompilerCallbacks({logLevel: 'silent'}); + const expectedMessages = [InfrastructureMessages.HINT_FilenameHasDifferingCase]; ncb.loadFile(makePathToFixture('invalid-keyboards', 'Hint_Filename_Has_Differing_Case.kmn')); - assert.isTrue(ncb.hasMessage(InfrastructureMessages.HINT_FilenameHasDifferingCase), - `HINT_FilenameHasDifferingCase not generated, instead got: `+JSON.stringify(ncb.messages,null,2)); + assertMessagesEqual(ncb.messages, expectedMessages); }); // INFO_WarningsHaveFailedBuild it('should generate INFO_WarningsHaveFailedBuild if only warnings failed the build', async function() { - // NOTE: we can probably re-use this format for most other message tests in the future const ncb = new NodeCompilerCallbacks({logLevel: 'silent'}); const filename = makePathToFixture('compiler-warnings-as-errors', 'keyboard.kmn'); const expectedMessages = [ @@ -80,8 +80,26 @@ describe('InfrastructureMessages', function () { InfrastructureMessages.INFO_FileNotBuiltSuccessfully ]; await unitTestEndpoints.build(filename, ncb, {compilerWarningsAsErrors: true}); - assert.deepEqual(ncb.messages.map(m => m.code), expectedMessages, - `actual callbacks.messages:\n${JSON.stringify(ncb.messages,null,2)}\n\n`+ - `did not match expected:\n${JSON.stringify(expectedMessages,null,2)}\n\n`); + assertMessagesEqual(ncb.messages, expectedMessages); + }); + + // ERROR_UnsupportedProjectVersion + + it('should generate ERROR_UnsupportedProjectVersion if a project file with an unsupported version is loaded', async function() { + const ncb = new NodeCompilerCallbacks({logLevel: 'silent'}); + const filename = makePathToFixture('invalid-projects', 'error_unsupported_project_version.kpj'); + const expectedMessages = [ + InfrastructureMessages.INFO_BuildingFile, + InfrastructureMessages.ERROR_UnsupportedProjectVersion, + InfrastructureMessages.INFO_ProjectNotBuiltSuccessfully + ]; + await unitTestEndpoints.build(filename, ncb, {compilerWarningsAsErrors: true}); + assertMessagesEqual(ncb.messages, expectedMessages); }); }); + +function assertMessagesEqual(actualMessages: CompilerEvent[], expectedMessages: number[]) { + assert.deepEqual(actualMessages.map(m => m.code), expectedMessages, + `actual callbacks.messages:\n${JSON.stringify(actualMessages,null,2)}\n\n`+ + `did not match expected:\n${JSON.stringify(expectedMessages,null,2)}\n\n`); +} \ No newline at end of file