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

Please review ibus-hangul test case #96

Merged
merged 3 commits into from
Apr 20, 2020
Merged

Conversation

epico
Copy link

@epico epico commented Feb 18, 2020

Currently we are writing test case for Fedora packages.

This is the test case for ibus-hangul.

Please review the patch, thanks!

@fujiwarat
Copy link

LGTM

@changwoo
Copy link

changwoo commented Mar 9, 2020

I suggests adding more Hangul input key (and mouse) sequences which had been broken often in ibus+ibus-hangul. I think installed-test is a good way to check such problems automatically.

For each test, commit text, preedit text and caret position need to be checked. For example

  • input "rksi" -> commit "가", preedit "냐" (testing implicit commit and next syllable preedit)

  • input "rk " -> commit "가 ", preedit "" (testing implicit commit by non-Hangul key and forwarding key event)

  • input "rk" + left arrow -> commit "가", caret on left of "가" (similar to above, testing that the commit is first processed before arrow key event forwarding)

  • input "rksi" + click on the very left -> commit "가냐", caret on left of "가" (commit text on previous position)

@fujiwarat
Copy link

I guess all the test cases can be appended space or Return key to commit the preedit and check the commit string only since users always would commit the preedit finally.

I think the current test does not work with mouse so the 4th case would not be implemented.

@changwoo
Copy link

I finally tested the PR branch and tried to add some test cases I wanted. But the current test code seems to be limited and works only with 1 Hangul character at a time. It's not easy to adds more tests.

For now I agree merging the branch as is. @choehwanjin please just merge it. I'm going to improve it later when I have more time.

@epico
Copy link
Author

epico commented Apr 20, 2020

@choehwanjin , Could you merge this pull request?

@choehwanjin choehwanjin merged commit 24eecbc into libhangul:master Apr 20, 2020
@choehwanjin
Copy link
Member

Done. Thank you.

@epico
Copy link
Author

epico commented Apr 21, 2020

Thanks!

@fujiwarat
Copy link

@choehwanjin Could you release a new version to include this change?

@choehwanjin
Copy link
Member

@fujiwarat This test always fails on my linux system. It should be fixed before release.

@choehwanjin
Copy link
Member

@fujiwarat I was mistaken. I had "preedit-mode" as "none", which caused the test to fail. So it's OK to release.

choehwanjin added a commit that referenced this pull request Aug 23, 2020
Update version to 1.5.4

#96 (comment)
@fujiwarat
Copy link

@choehwanjin Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants