Port braille related tests from ChromeVox upstream.

Port braille related tests from ChromeVox upstream.

This adds braille tests that excercise functionality that run in the background page, with the exception of an integration test that tests the message passing between content scripts and the background page for braille.

BUG=371692
Committed: https://crrev.com/c6484806dba78a8fef095ede95eeab4ef3719536
Cr-Commit-Position: refs/heads/master@{#294160}

[Peter Lundblad, 2014-09-08 15:16:22]

plundblad@chromium.org changed reviewers:
+ @chromium.orgdtseng

[Peter Lundblad, 2014-09-08 15:20:00]

NOTES:
- Most of these tests test things that we want to keep in chromevox next.
- These tests were heavy mock users perviously and so required a bit more
hand-work than most other tests that we ported earlier.
- I considered not bothering with braille_integration_test, but I think it has
value as long as we have the current chromevox. I considered making it run
inside ChromeVox and test with an actual message bridge, but decided it wasn't
worth the effort and potential flakiness.

[Peter Lundblad, 2014-09-08 15:22:34]

plundblad@chromium.org changed reviewers:
+ dtseng@chromium.org
- @chromium.orgdtseng

[David Tseng, 2014-09-08 21:46:00]

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
File
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_display_manager_test.unitjs
(right):

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_display_manager_test.unitjs:61:
* @param {?number} opt_selEnd last cell that should have a selection.
nit: number=

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
File
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table_test.extjs
(right):

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table_test.extjs:12:
* a webui-style test.
Is this a new test? Would be helpful to separate out from ported tests so I can
focus on actual new code.

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table_test.extjs:28:
assertEquals(60, tables.length);
Define as a constant above in the class constructor.

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table_test.extjs:45:
assertNotEquals(null, uncontractedTable);
assertNotNull? (if it exists).

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/testing/assert_additions.js
(right):

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/testing/assert_additions.js:17: *
@param {object} a The argument to check.
arg or obj?

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js
(right):

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:1: //
Copyright 2014 The Chromium Authors. All rights reserved.
Maybe call this file fake_event.js?

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:7: *
@constructor
Can you inherit from a ChromeEvent?

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:10: /**
@private {Function} */
Is this valid js doc?

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:17: * fails
the test if it is already set.
Why? This isn't the behavior of ChromeEvent.

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:26: * Gets
the listener of the event, failing the test if there's noone.
nit: none

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:29:
getListener: function() {
Is this part of the ChromeEvent interface?

[Peter Lundblad, 2014-09-09 11:19:27]

Hi,

(Feel free to check the commit box if you lgtm this;)

dtseng@chromium.org writes:
> 
>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
> File
>
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_display_manager_test.unitjs
> (right):
> 
>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
>
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_display_manager_test.unitjs:61:
> * @param {?number} opt_selEnd last cell that should have a selection.
> nit: number=

Done here and above.

>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
> File
>
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table_test.extjs
> (right):
> 
>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
>
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table_test.extjs:12:
> * a webui-style test.
> Is this a new test? Would be helpful to separate out from ported tests
> so I can focus on actual new code.

This is not new, tehre's a corresponding test upstream.

>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
>
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table_test.extjs:28:
> assertEquals(60, tables.length);
> Define as a constant above in the class constructor.

I dont' think that's worth it in a test.  Not changed.

>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
>
chrome/browser/resources/chromeos/chromevox/host/chrome/braille_table_test.extjs:45:
> assertNotEquals(null, uncontractedTable);
> assertNotNull? (if it exists).

Doesn't and e have assertEquals(null,...) elsewhere, so I think this is fine.
Can add assert(Not)Null if you feel strongly.

>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
> File
> chrome/browser/resources/chromeos/chromevox/testing/assert_additions.js
> (right):
> 
>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/testing/assert_additions.js:17:
> * @param {object} a The argument to check.
> arg or obj?

Changed to obj.

>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
> File chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js
> (right):
> 
>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:1:
> // Copyright 2014 The Chromium Authors. All rights reserved.
> Maybe call this file fake_event.js?

I'm thinking that we could add other common fake objects here.  Since they are
usually trivial, it seems like overkill to have many little files with those.
Can change if you feel strongly.

>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:7: *
> @constructor
> Can you inherit from a ChromeEvent?

Dunno if I can, but I did.  We are not compiling tests, so we can put
whatever we want here;)

>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:10:
> /** @private {Function} */
> Is this valid js doc?

Nope, changed.

>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:17:
> * fails the test if it is already set.
> Why? This isn't the behavior of ChromeEvent.

I only implement what's needed.  If we need a more complete implementation we
can extend this later.

>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:26:
> * Gets the listener of the event, failing the test if there's noone.
> nit: none

Done.

>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:29:
> getListener: function() {
> Is this part of the ChromeEvent interface?

Not as far as now, but it is used in the tests.

Thanks,
//Peter

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[David Tseng, 2014-09-09 20:37:48]

Just one small naming change; otherwise, LGTM

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
File chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js
(right):

https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:7: *
@constructor
On 2014/09/08 21:45:59, David Tseng wrote:
> Can you inherit from a ChromeEvent?

Maybe call this class FakeChromeEvent then :).

[Peter Lundblad, 2014-09-10 09:54:00]

The CQ bit was checked by plundblad@chromium.org

[Peter Lundblad, 2014-09-10 09:54:43]

dtseng@chromium.org writes:
> Just one small naming change; otherwise, LGTM
> 
> 
>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
> File chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js
> (right):
> 
>
https://codereview.chromium.org/484063003/diff/80001/chrome/browser/resources...
> chrome/browser/resources/chromeos/chromevox/testing/fake_objects.js:7: *
> @constructor
> On 2014/09/08 21:45:59, David Tseng wrote:
> > Can you inherit from a ChromeEvent?
> 
> Maybe call this class FakeChromeEvent then :).

Done.

> https://codereview.chromium.org/484063003/

-- 

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-09-10 09:55:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/plundblad@chromium.org/484063003/120001

[commit-bot: I haz the power, 2014-09-10 10:08:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-10 10:08:29]

Try jobs failed on following builders:
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)

[Peter Lundblad, 2014-09-10 12:32:23]

The CQ bit was checked by plundblad@chromium.org

[commit-bot: I haz the power, 2014-09-10 12:33:29]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/plundblad@chromium.org/484063003/120001

[commit-bot: I haz the power, 2014-09-10 12:34:54]

Committed patchset #7 (id:120001) as fce89a12057385fa9a35284211461669bd6dd679

[commit-bot: I haz the power, 2014-09-10 12:37:14]

Patchset 7 (id:??) landed as
https://crrev.com/c6484806dba78a8fef095ede95eeab4ef3719536
Cr-Commit-Position: refs/heads/master@{#294160}