Commit 04c5a199 authored by ortuno's avatar ortuno Committed by Commit bot

bluetooth: Check the order of event and promise resolution for readValue

Introduces assert_promise_event_order_ that checks the order of promise
resolution and events firing is correct.

Makes assert_event_fires_after_promise and assert_event_fires_before_promise
use this function.

Also removes some obsolete TODOs.

BUG=697698

Review-Url: https://codereview.chromium.org/2825393004
Cr-Commit-Position: refs/heads/master@{#467261}
parent 040755ec
...@@ -14,11 +14,10 @@ promise_test(() => { ...@@ -14,11 +14,10 @@ promise_test(() => {
.then(service => service.getCharacteristic('heart_rate_measurement')) .then(service => service.getCharacteristic('heart_rate_measurement'))
.then(characteristic => { .then(characteristic => {
char = characteristic; char = characteristic;
return assert_event_fires_after_promise(characteristic, return assert_promise_resolves_before_event(characteristic,
'startNotifications', 'startNotifications',
'characteristicvaluechanged', 'characteristicvaluechanged',
3 /* add 3 listeners */, 3 /* add 3 listeners */);
false /* ignore_event_promise_order */);
}) })
.then(() => char.stopNotifications()) .then(() => char.stopNotifications())
.then(() => assert_no_events(char, 'characteristicvaluechanged')); .then(() => assert_no_events(char, 'characteristicvaluechanged'));
......
...@@ -12,9 +12,8 @@ promise_test(() => { ...@@ -12,9 +12,8 @@ promise_test(() => {
.then(gattServer => gattServer.getPrimaryService('heart_rate')) .then(gattServer => gattServer.getPrimaryService('heart_rate'))
.then(service => service.getCharacteristic('heart_rate_measurement')) .then(service => service.getCharacteristic('heart_rate_measurement'))
.then(characteristic => { .then(characteristic => {
return assert_event_fires_after_promise( return assert_promise_resolves_before_event(
characteristic, 'startNotifications', 'characteristicvaluechanged', characteristic, 'startNotifications', 'characteristicvaluechanged')
1 /* attach 1 listener */, false /* ignore_event_promise_order */)
.then(() => characteristic.stopNotifications()) .then(() => characteristic.stopNotifications())
.then(() => assert_no_events(characteristic, .then(() => assert_no_events(characteristic,
'characteristicvaluechanged')); 'characteristicvaluechanged'));
......
...@@ -12,20 +12,15 @@ promise_test(() => { ...@@ -12,20 +12,15 @@ promise_test(() => {
.then(gattServer => gattServer.getPrimaryService('heart_rate')) .then(gattServer => gattServer.getPrimaryService('heart_rate'))
.then(service => service.getCharacteristic('body_sensor_location')) .then(service => service.getCharacteristic('body_sensor_location'))
.then(characteristic => { .then(characteristic => {
return assert_event_fires_after_promise(characteristic, return assert_promise_resolves_after_event(characteristic,
'readValue', 'readValue',
'characteristicvaluechanged', 'characteristicvaluechanged',
3 /* attach 3 listeners */, 3 /* attach 3 listeners */);
true /* ignore_event_promise_order */);
}).then(results => { }).then(results => {
let read_value = results[0].buffer; let read_value = results[0].buffer;
let event_values = results.slice(1).map(v => v.buffer); let event_values = results.slice(1).map(v => v.buffer);
for (let event_value of event_values) { for (let event_value of event_values) {
// TODO(ortuno): The DataView used to resolve the promise assert_equals(event_value, read_value);
// should be the same DataView as the one saved in the
// characteristic.
// http://crbug.com/543347
// assert_equals(event.target.value, value);
assert_array_equals(event_value, read_value); assert_array_equals(event_value, read_value);
} }
}); });
......
...@@ -12,19 +12,13 @@ promise_test(() => { ...@@ -12,19 +12,13 @@ promise_test(() => {
.then(gattServer => gattServer.getPrimaryService('heart_rate')) .then(gattServer => gattServer.getPrimaryService('heart_rate'))
.then(service => service.getCharacteristic('body_sensor_location')) .then(service => service.getCharacteristic('body_sensor_location'))
.then(characteristic => { .then(characteristic => {
return assert_event_fires_after_promise(characteristic, return assert_promise_resolves_after_event(characteristic,
'readValue', 'readValue',
'characteristicvaluechanged', 'characteristicvaluechanged');
1 /* attach 1 listener */,
true /* ignore_event_promise_order */);
}).then(results => { }).then(results => {
let read_value = results[0].buffer; let read_value = results[0].buffer;
let event_value = results[1].buffer; let event_value = results[1].buffer;
// TODO(ortuno): The DataView used to resolve the promise assert_equals(event_value, read_value);
// should be the same DataView as the one saved in the
// characteristic.
// http://crbug.com/543347
// assert_equals(event.target.value, value);
assert_array_equals(event_value, read_value); assert_array_equals(event_value, read_value);
}); });
}, 'Reading a characteristic should fire an event.'); }, 'Reading a characteristic should fire an event.');
......
...@@ -263,39 +263,64 @@ function eventPromise(target, type, options) { ...@@ -263,39 +263,64 @@ function eventPromise(target, type, options) {
}); });
} }
// Creates |num_listeners| promises. Each adds an event listener
// to object. The promises resolve once the object fires |event| but
// reject if the event is fired before |object|.|func|() resolves. // Helper function to assert that events are fired and a promise resolved
// Returns a promise that fulfills with the result of |object|.|func()| // in the correct order.
// and |event.target.value| of each of the other promises. // 'event' should be passed as |should_be_first| to indicate that the events
// If |ignore_event_promise_order| is set true, this function will ignore // should be fired first, otherwise 'promiseresolved' should be passed.
// the relative order of the event and the promise; otherwise it will assert // Attaches |num_listeners| |event| listeners to |object|. If all events have
// the event is triggered after the promise is resolved. // been fired and the promise resolved in the correct order, returns a promise
function assert_event_fires_after_promise(object, func, event, num_listeners, ignore_event_promise_order) { // that fulfills with the result of |object|.|func()| and |event.target.value|
num_listeners = num_listeners !== undefined ? num_listeners : 1; // of each of event listeners. Otherwise throws an error.
function assert_promise_event_order_(should_be_first, object, func, event, num_listeners) {
if (object[func] === undefined) { let order = [];
return Promise.reject('Function \'' + func + '\' not available in object.'); let event_promises = []
}
let should_resolve = false;
let event_promises = [];
for (let i = 0; i < num_listeners; i++) { for (let i = 0; i < num_listeners; i++) {
event_promises.push(new Promise((resolve, reject) => { event_promises.push(new Promise(resolve => {
let event_listener = (e) => { let event_listener = (e) => {
object.removeEventListener(event, event_listener); object.removeEventListener(event, event_listener);
if (should_resolve || ignore_event_promise_order) { order.push('event');
resolve(e.target.value); resolve(e.target.value);
} else { }
reject(event + ' was triggered before the promise resolved.');
}
};
object.addEventListener(event, event_listener); object.addEventListener(event, event_listener);
})); }));
} }
return object[func]().then(result => {
should_resolve = true; let func_promise = object[func]().then(result => {
return Promise.all([result, ...event_promises]); order.push('promiseresolved');
return result;
}); });
return Promise.all([func_promise, ...event_promises])
.then((result) => {
if (should_be_first !== order[0]) {
throw should_be_first === 'promiseresolved' ?
`'${event}' was fired before promise resolved.` :
`Promise resolved before '${event}' was fired.`;
}
if (order[0] !== 'promiseresolved' &&
order[order.length - 1] !== 'promiseresolved') {
throw 'Promise resolved in between event listeners.';
}
return result;
});
}
// See assert_promise_event_order_ above.
function assert_promise_resolves_before_event(
object, func, event, num_listeners=1) {
return assert_promise_event_order_(
'promiseresolved', object, func, event, num_listeners);
}
// See assert_promise_event_order_ above.
function assert_promise_resolves_after_event(
object, func, event, num_listeners=1) {
return assert_promise_event_order_(
'event', object, func, event, num_listeners);
} }
// Returns a promise that resolves after 100ms unless // Returns a promise that resolves after 100ms unless
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment