1052243 - jshangul.js need unit tests

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Keyboard, defect)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

+++ This bug was initially created as a clone of Bug #994495 +++

The Korean IME script does not comes with unit tests -- we should spend time to create one so that new features (in both the keyboard app and the IME) will not break it and stop people from typing Korean.

Comment 1

[Sudheesh Singanamalla (:ShellHacker)]

I've never written an unit test before and neither do i know Korean. Is this bug going to be hard to pick and give a shot ?

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Hi :ShellHacker,

The said code can be found here:
https://github.com/mozilla-b2g/gaia/blob/master/apps/keyboard/js/imes/jshangul/jshangul.js

I think it's possible to write unit tests without knowing Korean. 

The information about writing and running Unit Tests can be found here:

https://github.com/mozilla-b2g/gaia#unit-tests

We use Mocha and Sinon in our test running environment.


Please take the bug and set to ASSIGNED if you want to work on it!

Comment 3

[Channy Yun [:channy]]

Sangpire has made some of unit test codes for his jshangule. 
Please add unit test code as a patch.

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Channy Yun [:channy] from comment #3)
> Sangpire has made some of unit test codes for his jshangule. 
> Please add unit test code as a patch.

You mean https://github.com/sangpire/jangmiIME/blob/master/test/test.coffee ?

In this case, do you think we should migrate these test cases to Gaia code base?

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I, for example, did not migrate JSZhuyin test cases from https://github.com/timdream/jszhuyin/tree/master/test to Gaia.

Comment 6

[BYUN Sangpil]

Sorry for my late reply.

I had made test codes for korean keyboard.
https://github.com/sangpire/gaia/blob/Bug994495/apps/keyboard/test/unit/jshangul_test.js

But, It needs a check.

I guess I could finish it by next weekends.

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Assign to you then.

Comment 8

[BYUN Sangpil]

Attachment: GitHub Pull Request URI.

I write unit tests for 'jshangul.js'.

Sorry, this patch include a bug fix.
Which is can't type JUNG SUNG and CHO SUNG without First CHO SUNG.
For example, when I type 'ㅏ' and 'ㄱ' then text should be like  'ㅏㄱ'.
The only input 'ㅏ', now.

This is found when I made unit tests.

Comment 9

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8475976 [details] [review]
GitHub Pull Request URI.

Thank you very much for the tests!

Please address the Github comment, amend the comment, force push and I will merge it for you.

Comment 10

[BYUN Sangpil]

Hi :timdream.

I changed it. :)

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

master: https://github.com/mozilla-b2g/gaia/commit/d9efe6251b8dcbb90c02ee8f6b21e54a24745624

Thanks for the contribution!

Comment 13

[Channy Yun [:channy]]

Tim, Would you estimate when Korean IME can push to flame phone?

Comment 14

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Channy Yun [:channy] from comment #13)
> Tim, Would you estimate when Korean IME can push to flame phone?

We only ship a few layouts out of disk space concern. Given the fact there isn't a dictionary in the build for Korean IME, I think we could ship it by default in Nightly builds. Let's me file a bug to turn that on.

For the rest of the layouts it's being tracked in bug 1050574.

Comment 15

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Filed bug 1058374 and patch submitted.