Short-circuit failures in BLE js tests

Short-circuit failures in BLE js tests

Use chrome.test.sendMessage instead of chrome.test.fail so we
short-circuit instead of timeout on javascript failures where we're
waiting for a go signal.

BUG=388958
R=armansito@chromium.org

Committed: https://crrev.com/5f71564cd586274f373696c18930f72cab800b50
Cr-Commit-Position: refs/heads/master@{#297292}

[armansito, 2014-09-23 20:05:56]

So, in your solution we simple terminate the test by calling sendMessage and
making the listener exit its listen loop. The thing is the error messages and
checks that are done in the JS code might get lost this way. Instead, it would
be nice to call runTests so that we get the nicely formatted output.

Something like:

var error;

function testBlaBla() {
  if (error !== undefined) {
    chrome.test.fail("Test failed: " + error);
    chrome.test.sendMessage("fail");
    return;
  }

  ...

  chrome.test.succeed();
}

...

// Error case
error = "bla bla";
chrome.test.runTests([testBlaBla]);

...

// Success case
chrome.test.sendMessage("ready", function (message) {
  chrome.test.runTests([testBlaBla]);
});

https://codereview.chromium.org/593163002/diff/20001/extensions/browser/api/b...
File extensions/browser/api/bluetooth_low_energy/bluetooth_low_energy_apitest.cc
(right):

https://codereview.chromium.org/593163002/diff/20001/extensions/browser/api/b...
extensions/browser/api/bluetooth_low_energy/bluetooth_low_energy_apitest.cc:284:
ASSERT_EQ("ready", listener.message()) << listener.message();
Instead of doing this, you can give an ExtensionTestMessageListener an "error
message" to observe by calling set_failure_message() on the instance. This would
cause WaitUntilSatisfied to return false, and then in the tests you send the
error message for failure.

E.g.

  ExtensionTestMessageListener listener("ready", true);
  listener.set_failure_message("fail");
  ...
  EXPECT_TRUE(listener.WaitUntilSatisfied());

[Marie Janssen, 2014-09-25 19:03:17]

Added patchset 3, which does this runTests() shuffle most of the time.  
It doesn't do it where we didn't use runTests(): GetRemovedCharacteristic,
GetRemovedDescriptor, GetRemovedService, and UuidPermissionEvents

[armansito, 2014-09-29 17:40:54]

https://codereview.chromium.org/593163002/diff/40001/chrome/test/data/extensi...
File
chrome/test/data/extensions/api_test/bluetooth_low_energy/characteristic_properties/runtest.js
(right):

https://codereview.chromium.org/593163002/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/bluetooth_low_energy/characteristic_properties/runtest.js:51:
if (error !== undefined) return true;
nit: return in new line.

https://codereview.chromium.org/593163002/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/bluetooth_low_energy/characteristic_properties/runtest.js:58:
return error !== undefined;
You can just return false here.

https://codereview.chromium.org/593163002/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/bluetooth_low_energy/characteristic_properties/runtest.js:63:
if (failOnError()) return;
return in new line.

https://codereview.chromium.org/593163002/diff/40001/chrome/test/data/extensi...
File
chrome/test/data/extensions/api_test/bluetooth_low_energy/get_characteristic/runtest.js
(right):

https://codereview.chromium.org/593163002/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/bluetooth_low_energy/get_characteristic/runtest.js:59:
if (!expectFailed(result)) return;
Returns in new line (here and elsewhere in CL)

https://codereview.chromium.org/593163002/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/bluetooth_low_energy/get_characteristic/runtest.js:78:
chrome.test.runTests([testGetCharacteristic]);
Don't we want to call sendMessage('fail', ..) here?

[Marie Janssen, 2014-09-29 18:44:10]

Updated patch for these comments.

https://codereview.chromium.org/593163002/diff/40001/chrome/test/data/extensi...
File
chrome/test/data/extensions/api_test/bluetooth_low_energy/characteristic_properties/runtest.js
(right):

https://codereview.chromium.org/593163002/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/bluetooth_low_energy/characteristic_properties/runtest.js:51:
if (error !== undefined) return true;
On 2014/09/29 17:40:53, armansito wrote:
> nit: return in new line.

Done, and elsewhere too.

https://codereview.chromium.org/593163002/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/bluetooth_low_energy/characteristic_properties/runtest.js:58:
return error !== undefined;
On 2014/09/29 17:40:53, armansito wrote:
> You can just return false here.

Done.

https://codereview.chromium.org/593163002/diff/40001/chrome/test/data/extensi...
File
chrome/test/data/extensions/api_test/bluetooth_low_energy/get_characteristic/runtest.js
(right):

https://codereview.chromium.org/593163002/diff/40001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/bluetooth_low_energy/get_characteristic/runtest.js:78:
chrome.test.runTests([testGetCharacteristic]);
On 2014/09/29 17:40:53, armansito wrote:
> Don't we want to call sendMessage('fail', ..) here?

This is fixed with moving sendMessage() to the test*();

[armansito, 2014-09-29 19:55:44]

lgtm with nits (you don't have to wait for another lgtm once you fix these).

https://codereview.chromium.org/593163002/diff/60001/chrome/test/data/extensi...
File
chrome/test/data/extensions/api_test/bluetooth_low_energy/characteristic_properties/runtest.js
(right):

https://codereview.chromium.org/593163002/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/bluetooth_low_energy/characteristic_properties/runtest.js:53:
return true;
nit: Add a newline after the return statements, makes it easier to read (here
and elsewhere).

https://codereview.chromium.org/593163002/diff/60001/chrome/test/data/extensi...
File
chrome/test/data/extensions/api_test/bluetooth_low_energy/get_services/runtest.js
(right):

https://codereview.chromium.org/593163002/diff/60001/chrome/test/data/extensi...
chrome/test/data/extensions/api_test/bluetooth_low_energy/get_services/runtest.js:16:
services[0].uuid);
nit: Left-align the trailing arguments. If the first argument isn't in a new
line then align the args with the first arg.

[Marie Janssen, 2014-09-29 21:28:23]

The CQ bit was checked by jamuraa@chromium.org

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

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/593163002/80001

[commit-bot: I haz the power, 2014-09-29 22:43:03]

Committed patchset #5 (id:80001) as 964196856146d72ab7e6a4d4e0f0cf7426697cf1

[commit-bot: I haz the power, 2014-09-29 22:45:25]

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