Port AbstractTtsTest as TtsBackgroundTest.

Port AbstractTtsTest as TtsBackgroundTest.

Modifications from upstream:
- renamed class as it doesn't make much sense to test an "abstract" class.
- made this into an E2E test so we can test against the real ChromeVox message strings and not have to fake out a lot of objects.

Committed: https://crrev.com/e7ea9d8c585bac871ddd0f37bf9da9e6c86f8d73
Cr-Commit-Position: refs/heads/master@{#294922}

[David Tseng, 2014-09-12 21:46:44]

dtseng@chromium.org changed reviewers:
+ dmazzoni@chromium.org, plundblad@chromium.org

[Peter Lundblad, 2014-09-15 08:42:09]

lgtm

We don't need to run as an e2e test to get 'real' messages.
testmsgs already exists and picks up our strings.  I don't know what other
objects you'd need to fake out to write this unit test.
I'd prefer to have unit tests isolated and not depend on the whole
extension when feasable.

Thanks,
//Peter

https://codereview.chromium.org/571643004/diff/1/chrome/browser/resources/chr...
File
chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background_test.extjs
(right):

https://codereview.chromium.org/571643004/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background_test.extjs:20:
};
nit: outdent

https://codereview.chromium.org/571643004/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background_test.extjs:69:
testDone();
nit: is it clearer to use the SYNC_TEST_F function that you wrote?

[David Tseng, 2014-09-15 16:36:36]

Apologies. I didn't look at your generated test messages file before; it
indeed contains real messages.

As for faking out objects, there are a slew of them...
chrome.i18n, chrome.storage, etc; basically, every chrome API that
TtsBackground uses. IMO, anything under the chromevox/background directory
*should* be an e2e test with few execptions.


On Mon, Sep 15, 2014 at 1:42 AM, <plundblad@chromium.org> wrote:

> lgtm
>
> We don't need to run as an e2e test to get 'real' messages.
> testmsgs already exists and picks up our strings.  I don't know what other
> objects you'd need to fake out to write this unit test.
> I'd prefer to have unit tests isolated and not depend on the whole
> extension when feasable.
>
> Thanks,
> //Peter
>
>
>
>
>
> https://codereview.chromium.org/571643004/diff/1/chrome/
> browser/resources/chromeos/chromevox/host/chrome/tts_background_test.extjs
> File
> chrome/browser/resources/chromeos/chromevox/host/
> chrome/tts_background_test.extjs
> (right):
>
> https://codereview.chromium.org/571643004/diff/1/chrome/
> browser/resources/chromeos/chromevox/host/chrome/tts_
> background_test.extjs#newcode20
> chrome/browser/resources/chromeos/chromevox/host/
> chrome/tts_background_test.extjs:20:
> };
> nit: outdent
> Done
> https://codereview.chromium.org/571643004/diff/1/chrome/
> browser/resources/chromeos/chromevox/host/chrome/tts_
> background_test.extjs#newcode69
> chrome/browser/resources/chromeos/chromevox/host/
> chrome/tts_background_test.extjs:69:
> testDone();
> nit: is it clearer to use the SYNC_TEST_F function that you wrote?
> Done
> https://codereview.chromium.org/571643004/
>
>
> To unsubscribe from this group and stop receiving emails from it, send an
> email to chromium-reviews+unsubscribe@chromium.org.
>

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

[David Tseng, 2014-09-15 16:41:31]

The CQ bit was checked by dtseng@chromium.org

[commit-bot: I haz the power, 2014-09-15 16:50:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/571643004/20001

[commit-bot: I haz the power, 2014-09-15 18:56:50]

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

[commit-bot: I haz the power, 2014-09-15 18:56:51]

Failed to apply patch for
chrome/browser/resources/chromeos/chromevox/chromevox_tests.gypi:
While running git apply --index -p1;
  error: patch failed:
chrome/browser/resources/chromeos/chromevox/chromevox_tests.gypi:145
  error: chrome/browser/resources/chromeos/chromevox/chromevox_tests.gypi: patch
does not apply

Patch:       chrome/browser/resources/chromeos/chromevox/chromevox_tests.gypi
Index: chrome/browser/resources/chromeos/chromevox/chromevox_tests.gypi
diff --git a/chrome/browser/resources/chromeos/chromevox/chromevox_tests.gypi
b/chrome/browser/resources/chromeos/chromevox/chromevox_tests.gypi
index
d08c7fbc2a064dd1e26431f9bf2f3fe8660e1d7f..35394174ff70b148ff09f86e514637795e34d43b
100644
--- a/chrome/browser/resources/chromeos/chromevox/chromevox_tests.gypi
+++ b/chrome/browser/resources/chromeos/chromevox/chromevox_tests.gypi
@@ -145,6 +145,7 @@
         'host/chrome/braille_integration_test.unitjs',
         'host/chrome/braille_table_test.extjs',
         'host/chrome/expanding_braille_translator_test.unitjs',
+        'host/chrome/tts_background_test.extjs',
       ],
       'conditions': [
         ['use_chromevox_next==1', {

[David Tseng, 2014-09-15 21:30:25]

The CQ bit was checked by dtseng@chromium.org

[commit-bot: I haz the power, 2014-09-15 22:03:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/571643004/40001

[commit-bot: I haz the power, 2014-09-15 23:37:02]

Committed patchset #3 (id:40001) as a1f1b8ad8c9d943a542653a46dc8330ba8cace8e

[commit-bot: I haz the power, 2014-09-15 23:46:08]

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