Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(developer) kmn compiler messages unit tests #11284

Merged
merged 10 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ group(main) using keys
c maximum store length is 4096 UTF-16 code units, including U+0000 terminator
c #define GLOBAL_BUFSIZE 4096 // compfile.h
c so we need 0x101E - 0x0020 + 1 = 0x0FFF --> 4095 words
c See #11136 for calculation adjustment TODO

store(x) U+0020 .. U+101E

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ group(main) using keys
c
c maximum store length is 4096 UTF-16 code units, including U+0000 terminator
c #define GLOBAL_BUFSIZE 4096 // compfile.h
c so we need 0x101B - 0x0020 + 1 = 0x0FFD --> 4092 words, + 4 = 4096 = too long
c See #11136 for calculation adjustment TODO
c so we need 0x101B - 0x0020 + 1 = 0x0FFC --> 4092 words, + 4 = 4096 = too long

store(x) U+0020 .. U+101B

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@ group(main) using keys

c maximum store length is 4096 UTF-16 code units, including U+0000 terminator
c #define GLOBAL_BUFSIZE 4096 // compfile.h
c so we need 0x101C - 0x0020 + 1 = 0x0FFD --> 4093 words
c + 1, for 'a' in the rule below = 4094, which triggers the buffer boundary check.
c Noting that this is conservative and losing 2 possible chars, but not fixing
c so we need 0x101D - 0x0020 + 1 = 0x0FFE --> 4094 words
c + 1, for 'a' in the rule below = 4095, which triggers the buffer boundary check.
c Noting that this is conservative and losing 1 possible char, but not fixing
c in compiler.cpp at this time.
c See #11136 for calculation adjustment TODO

store(x) U+0020 .. U+101C
store(x) U+0020 .. U+101D

'a' outs(x) + 'x' > 'x' context
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ group(main) using keys

c maximum store length is 4096 UTF-16 code units, including U+0000 terminator
c #define GLOBAL_BUFSIZE 4096 // compfile.h
c so we need 0x101E - 0x0020 + 1 = 0x0FFF --> 4095 words
c each vk is 5 words long UC_SENTINEL CODE_EXTENDED shift key CODE_EXTENDEDEND (some long history here!)
c we start filling the buffer with 4066 words and then the remaining 30 bytes = 6 VKs A-F
c See #11136 for calculation adjustment TODO
c so we need 0x101D - 0x0020 + 1 = 0x0FFE --> 4094 words
c each VK is 5 words long UC_SENTINEL CODE_EXTENDED shift key CODE_EXTENDEDEND (some long history here!)
c we start filling the buffer with 4064 words (0x0FFF - 0x0020 + 1) and then the remaining 30 bytes = 6 VKs A-F

store(x) U+0020 .. U+1000 [K_A] .. [K_F]
store(x) U+0020 .. U+0FFF [K_A] .. [K_F]

any(x) + 'x' > 'x' context
20 changes: 10 additions & 10 deletions developer/src/kmc-kmn/test/test-messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ describe('KmnCompilerMessages', function () {

// ERROR_DuplicateGroup

it('should generate CERR_DuplicateGroup if the kmn contains two groups with the same name', async function() {
it('should generate ERROR_DuplicateGroup if the kmn contains two groups with the same name', async function() {
await testForMessage(this, ['invalid-keyboards', 'error_duplicate_group.kmn'], KmnCompilerMessages.ERROR_DuplicateGroup);
assert.equal(callbacks.messages[0].message, "A group with this name has already been defined. Group 'ខ្មែរ' declared on line 9");
});

// ERROR_DuplicateStore

it('should generate CERR_DuplicateStore if the kmn contains two stores with the same name', async function() {
it('should generate ERROR_DuplicateStore if the kmn contains two stores with the same name', async function() {
await testForMessage(this, ['invalid-keyboards', 'error_duplicate_store.kmn'], KmnCompilerMessages.ERROR_DuplicateStore);
assert.equal(callbacks.messages[0].message, "A store with this name has already been defined. Store 'ខ្មែរ' declared on line 11");
});
Expand Down Expand Up @@ -96,30 +96,30 @@ describe('KmnCompilerMessages', function () {

// ERROR_OutsTooLong

it('should generate ERROR_OutsTooLong if a store referenced in outs() is too long (more than GLOBAL_BUFSIZE elements)', async function() {
it('should generate ERROR_OutsTooLong if a store referenced in outs() is too long and would overflow the buffer', async function() {
await testForMessage(this, ['invalid-keyboards', 'error_outs_too_long.kmn'], KmnCompilerMessages.ERROR_OutsTooLong);
// callbacks.printMessages();
assert.equal(callbacks.messages[0].message, "Store cannot be inserted with outs() as it makes the extended string too long character offset: 5");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we really want to test the message content because adjusting the text of a message should probably not be affecting unit tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, but I limited myself to just correcting the four failing tests. Six other pre-existing tests check the error message, only two do not, so for consistency I replaced commented out code to just print the message with assertions for the four tests I touched. Options include (i) simply removing the assertions on the messages on the four corrected tests, accepting the inconsistency; (ii) removing the assertions on messages from all existing tests as well as the four I corrected; (iii) looking to find a way (not sure how difficult this would be as I am not yet fully familiar with the interaction of the TS wrapper and kmcmplib) to read the messages from kmcmplib and assert these in the TS ... not really necessary in a unit test though ... operating more at the functional level; (iv) look to mock some aspect of the interaction so there is no dependency on the actual message text in kmcmplib.

Given effort vs. return, I am thinking (ii); what do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think (ii) makes sense, yes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

});

// ERROR_ExtendedStringTooLong

it('should generate ERROR_ExtendedStringTooLong if an extended string is too long (more than GLOBAL_BUFSIZE elements)', async function() {
it('should generate ERROR_ExtendedStringTooLong if an extended string is too long and would overflow the buffer', async function() {
await testForMessage(this, ['invalid-keyboards', 'error_extended_string_too_long.kmn'], KmnCompilerMessages.ERROR_ExtendedStringTooLong);
// callbacks.printMessages();
assert.equal(callbacks.messages[0].message, "Extended string is too long character offset: 9");
});

// ERROR_VirtualKeyExpansionTooLong

it('should generate ERROR_VirtualKeyExpansionTooLong if a virtual key expansion is too long (more than GLOBAL_BUFSIZE elements)', async function() {
it('should generate ERROR_VirtualKeyExpansionTooLong if a virtual key expansion is too long and would overflow the buffer', async function() {
await testForMessage(this, ['invalid-keyboards', 'error_virtual_key_expansion_too_long.kmn'], KmnCompilerMessages.ERROR_VirtualKeyExpansionTooLong);
// callbacks.printMessages();
assert.equal(callbacks.messages[0].message, "Virtual key expansion is too large");
});

// ERROR_CharacterRangeTooLong

it('should generate ERROR_CharacterRangeTooLong if a character range would expand to be too long (more than GLOBAL_BUFSIZE elements)', async function() {
it('should generate ERROR_CharacterRangeTooLong if a character range would expand to be too long and would overflow the buffer', async function() {
await testForMessage(this, ['invalid-keyboards', 'error_character_range_too_long.kmn'], KmnCompilerMessages.ERROR_CharacterRangeTooLong);
// callbacks.printMessages();
assert.equal(callbacks.messages[0].message, "Character range is too large and cannot be expanded");
});

});
Loading