JavaScript bindings for Mojo message validation

mojo/public/js/bindings/validation_unittests.js

 // 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.
 
 define([
     "file",
     "gin/test/expect",
     "mojo/public/interfaces/bindings/tests/validation_test_interfaces.mojom",
     "mojo/public/js/bindings/buffer",
     "mojo/public/js/bindings/codec",
@@ skipping 144 matching lines @@
       testEmptyInput();
       testBlankInput();
       testHandles();
       testAnchors();
     } catch (e) {
       return e.toString();
     }
     return null;
   }
 
-  function getMessageTestFiles() {
+  function getMessageTestFiles(key) {
     var sourceRoot = file.getSourceRootDirectory();
     expect(sourceRoot).not.toBeNull();
 
     var testDir = sourceRoot +
       "/mojo/public/interfaces/bindings/tests/data/validation/";
     var testFiles = file.getFilesInDirectory(testDir);
     expect(testFiles).not.toBeNull();
     expect(testFiles.length).toBeGreaterThan(0);
 
-   // The ".data" pathnames with the extension removed.
-   var testPathNames = testFiles.filter(function(s) {
-     return s.substr(-5) == ".data";
-   }).map(function(s) {
-     return testDir + s.slice(0, -5);
-   });
-
-   // For now, just checking the message header tests.
-   return testPathNames.filter(function(s) {
-     return s.indexOf("_msghdr_") != -1;
-   });
+    // The matching ".data" pathnames with the extension removed.
+    return testFiles.filter(function(s) {
+      return s.substr(-5) == ".data";
+    }).map(function(s) {
+      return testDir + s.slice(0, -5);
+    }).filter(function(s) {
+      return s.indexOf(key) != -1;
+    });
   }
 
   function readTestMessage(filename) {
     var contents = file.readFileToString(filename + ".data");
     expect(contents).not.toBeNull();
     return parser.parseTestMessage(contents);
   }
 
   function readTestExpected(filename) {
     var contents = file.readFileToString(filename + ".expected");
     expect(contents).not.toBeNull();
     return contents.trim();
   }
 
-  function testValidateMessageHeader() {
-    var testFiles = getMessageTestFiles();
+  function testMessageValidation(key, filters) {
+    var testFiles = getMessageTestFiles(key);
     expect(testFiles.length).toBeGreaterThan(0);
 
+    var noError = validator.validationError.NONE;
     for (var i = 0; i < testFiles.length; i++) {
+      // TODO(hansmuller): Temporarily skipping array pointer overflow tests.
+      if (testFiles[i].indexOf("overflow") != -1)

[yzshen1, 2014/08/14 00:06:32]

It may be nice to output a warning with the file name saying that the test is
skipped.

[hansmuller, 2014/08/14 00:57:57]

Good point, I've done that.

+        continue;
+
       var testMessage = readTestMessage(testFiles[i]);
-      // TODO(hansmuller): add the message handles.
-      var message = new codec.Message(testMessage.buffer);
-      var actualResult = new validator.Validator(message).validateMessage();
+      var handles = new Array(testMessage.handleCount);
+      var message =  new codec.Message(testMessage.buffer, handles);

[yzshen1, 2014/08/14 00:06:32]

extra space after =

[hansmuller, 2014/08/14 00:57:57]

Done.

+      var messageValidator = new validator.Validator(message);
+
+      var err = messageValidator.validateMessageHeader();

[yzshen1, 2014/08/14 00:06:32]

Something to think about:
It seems a little inconsistent:
messageValidator.validateMessageHeader() and
XXX.validateXXXXRequest(messageValidator).

Maybe we could have a ValidationContext object, and then do:
validateMessageHeader(validationContext)
XXX.validateXXXRequest(validationContext)
?

[hansmuller, 2014/08/14 00:57:56]

Acknowledged.

+      for (var j = 0; err === noError && j < filters.length; ++j)
+        err = filters[j](messageValidator);
+
+      var actualResult = (err === noError) ? "PASS" : err;
       var expectedResult = readTestExpected(testFiles[i]);
       expect(actualResult).toEqual(expectedResult);

[yzshen1, 2014/08/14 00:06:32]

It seems nice if we print out the file name on failure.

[hansmuller, 2014/08/14 00:57:57]

I agree. The simple JS expect framework doesn't appear to provide a way to
inject a failure message, so I've added an extra (trivial) test as well.

     }
   }
 
-  testValidateMessageHeader();
+  function testConformanceMessageValidation() {
+    testMessageValidation("conformance_", [
+      testInterface.validateConformanceTestInterfaceRequest,
+      testInterface.validateConformanceTestInterfaceResponse

[yzshen1, 2014/08/14 00:06:32]

All the conformance test cases are for ConformanceTestInterface*Request*.

It is incorrect to include the Response validator here.

(And the goal is to test our validation logic, not the ConformanceTestInterface
itself. We don't have to test its both Validate functions. Besides, the Response
validator is empty anyway.)

[hansmuller, 2014/08/14 00:57:56]

Done.

+    ]);
+  }
+
+  function testIntegrationMessageValidation() {
+    testMessageValidation("integration_", [

[yzshen1, 2014/08/14 00:06:32]

The goal of the integration tests is to make sure validators are hooked up with
the router correctly. This test is not doing that. Maybe we can do the
integration tests in the next CL, when the router actually uses the payload
validators?

+      testInterface.validateIntegrationTestInterface1Request,

[yzshen1, 2014/08/14 00:06:32]

The tests are designed for a message pipe endpoint receiving Interface1 request
and Interface2 response.

It doesn't make sense to add all the four payload validators.

[hansmuller, 2014/08/14 00:57:57]

I hadn't completely understood the scope of message names, thanks for clearing
that up.  I've just included the two filter methods you suggested.

+      testInterface.validateIntegrationTestInterface1Response,
+      testInterface.validateIntegrationTestInterface2Request,
+      testInterface.validateIntegrationTestInterface2Response
+    ]);
+  }
+
+  testConformanceMessageValidation();
+  testIntegrationMessageValidation();
   expect(checkTestMessageParser()).toBeNull();
   this.result = "PASS";
 });