Please review ibus-hangul test case

libhangul / ibus-hangul

The hangul engine for IBus

GNU General Public License v2.0

67 stars 18 forks source link

Please review ibus-hangul test case #96

Closed epico closed 4 years ago

epico commented 4 years ago

Currently we are writing test case for Fedora packages.

This is the test case for ibus-hangul.

Please review the patch, thanks!

fujiwarat commented 4 years ago

LGTM

changwoo commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@choehwanjin , Could you merge this pull request?

choehwanjin commented 4 years ago

Done. Thank you.

epico commented 4 years ago

Thanks!

fujiwarat commented 4 years ago

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

choehwanjin commented 4 years ago

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

choehwanjin commented 4 years ago

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

fujiwarat commented 4 years ago

@choehwanjin Thank you!