Skip to content

Commit

Permalink
chore(developer): handle project version checks cleanly in kmc
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mcdurdin committed Nov 13, 2023
1 parent 4fe5df1 commit 65b7b20
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 15 deletions.
10 changes: 6 additions & 4 deletions common/web/types/src/kpj/kpj-file-reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <Details/> 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 <Details/> as '' so we will just delete the empty string
if(typeof file.Details == 'string') {
delete file.Details;
}
}
}
return data as KPJFile;
Expand Down
6 changes: 5 additions & 1 deletion developer/src/kmc/src/messages/infrastructureMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

4 changes: 4 additions & 0 deletions developer/src/kmc/src/util/projectLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()}));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version="1.0" encoding="utf-8"?>
<KeymanDeveloperProject>
<Options>
<BuildPath>$PROJECTPATH\build</BuildPath>
<SourcePath>$PROJECTPATH\source</SourcePath>
<CompilerWarningsAsErrors>True</CompilerWarningsAsErrors>
<WarnDeprecatedCode>True</WarnDeprecatedCode>
<CheckFilenameConventions>False</CheckFilenameConventions>
<ProjectType>keyboard</ProjectType>
<!-- ERROR_UnsupportedProjectVersion -->
<Version>3.0</Version>
</Options>
</KeymanDeveloperProject>
38 changes: 28 additions & 10 deletions developer/src/kmc/test/test-infrastructureMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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<Error>(ncb.messages[0].exceptionVar, Error);
});

Expand Down Expand Up @@ -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 = [
Expand All @@ -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`);
}

0 comments on commit 65b7b20

Please sign in to comment.