977451 - [keyboard refactor] Unit test for autocorrect.js and dependencies (predictions.js/worker.js/suggestions.js)

Status: RESOLVED FIXED

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

Description

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

We need autocorrect.js and it's dependency (predictions.js/worker.js/suggestions.js) covered by tests.

Comment 1

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

Maybe the dependencies warrants it's own unit tests but let's write a test with a larger scope first?

Comment 2

[Jan Jongboom [:janjongboom] (inactive)]

This has tests, they are in normal keyboard already. The code for this has not been changed.

Comment 3

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

https://github.com/timdream/gaia/tree/keyboard2-autocorrect-test

WIP

Comment 4

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

(In reply to Jan Jongboom [:janjongboom] from comment #2)
> This has tests, they are in normal keyboard already. The code for this has
> not been changed.

You are right on the predictions.js and worker.js, but not autocorrect.js and suggestions.js.
I wonder if we should copy the predictions.js and worker.js tests here too.

Comment 5

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

Pull request updated again.

Comment 6

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

Attachment: Github: https://github.com/mozilla-b2g/gaia/pull/17677

Rudy, could you review this? I added the tests and removed two events since these events have no value outside of the two instances.


This needs to land before language switch support if your patch there includes changes to autocorrect.js.

Comment 7

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 8397528 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/17677

I'd like to see if Jan has bandwidth to look at this part of tests.
I believe he is a better reviewer for suggestion/auto correction and I have some blockers and tasks to handle, so will come to the review later.

Jan,

Could you help?
Thanks.

Comment 8

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8397528 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/17677

See my comment on GH.

Comment 9

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

Comment on attachment 8397528 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/17677

Pull request updated.

Comment 10

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8397528 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/17677

Almost there. See GH.

Comment 11

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

Comment on attachment 8397528 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/17677

if https://github.com/mozilla-b2g/gaia/pull/17677#issuecomment-39734961
is the only issue left I already have it right here:

https://github.com/mozilla-b2g/gaia/pull/17677/files#r11370975

Comment 12

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8397528 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/17677

r=me, if the 'char' assertions are removed.

Comment 13

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

Patch fixed and rebased, waiting for Travis before merging.

Comment 14

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

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