[mojo-js] Converts binding unittest into layout test.

[mojo-js] Converts binding unittest into layout test.

This patch prepares to remove dependency on //mojo/edk/js to run
js-bindings unittets.

BUG=647036
Review-Url: https://codereview.chromium.org/2743623003
Cr-Commit-Position: refs/heads/master@{#455925}
Committed: https://chromium.googlesource.com/chromium/src/+/5bf97f6bf369fc1d5b5bb77585f5c13dc218c216

[alokp, 2017-03-09 14:00:15]

alokp@chromium.org changed reviewers:
+ jbroman@chromium.org, yzshen@chromium.org

[alokp, 2017-03-09 14:00:16]

I plan to convert all tests in //mojo/public/js/tests in a similar fashion if
this approach looks reasonable. PTAL.

[alokp, 2017-03-09 14:15:00]

The CQ bit was checked by alokp@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-09 14:15:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 15:43:58]

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

[commit-bot: I haz the power, 2017-03-09 15:43:59]

Dry run: This issue passed the CQ dry run.

[yzshen1, 2017-03-09 17:03:54]

LGTM!

[jbroman, 2017-03-09 20:09:36]

LGTM with one suggestion

https://codereview.chromium.org/2743623003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/mojo/binding.html (right):

https://codereview.chromium.org/2743623003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/mojo/binding.html:56: binding_test((bindings,
math) => {
Nice! I realize this is mostly code movement, but you could even use JS async
functions here (which we've shipped since M55) here. It sugars over some of the
promise management boilerplate.

e.g. this one can be written as:

binding_test(async function(bindings, math) {
  var calc1 = new math.CalculatorPtr();
  var calcBinding = new bindings.Binding(math.Calculator,
                                         new CalculatorImpl(),
                                         bindings.makeRequest(calc1));
  assert_equals((await calc1.add(2)).value, 2);

  var calc2 = new math.CalculatorPtr();
  calcBinding.bind(bindings.makeRequest(calc2));
  assert_equals((await calc2.add(2)).value, 4);
}, 'reusable');

and the more complicated one below would be:

binding_test(async function(bindings, math) {
  var calc1 = new math.CalculatorPtr();
  var calc2 = new math.CalculatorPtr();
  var calcImpl = new CalculatorImpl();
  var bindingSet = new bindings.BindingSet(math.Calculator);
  assert_true(bindingSet.isEmpty());
  bindingSet.addBinding(calcImpl, bindings.makeRequest(calc1));
  bindingSet.addBinding(calcImpl, bindings.makeRequest(calc2));
  assert_false(bindingSet.isEmpty());

  assert_equals((await calc1.add(3)).value, 3);
  assert_equals((await calc2.add(4)).value, 7);
  await new Promise((resolve, reject) => {
    bindingSet.setConnectionErrorHandler(() => resolve());
    calc1.ptr.reset();
  });
  assert_equals((await calc2.add(5)).value, 12);

  bindingSet.closeAllBindings();
  assert_true(bindingSet.isEmpty());
}, 'binding set');

But I'm also fine with this as-is, if you prefer.

[jbroman, 2017-03-09 20:33:34]

On 2017/03/09 at 20:09:36, jbroman wrote:
> LGTM with one suggestion
> 
>
https://codereview.chromium.org/2743623003/diff/20001/third_party/WebKit/Layo...
> File third_party/WebKit/LayoutTests/mojo/binding.html (right):
> 
>
https://codereview.chromium.org/2743623003/diff/20001/third_party/WebKit/Layo...
> third_party/WebKit/LayoutTests/mojo/binding.html:56: binding_test((bindings,
math) => {
> Nice! I realize this is mostly code movement, but you could even use JS async
functions here (which we've shipped since M55) here. It sugars over some of the
promise management boilerplate.
> 
> e.g. this one can be written as:
> 
> binding_test(async function(bindings, math) {
>   var calc1 = new math.CalculatorPtr();
>   var calcBinding = new bindings.Binding(math.Calculator,
>                                          new CalculatorImpl(),
>                                          bindings.makeRequest(calc1));
>   assert_equals((await calc1.add(2)).value, 2);
> 
>   var calc2 = new math.CalculatorPtr();
>   calcBinding.bind(bindings.makeRequest(calc2));
>   assert_equals((await calc2.add(2)).value, 4);
> }, 'reusable');
> 
> and the more complicated one below would be:
> 
> binding_test(async function(bindings, math) {
>   var calc1 = new math.CalculatorPtr();
>   var calc2 = new math.CalculatorPtr();
>   var calcImpl = new CalculatorImpl();
>   var bindingSet = new bindings.BindingSet(math.Calculator);
>   assert_true(bindingSet.isEmpty());
>   bindingSet.addBinding(calcImpl, bindings.makeRequest(calc1));
>   bindingSet.addBinding(calcImpl, bindings.makeRequest(calc2));
>   assert_false(bindingSet.isEmpty());
> 
>   assert_equals((await calc1.add(3)).value, 3);
>   assert_equals((await calc2.add(4)).value, 7);
>   await new Promise((resolve, reject) => {
>     bindingSet.setConnectionErrorHandler(() => resolve());
>     calc1.ptr.reset();
>   });
>   assert_equals((await calc2.add(5)).value, 12);
> 
>   bindingSet.closeAllBindings();
>   assert_true(bindingSet.isEmpty());
> }, 'binding set');
> 
> But I'm also fine with this as-is, if you prefer.

Oh, and you could keep using arrow functions for async, if you want.

binding_test(async (bindings, math) => {
  // ...
});

[alokp, 2017-03-09 22:13:38]

https://codereview.chromium.org/2743623003/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/mojo/binding.html (right):

https://codereview.chromium.org/2743623003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/mojo/binding.html:56: binding_test((bindings,
math) => {
On 2017/03/09 20:09:35, jbroman wrote:
> Nice! I realize this is mostly code movement, but you could even use JS async
> functions here (which we've shipped since M55) here. It sugars over some of
the
> promise management boilerplate.
> 
> e.g. this one can be written as:
> 
> binding_test(async function(bindings, math) {
>   var calc1 = new math.CalculatorPtr();
>   var calcBinding = new bindings.Binding(math.Calculator,
>                                          new CalculatorImpl(),
>                                          bindings.makeRequest(calc1));
>   assert_equals((await calc1.add(2)).value, 2);
> 
>   var calc2 = new math.CalculatorPtr();
>   calcBinding.bind(bindings.makeRequest(calc2));
>   assert_equals((await calc2.add(2)).value, 4);
> }, 'reusable');
> 
> and the more complicated one below would be:
> 
> binding_test(async function(bindings, math) {
>   var calc1 = new math.CalculatorPtr();
>   var calc2 = new math.CalculatorPtr();
>   var calcImpl = new CalculatorImpl();
>   var bindingSet = new bindings.BindingSet(math.Calculator);
>   assert_true(bindingSet.isEmpty());
>   bindingSet.addBinding(calcImpl, bindings.makeRequest(calc1));
>   bindingSet.addBinding(calcImpl, bindings.makeRequest(calc2));
>   assert_false(bindingSet.isEmpty());
> 
>   assert_equals((await calc1.add(3)).value, 3);
>   assert_equals((await calc2.add(4)).value, 7);
>   await new Promise((resolve, reject) => {
>     bindingSet.setConnectionErrorHandler(() => resolve());
>     calc1.ptr.reset();
>   });
>   assert_equals((await calc2.add(5)).value, 12);
> 
>   bindingSet.closeAllBindings();
>   assert_true(bindingSet.isEmpty());
> }, 'binding set');
> 
> But I'm also fine with this as-is, if you prefer.

This is awesome. TIL

[alokp, 2017-03-09 22:14:44]

The CQ bit was checked by alokp@chromium.org

[alokp, 2017-03-09 22:14:44]

The patchset sent to the CQ was uploaded after l-g-t-m from yzshen@chromium.org,
jbroman@chromium.org
Link to the patchset: https://codereview.chromium.org/2743623003/#ps40001
(title: "uses async/await")

[commit-bot: I haz the power, 2017-03-09 22:15:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 23:39:08]

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

[commit-bot: I haz the power, 2017-03-09 23:39:09]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[alokp, 2017-03-09 23:45:40]

The CQ bit was checked by alokp@chromium.org

[commit-bot: I haz the power, 2017-03-09 23:46:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-10 00:35:32]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489103140230780,
"parent_rev": "aa1f961e37c8778191049bca02608cc54f389366", "commit_rev":
"5bf97f6bf369fc1d5b5bb77585f5c13dc218c216"}

[commit-bot: I haz the power, 2017-03-10 00:36:34]

Description was changed from

==========
[mojo-js] Converts binding unittest into layout test.

This patch prepares to remove dependency on //mojo/edk/js to run
js-bindings unittets.

BUG=647036
==========

to

==========
[mojo-js] Converts binding unittest into layout test.

This patch prepares to remove dependency on //mojo/edk/js to run
js-bindings unittets.

BUG=647036

Review-Url: https://codereview.chromium.org/2743623003
Cr-Commit-Position: refs/heads/master@{#455925}
Committed:
https://chromium.googlesource.com/chromium/src/+/5bf97f6bf369fc1d5b5bb77585f5...
==========

[commit-bot: I haz the power, 2017-03-10 00:36:36]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/5bf97f6bf369fc1d5b5bb77585f5...