Mojo JS bindings: fix the unittests.

This CL removes the build target with the old EDK and makes the one with the new
EDK compile & pass.

This CL also merges the run_all_unittests.cc in mojo/edk/system and
mojo/edk/test.

BUG=536669
TEST=mojo_js_unittests should be able to run.

Review URL: https://codereview.chromium.org/1511783002

Cr-Commit-Position: refs/heads/master@{#364098}

mojo/BUILD.gn

@@ -74,9 +74,10 @@ group("tests") {
     "//mojo/application/public/cpp/tests:mojo_public_application_unittests",
     "//mojo/common:mojo_common_unittests",
     "//mojo/converters/surfaces/tests:mojo_surfaces_lib_unittests",
+    "//mojo/edk/js/test:js_unittests",
 
     # TODO(use_chrome_edk):
-    #"//mojo/edk/js/test:js_unittests",
+    #"//mojo/edk/js/test:js_integration_tests2",
     #"//mojo/edk/system:mojo_message_pipe_perftests",
     #"//mojo/edk/system:mojo_system_unittests",
     #"//mojo/edk/test:mojo_public_bindings_unittests",
@@ -84,7 +85,6 @@ group("tests") {
     #"//mojo/edk/test:mojo_public_system_perftests",
     #"//mojo/edk/test:mojo_public_system_unittests",
     #"//mojo/edk/test:mojo_public_utility_unittests",
-    "//third_party/mojo/src/mojo/edk/js/test:js_unittests",
     "//third_party/mojo/src/mojo/edk/system:mojo_message_pipe_perftests",
     "//third_party/mojo/src/mojo/edk/system:mojo_system_unittests",
     "//third_party/mojo/src/mojo/edk/test:mojo_public_bindings_unittests",

mojo/edk/js/BUILD.gn

@@ -13,7 +13,7 @@ group("tests") {
   deps = [
     # TODO(use_chrome_edk): remove "2"
     "test:js_integration_tests2",
-    "test:js_unittests2",
+    "test:js_unittests",
   ]
 }
 
@@ -47,17 +47,3 @@ source_set("js") {
     "//mojo/public/cpp/system",
   ]
 }
-
-source_set("js_unittests") {
-  testonly = true
-  sources = [
-    "handle_unittest.cc",
-  ]
-
-  deps = [
-    "//mojo/edk/js",
-    "//mojo/edk/test:test_support",
-    "//mojo/public/cpp/system",
-    "//testing/gtest",
-  ]
-}

mojo/edk/js/test/BUILD.gn

@@ -4,15 +4,15 @@
 
 import("//testing/test.gni")
 
-# TODO(use_chrome_edk): remove "2"
-test("js_unittests2") {
+test("js_unittests") {
+  output_name = "mojo_js_unittests"
+
   deps = [
-    "../../js",
-    "../../js:js_unittests",
-    "../../test:run_all_unittests",
-    "../../test:test_support",
     "//base",
     "//gin:gin_test",
+    "//mojo/edk/js",
+    "//mojo/edk/test:run_all_unittests",
+    "//mojo/edk/test:test_support",
     "//mojo/environment:chromium",
     "//mojo/public/cpp/environment",
     "//mojo/public/cpp/system",

mojo/edk/system/BUILD.gn

@@ -159,7 +159,6 @@ test("mojo_system_unittests2") {
     "options_validation_unittest.cc",
     "platform_handle_dispatcher_unittest.cc",
     "raw_channel_unittest.cc",
-    "run_all_unittests.cc",
     "shared_buffer_dispatcher_unittest.cc",
     "simple_dispatcher_unittest.cc",
     "waiter_test_utils.cc",
@@ -177,6 +176,7 @@ test("mojo_system_unittests2") {
     "../test:test_support",
     "//base",
     "//base/test:test_support",
+    "//mojo/edk/test:run_all_unittests",
     "//testing/gtest",
 
     # TODO(use_chrome_edk): temporary since the Mojo wrapper primitives are

mojo/edk/system/run_all_unittests.cc

@@ -1,45 +0,0 @@
-// Copyright 2014 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "base/bind.h"
-#include "base/command_line.h"
-#include "base/test/launcher/unit_test_launcher.h"
-#include "base/test/test_io_thread.h"
-#include "base/test/test_suite.h"
-#include "mojo/edk/embedder/embedder.h"
-#include "mojo/edk/test/multiprocess_test_helper.h"
-#include "mojo/edk/test/scoped_ipc_support.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-int main(int argc, char** argv) {
-// Silence death test thread warnings on Linux. We can afford to run our death
-// tests a little more slowly (< 10 ms per death test on a Z620).
-// On android, we need to run in the default mode, as the threadsafe mode
-// relies on execve which is not available.
-#if !defined(OS_ANDROID)
-  testing::GTEST_FLAG(death_test_style) = "threadsafe";
-#endif
-  base::TestSuite test_suite(argc, argv);
-
-  // Must be run before mojo::edk::Init.
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          mojo::edk::test::kBrokerHandleSwitch)) {
-    mojo::edk::PreInitializeChildProcess();
-  }
-
-  // TODO(use_chrome_edk): temporary to force new EDK.
-  base::CommandLine::ForCurrentProcess()->AppendSwitch("--use-new-edk");
-
-  mojo::edk::Init();
-
-  base::TestIOThread test_io_thread(base::TestIOThread::kAutoStart);
-  // Leak this because its destructor calls mojo::edk::ShutdownIPCSupport which
-  // really does nothing in the new EDK but does depend on the current message
-  // loop, which is destructed inside base::LaunchUnitTests.
-  new mojo::edk::test::ScopedIPCSupport(test_io_thread.task_runner());
-
-  return base::LaunchUnitTests(
-      argc, argv,
-      base::Bind(&base::TestSuite::Run, base::Unretained(&test_suite)));
-}

mojo/edk/test/run_all_unittests.cc

@@ -5,10 +5,12 @@
 #include <signal.h>
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/test/launcher/unit_test_launcher.h"
 #include "base/test/test_io_thread.h"
 #include "base/test/test_suite.h"
 #include "mojo/edk/embedder/embedder.h"
+#include "mojo/edk/test/multiprocess_test_helper.h"
 #include "mojo/edk/test/scoped_ipc_support.h"
 #include "mojo/edk/test/test_support_impl.h"
 #include "mojo/public/tests/test_support_private.h"
@@ -32,10 +34,23 @@ int main(int argc, char** argv) {
 
   base::TestSuite test_suite(argc, argv);
 
+  // Must be run before mojo::edk::Init.
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          mojo::edk::test::kBrokerHandleSwitch)) {
+    mojo::edk::PreInitializeChildProcess();
+  }
+
+  // TODO(use_chrome_edk): temporary to force new EDK.
+  base::CommandLine::ForCurrentProcess()->AppendSwitch("--use-new-edk");
+
   mojo::edk::Init();
+
   mojo::test::TestSupport::Init(new mojo::edk::test::TestSupportImpl());
   base::TestIOThread test_io_thread(base::TestIOThread::kAutoStart);
-  mojo::edk::test::ScopedIPCSupport ipc_support(test_io_thread.task_runner());
+  // Leak this because its destructor calls mojo::edk::ShutdownIPCSupport which
+  // really does nothing in the new EDK but does depend on the current message
+  // loop, which is destructed inside base::LaunchUnitTests.
+  new mojo::edk::test::ScopedIPCSupport(test_io_thread.task_runner());
 
   return base::LaunchUnitTests(
       argc, argv,

mojo/mojo_edk_tests.gyp

@@ -18,7 +18,7 @@
         # to be built.
         'mojo_message_pipe_perftests2',
         'mojo_system_unittests2',
-        'mojo_js_unittests2',
+        'mojo_js_unittests',
         'mojo_js_integration_tests2',
       ],
     },
@@ -94,7 +94,7 @@
     },
     {
       # GN version: //mojo/edk/js/test:js_unittests
-      'target_name': 'mojo_js_unittests2',
+      'target_name': 'mojo_js_unittests',
       'type': 'executable',
       'dependencies': [
         '../gin/gin.gyp:gin_test',

mojo/public/js/connector.js

@@ -110,8 +110,8 @@ define("mojo/public/js/connector", [
   };
 
   // The TestConnector subclass is only intended to be used in unit tests. It
-  // enables delivering a message to the pipe's handle without an async wait.
-
+  // doesn't automatically listen for input messages. Instead, you need to
+  // call waitForNextMessage to block and wait for the next incoming message.
   function TestConnector(handle) {
     Connector.call(this, handle);
   }
@@ -119,10 +119,12 @@ define("mojo/public/js/connector", [
   TestConnector.prototype = Object.create(Connector.prototype);
 
   TestConnector.prototype.waitToReadMore_ = function() {
-  };
+  }
 
-  TestConnector.prototype.deliverMessage = function() {
-    this.readMore_(core.RESULT_OK);
+  TestConnector.prototype.waitForNextMessage = function() {
+    var wait = core.wait(this.handle_, core.HANDLE_SIGNAL_READABLE,
+                         core.DEADLINE_INDEFINITE);
+    this.readMore_(wait.result);
   }
 
   var exports = {};

mojo/public/js/core_unittests.js

@@ -156,6 +156,9 @@ define([
     expect(write.result).toBe(core.RESULT_OK);
     expect(write.numBytes).toBe(42);
 
+    var wait = core.wait(pipe.consumerHandle, core.HANDLE_SIGNAL_READABLE,
+                         core.DEADLINE_INDEFINITE);
+    expect(wait.result).toBe(core.RESULT_OK);
     var peeked = core.readData(
          pipe.consumerHandle,
          core.READ_DATA_FLAG_PEEK | core.READ_DATA_FLAG_ALL_OR_NONE);

mojo/public/js/validation_unittests.js

@@ -180,7 +180,7 @@ define([
     return null;
   }
 
-  function getMessageTestFiles(key) {
+  function getMessageTestFiles(prefix) {
     var sourceRoot = file.getSourceRootDirectory();
     expect(sourceRoot).not.toBeNull();
 
@@ -192,11 +192,9 @@ define([
 
     // The matching ".data" pathnames with the extension removed.
     return testFiles.filter(function(s) {
-      return s.substr(-5) == ".data";
+      return s.substr(-5) == ".data" && s.indexOf(prefix) == 0;
     }).map(function(s) {
       return testDir + s.slice(0, -5);
-    }).filter(function(s) {
-      return s.indexOf(key) != -1;
     });
   }
 
@@ -220,8 +218,8 @@ define([
     expect(actualResult).toEqual(expectedResult);
   }
 
-  function testMessageValidation(key, filters) {
-    var testFiles = getMessageTestFiles(key);
+  function testMessageValidation(prefix, filters) {
+    var testFiles = getMessageTestFiles(prefix);
     expect(testFiles.length).toBeGreaterThan(0);
 
     for (var i = 0; i < testFiles.length; i++) {
@@ -260,15 +258,35 @@ define([
         testInterface.ConformanceTestInterface.validateRequest]);
   }
 
-  function testIntegratedMessageValidation(testFilesPattern) {
+  function testBoundsCheckMessageValidation() {
+    testMessageValidation("boundscheck_", [
+        testInterface.BoundsCheckTestInterface.validateRequest]);
+  }
+
+  function testResponseConformanceMessageValidation() {
+    testMessageValidation("resp_conformance_", [
+        testInterface.ConformanceTestInterface.validateResponse]);
+  }
+
+  function testResponseBoundsCheckMessageValidation() {
+    testMessageValidation("resp_boundscheck_", [
+        testInterface.BoundsCheckTestInterface.validateResponse]);
+  }
+
+  function testIntegratedMessageValidation(testFilesPattern,
+                                           localFactory,
+                                           remoteFactory) {
     var testFiles = getMessageTestFiles(testFilesPattern);
     expect(testFiles.length).toBeGreaterThan(0);
 
+    var testMessagePipe = new core.createMessagePipe();
+    expect(testMessagePipe.result).toBe(core.RESULT_OK);
+    var testConnection = new connection.TestConnection(
+        testMessagePipe.handle1, localFactory, remoteFactory);
+
     for (var i = 0; i < testFiles.length; i++) {
       var testMessage = readTestMessage(testFiles[i]);
       var handles = new Array(testMessage.handleCount);
-      var testMessagePipe = new core.createMessagePipe();
-      expect(testMessagePipe.result).toBe(core.RESULT_OK);
 
       var writeMessageValue = core.writeMessage(
           testMessagePipe.handle0,
@@ -277,40 +295,52 @@ define([
           core.WRITE_MESSAGE_FLAG_NONE);
       expect(writeMessageValue).toBe(core.RESULT_OK);
 
-      var testConnection = new connection.TestConnection(
-          testMessagePipe.handle1,
-          testInterface.IntegrationTestInterface.stubClass,
-          testInterface.IntegrationTestInterface.proxyClass);
-
       var validationError = noError;
       testConnection.router_.validationErrorHandler = function(err) {
         validationError = err;
       }
 
-      testConnection.router_.connector_.deliverMessage();
+      testConnection.router_.connector_.waitForNextMessage();
       checkValidationResult(testFiles[i], validationError);
-
-      testConnection.close();
-      expect(core.close(testMessagePipe.handle0)).toBe(core.RESULT_OK);
     }
+
+    testConnection.close();
+    expect(core.close(testMessagePipe.handle0)).toBe(core.RESULT_OK);
   }
 
   function testIntegratedMessageHeaderValidation() {
-    testIntegratedMessageValidation("integration_msghdr");
+    testIntegratedMessageValidation(
+        "integration_msghdr",
+        testInterface.IntegrationTestInterface.stubClass,
+        undefined);
+    testIntegratedMessageValidation(
+        "integration_msghdr",
+        undefined,
+        testInterface.IntegrationTestInterface.proxyClass);
   }
 
   function testIntegratedRequestMessageValidation() {
-    testIntegratedMessageValidation("integration_intf_rqst");
+    testIntegratedMessageValidation(
+        "integration_intf_rqst",
+        testInterface.IntegrationTestInterface.stubClass,
+        undefined);
   }
 
   function testIntegratedResponseMessageValidation() {
-    testIntegratedMessageValidation("integration_intf_resp");
+    testIntegratedMessageValidation(
+        "integration_intf_resp",
+        undefined,
+        testInterface.IntegrationTestInterface.proxyClass);
   }
 
   expect(checkTestMessageParser()).toBeNull();
   testConformanceMessageValidation();
+  testBoundsCheckMessageValidation();
+  testResponseConformanceMessageValidation();
+  testResponseBoundsCheckMessageValidation();
   testIntegratedMessageHeaderValidation();
   testIntegratedResponseMessageValidation();
   testIntegratedRequestMessageValidation();
+
   this.result = "PASS";
 });

third_party/mojo/mojo_edk_tests.gyp

@@ -24,7 +24,6 @@
         'mojo_public_system_unittests',
         'mojo_public_utility_unittests',
         'mojo_system_unittests',
-        'mojo_js_unittests',
         'mojo_js_integration_tests',
       ],
     },
@@ -255,24 +254,6 @@
         'src/mojo/edk/system/test_utils.h',
       ],
     },
-    {
-      # GN version: //mojo/edk/js/test:js_unittests
-      'target_name': 'mojo_js_unittests',
-      'type': 'executable',
-      'dependencies': [
-        '../../gin/gin.gyp:gin_test',
-        'mojo_edk.gyp:mojo_common_test_support',
-        'mojo_edk.gyp:mojo_run_all_unittests',
-        'mojo_edk.gyp:mojo_js_lib',
-        'mojo_public.gyp:mojo_environment_standalone',
-        'mojo_public.gyp:mojo_public_test_interfaces',
-        'mojo_public.gyp:mojo_utility',
-      ],
-      'sources': [
-        'src/mojo/edk/js/handle_unittest.cc',
-        'src/mojo/edk/js/test/run_js_tests.cc',
-      ],
-    },
     {
       # GN version: //mojo/edk/js/test:js_integration_tests
       'target_name': 'mojo_js_integration_tests',

third_party/mojo/src/mojo/edk/js/BUILD.gn

@@ -12,7 +12,6 @@ group("tests") {
   testonly = true
   deps = [
     "test:js_integration_tests",
-    "test:js_unittests",
   ]
 }
 

third_party/mojo/src/mojo/edk/js/test/BUILD.gn

@@ -4,27 +4,6 @@
 
 import("//testing/test.gni")
 
-test("js_unittests") {
-  deps = [
-    "../../js",
-    "../../js:js_unittests",
-    "../../test:run_all_unittests",
-    "../../test:test_support",
-    "//base",
-    "//gin:gin_test",
-    "//mojo/environment:chromium",
-    "//mojo/public/cpp/environment",
-    "//mojo/public/cpp/system",
-    "//mojo/public/cpp/utility",
-    "//mojo/public/interfaces/bindings/tests:test_interfaces",
-    "//mojo/public/interfaces/bindings/tests:test_interfaces_experimental",
-  ]
-
-  sources = [
-    "run_js_tests.cc",
-  ]
-}
-
 test("js_integration_tests") {
   deps = [
     "../../js",