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.

Sign up for GitHub

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

Jump to bottom

chore(core): dx: ldml test improvement, backspace test 🙀 #9759

Merged
merged 8 commits into from
Oct 20, 2023
Merged

chore(core): dx: ldml test improvement, backspace test 🙀 #9759

merged 8 commits into from
Oct 20, 2023

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Oct 13, 2023 •
edited
Loading

  • add a filter option to run just one subtest. I don't try to do anything with the "embedded" (@@keys …) tests but skip them if any filter is set. This is very helpful for running within the debugger.
  • support backspace event, revamp test JSON format
  • backspace transforms not implemented, but a test is ready for them: feat(core): backspace transforms 🙀  #9760

For: #9468

@keymanapp-test-bot skip

@srl295
chore(core): dx: ldml test subselection 🙀
6a62d01 
- add a filter option to run just one subtest
- support backspace event, revamp test JSON format

For: #9468
@srl295 srl295 added this to the A17S23 milestone Oct 13, 2023
@srl295 srl295 self-assigned this Oct 13, 2023
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Oct 13, 2023 •
edited
Loading

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

@github-actions github-actions bot added the chore label Oct 13, 2023
@srl295
feat(core): add backspace test 🙀
55bf064 
- test needs to be "t" not "ta"

For: #9760
@srl295 srl295 changed the title chore(core): dx: ldml test subselection 🙀 chore(core): dx: ldml test improvement, backspace test 🙀 Oct 13, 2023
srl295
srl295 commented Oct 13, 2023
View reviewed changes
core/tests/unit/ldml/keyboards/k_006_backspace-test.xml
<keystroke key="a" />
<keystroke key="b" />
<backspace />
<check result="ta" /> <!-- TODO-LDML: #9760 Should be 't' due to backspace transform -->
Copy link
Member Author

Choose a reason for hiding this comment

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

This test verifies that <backspace /> works at all, #9760 to fix it for transforms

@srl295
Copy link
Member Author

srl295 commented Oct 13, 2023

OK this is a little concerning… https://build.palaso.org/buildConfiguration/Keyman_Test_Common_Mac/414588?hideProblemsFromDependencies=false&hideTestsFromDependencies=false&expandBuildChangesSection=true&expandBuildProblemsSection=true&showLog=414588_726_114&logView=flowAware — it's acting as if common/web/types is out of date. (see the .ts change in this PR)

@mcdurdin
Copy link
Member

it's acting as if common/web/types is out of date.

I'm not really seeing the problem you are? Line 401 returns an object which looks like it has a similar structure to what the previous version of the reader returned, so that would fail the test because it doesn't match the actual fixture.

@mcdurdin mcdurdin modified the milestones: A17S23, A17S24 Oct 15, 2023
@srl295
fix(common): ldml test fix 🙀
f89be2d 
- the wrong test case was updated previously

For: #9468
@srl295
Copy link
Member Author

srl295 commented Oct 16, 2023 •
edited
Loading

OK. User error. There are two test cases which test this two different sets of data. Both are fixed and have comments referencing the other.

srl295 added 2 commits October 16, 2023 10:52
@srl295
fix(common,developer): ldml test fix 🙀
d46e082 
- There are two cases. Added comments to cross-link them.

For: #9468
@srl295
Merge branch 'feat/core/9468-normalization-epic-ldml' into feat/core/…
c3a74dd 
…9468-test-improvement-epic-ldml
mcdurdin
mcdurdin approved these changes Oct 16, 2023
View reviewed changes
Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

LGTM

Base automatically changed from feat/core/9468-normalization-epic-ldml to master October 20, 2023 03:57
@srl295 srl295 merged commit 6047a2c into master Oct 20, 2023
@srl295 srl295 deleted the feat/core/9468-test-improvement-epic-ldml branch October 20, 2023 03:58
@keyman-server
Copy link
Collaborator

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

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

@mcdurdin mcdurdin mcdurdin approved these changes

@darcywong00 darcywong00 Awaiting requested review from darcywong00 darcywong00 is a code owner

@jahorton jahorton Awaiting requested review from jahorton jahorton is a code owner

@rc-swag rc-swag Awaiting requested review from rc-swag rc-swag is a code owner

Assignees

@srl295 srl295

Labels
chore common/web/ common/ core/ Keyman Core developer/compilers/ developer/
Projects
None yet
Milestone
A17S24
Development

Successfully merging this pull request may close these issues.
3 participants
@srl295 @mcdurdin @keyman-server