Skip to content

test(core): clarify output of test_unicode#11572

Merged
srl295 merged 2 commits intomasterfrom
test/core/10183-clarify-unicode-test
Jun 4, 2024
Merged

test(core): clarify output of test_unicode#11572
srl295 merged 2 commits intomasterfrom
test/core/10183-clarify-unicode-test

Conversation

@srl295
Copy link
Copy Markdown
Member

@srl295 srl295 commented May 27, 2024

  • group output by functional area

#10183

Sample output:

----------------------------------- stdout -----------------------------------
= test_all
== load_json loading /Users/srl295/src/keymanapp/keyman/core/build/mac-x86_64/release/tests/unit/ldml/nodeversions.json
== load_json loading /Users/srl295/src/keymanapp/keyman/core/build/mac-x86_64/release/tests/unit/ldml/package.json
= get_block_unicode_ver load /Users/srl295/src/keymanapp/keyman/core/build/mac-x86_64/release/tests/unit/ldml/Blocks.txt
6
== test: test_unicode_versions
ICU Versions:
* 73.1	..linked from C++
* 74.2	..in Node.js

Unicode Versions:
* 15.0	..in ICU linked from C++
* 15.1	..in ICU in Node.js
* 15.1.0	..in Keyman repo Blocks.txt

Node.js
* "18.20.2"	Actual version of Node.js
* ^18.x	Version of Node.js requested by package.json

All OK!

@keymanapp-test-bot skip

- group output by functional area

#10183
@srl295 srl295 requested review from mcdurdin and rc-swag as code owners May 27, 2024 21:43
@srl295 srl295 self-assigned this May 27, 2024
@srl295 srl295 requested review from ermshiperete and removed request for mcdurdin and rc-swag May 27, 2024 21:43
@srl295 srl295 added this to the A18S3 milestone May 27, 2024
@keymanapp-test-bot
Copy link
Copy Markdown

keymanapp-test-bot bot commented May 27, 2024

@github-actions github-actions bot added core/ Keyman Core test Any acceptance test issue labels May 27, 2024
@srl295 srl295 requested a review from mcdurdin May 27, 2024 21:44
Comment on lines +90 to +96
std::string result = block_line.substr(prefix.length());
const std::string txt_suffix = ".txt"; // trim off this suffix
auto txt_pos = result.find(txt_suffix, 0);
if (txt_pos != std::string::npos) {
result.resize(txt_pos);
}
return result;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand this change?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Maybe needs a comment! It drops the ".txt" suffix that is present in the original file.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Previous output without this change:

block_unicode_ver 15.1.0.txt

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Gotcha, a comment would be nice to clarify why this is being done.

Comment on lines +90 to +96
std::string result = block_line.substr(prefix.length());
const std::string txt_suffix = ".txt"; // trim off this suffix
auto txt_pos = result.find(txt_suffix, 0);
if (txt_pos != std::string::npos) {
result.resize(txt_pos);
}
return result;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Gotcha, a comment would be nice to clarify why this is being done.

@srl295 srl295 merged commit b7de602 into master Jun 4, 2024
@srl295 srl295 deleted the test/core/10183-clarify-unicode-test branch June 4, 2024 13:44
@keyman-server
Copy link
Copy Markdown
Collaborator

Changes in this pull request will be available for download in Keyman version 18.0.48-alpha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core/ Keyman Core test Any acceptance test issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants