We need a unit test for Pinyin IME especially for its engine library.

Rudy, Could you please help review this unit-testing patch in your spare time? Thanks a lot!

Comment on attachment 823791 [details] Pull Request 13161 r+ for tests only patch. 2 questions, 1. I guess we don't need to mock indexedDB. 2. Do you think we should remove the test page since now we have unit tests? apps/keyboard/js/imes/jspinyin/tests/index.html Thanks for making the pinyin more robust.

Rudy, Thanks for reviewing. About the questions, 1. Yes, we don't need the mock indexedDB if we push the prepared data into indexedDB in "setup" stage. I think, however, we can keep the mock indexedDB because it can help us to test the behavior of the engine when indexedDB failed. 2. Since "jspinyin/tests/index.html" can be used to measure the performance of the pinyin engine, I propose to leave it as-is.

travis passed: https://travis-ci.org/mozilla-b2g/gaia/builds/13250554 merged in gaia master: https://github.com/mozilla-b2g/gaia/commit/9ab17bf1759b8abd5e1de7e71c98f617a87a7a25

Sadly I think this breaks the other keyboard tests (via some test runner bug) unless I am missing something entirely I don't see 300~ or so of the tests that where there before. You can verify this by looking at the number of tests passed in your PR vs right before

https://travis-ci.org/mozilla-b2g/gaia/jobs/13251844#L1531 https://travis-ci.org/mozilla-b2g/gaia/jobs/13250556#L1551 Looks like around 1k~ tests lost hmm

This is sad but I backed out the merge: https://github.com/mozilla-b2g/gaia/commit/c43efb5defdd97f778fab95354fe5df9d37a6665 We need to figure out what happened to the other tests =/

Really thanks for backing out. I will look into it soon!

I found the issue is caused by the conflict of global variables like "InputMethods." I tried to isolate all the global variables I used and also modified the latin and suggest unit test to prevent this situation. Rudy, could you please review it again when you are free? Thanks a lot.

Comment on attachment 826242 [details] Pull Request: https://github.com/mozilla-b2g/gaia/pull/13222 I guess this is just a workaround for the issue you found in testing framework. r+ if the following call is ok to exist in latin_test.js - suiteSetup(init); Please also file a bug for the overall issue about global var and better to have a simple test case there to identify the problem easily. Thanks for your work, great job!

Rudy, thanks for your review. I will make further investigation to confirm whether it is a bug or not and then file a bug.

travis passed: https://travis-ci.org/mozilla-b2g/gaia/builds/13504738 merged in gaia master: https://github.com/mozilla-b2g/gaia/commit/bc69b2a33a9a3f8b3044f37f82efd65ca82746c2

clear ni.

Comment on attachment 826242 [details] Pull Request: https://github.com/mozilla-b2g/gaia/pull/13222 clear feedback.

Comment on attachment 826242 [details] Pull Request: https://github.com/mozilla-b2g/gaia/pull/13222 clear feedback.