[Import Maps] Migrate Jest-based parsing tests into JSON-based

This CL - Converts Jest-based parsing tests into JSONs (with minor refinement, and removing test cases depending on interoperability issues of underlying URL parsers), - Adjust the JSON schema for parsing tests, and - Removes imported resolution tests. The test failures (virtual/import-maps-without-builtin-modules/external/wpt/import-maps/common/parsing.tentative.html) are due to WICG/import-maps#184, i.e. whether `null` mappings are removed from parsed import map or not. Bug: 1026809, WICG/import-maps#170 Change-Id: If1e53cbeafd44c1de1a7fa38ef646fcd5721b495 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1982509 Reviewed-by: Domenic Denicola <[email protected]> Commit-Queue: Hiroshige Hayashizaki <[email protected]> Cr-Commit-Position: refs/heads/master@{#731910}
web-platform-tests · Jan 15, 2020 · 9b27a5e · 9b27a5e
1 parent 61616b5
commit 9b27a5e
Show file tree

Hide file tree

Showing 23 changed files with 921 additions and 707 deletions.
diff --git a/import-maps/common/README.md b/import-maps/common/README.md
@@ -6,7 +6,12 @@ tests for the reference JavaScript implementation at [WICG repository](https://g
 
 ## Basics
 
-A **test object** describes a set of parameters (import maps and base URLs) and specifiers to be tested.
+A **test object** describes a set of parameters (import maps and base URLs) and test expectations.
+Test expectations can be:
+
+- Expected resulting URLs for specifiers (resolution tests), or
+- Expected parsed import maps (parsing tests).
+
 Each JSON file under [resources/](resources/) directory consists of a test object.
 A minimum test object would be:
 
@@ -33,10 +38,11 @@ Required fields:
     - In WPT tests, this is used for the test name of `promise_test()` together with specifier to be resolved, like `"Main test name: a"`.
 - `importMap` (object or string): the import map to be attached.
 - `importMapBaseURL` (string): the base URL used for [parsing the import map](https://wicg.github.io/import-maps/#parse-an-import-map-string).
-- `baseURL` (string): the base URL used in [resolving a specifier](https://wicg.github.io/import-maps/#resolve-a-module-specifier) for each specifiers.
-- `expectedResults` (object; string to (string or null)): test cases.
+- (resolution tests only) `expectedResults` (object; string to (string or null)): resolution test cases.
     - The keys are specifiers to be resolved.
     - The values are expected resolved URLs. If `null`, resolution should fail.
+- (resolution tests only) `baseURL` (string): the base URL used in [resolving a specifier](https://wicg.github.io/import-maps/#resolve-a-module-specifier) for each specifiers.
+- (parsing tests only) `expectedParsedImportMap` (object): the expected parsed import map for parsing test cases.
 
 Optional fields:
 

diff --git a/import-maps/common/parsing.tentative.html b/import-maps/common/parsing.tentative.html
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<meta name="timeout" content="long">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<body>
+<script type="module">
+import { runTestsFromJSON } from "./resources/common-test-helper.js";
+
+for (const json of [
+  'resources/parsing-addresses-absolute.json',
+  'resources/parsing-addresses-invalid.json',
+  'resources/parsing-addresses.json',
+  'resources/parsing-invalid-json.json',
+  'resources/parsing-schema-normalization.json',
+  'resources/parsing-schema-scope.json',
+  'resources/parsing-schema-specifier-map.json',
+  'resources/parsing-schema-toplevel.json',
+  'resources/parsing-scope-keys.json',
+  'resources/parsing-specifier-keys.json',
+  'resources/parsing-trailing-slashes.json',
+]) {
+  promise_test(() =>
+    runTestsFromJSON(json),
+    "Test helper: fetching and sanity checking test JSON: " + json);
+}
+</script>
diff --git a/import-maps/common/resources/common-test-helper.js b/import-maps/common/resources/common-test-helper.js
@@ -2,17 +2,15 @@ setup({allow_uncaught_exception : true});
 
 // Creates a new Document (via <iframe>) and add an inline import map.
 function parse(importMap, importMapBaseURL) {
-  return new Promise((resolve, reject) => {
+  return new Promise(resolve => {
     const importMapString = JSON.stringify(importMap);
     const iframe = document.createElement('iframe');
 
     window.addEventListener('message', event => {
-        if (event.data.type === 'Success') {
-          resolve(iframe);
-        } else {
-          // Currently we don't distinguish fetch errors and parse errors.
-          reject(event.data.error);
-        }
+        // Parsing result is saved here and checked later, rather than
+        // rejecting the promise on errors.
+        iframe.parseImportMapResult = event.data.type;
+        resolve(iframe);
       },
       {once: true});
 
@@ -40,18 +38,28 @@ function parse(importMap, importMapBaseURL) {
       // Handle specifier resolution requests from the parent frame.
       window.addEventListener('message', event => {
         try {
-          // URL resolution is tested using Chromium's internals.
-          // TODO(hiroshige): Remove the Chromium-specific dependency.
-          const result = internals.resolveModuleSpecifier(
-              event.data.specifier,
-              event.data.baseURL,
-              document);
-          parent.postMessage({type: 'ResolutionSuccess', result: result}, '*');
+          if (event.data.action === 'resolve') {
+            // URL resolution is tested using Chromium's internals.
+            // TODO(hiroshige): Remove the Chromium-specific dependency.
+            const result = internals.resolveModuleSpecifier(
+                event.data.specifier,
+                event.data.baseURL,
+                document);
+            parent.postMessage({type: 'ResolutionSuccess', result: result}, '*');
+         } else if (event.data.action === 'getParsedImportMap') {
+           parent.postMessage({
+               type: 'GetParsedImportMapSuccess',
+               result: internals.getParsedImportMap(document)}, '*');
+         } else {
+           parent.postMessage({
+               type: 'Failure',
+               result: "Invalid Action: " + event.data.action}, '*');
+         }
         } catch (e) {
           // We post error names instead of error objects themselves and
           // re-create error objects later, to avoid issues around serializing
           // error objects which is a quite new feature.
-          parent.postMessage({type: 'ResolutionFailure', result: e.name}, '*');
+          parent.postMessage({type: 'Failure', result: e.name}, '*');
         }
       });
       </script>
@@ -77,7 +85,7 @@ function resolve(specifier, parsedImportMap, baseURL) {
     window.addEventListener('message', event => {
         if (event.data.type === 'ResolutionSuccess') {
           resolve(event.data.result);
-        } else if (event.data.type === 'ResolutionFailure') {
+        } else if (event.data.type === 'Failure') {
           if (event.data.result === 'TypeError') {
             reject(new TypeError());
           } else {
@@ -90,7 +98,21 @@ function resolve(specifier, parsedImportMap, baseURL) {
       {once: true});
 
     parsedImportMap.contentWindow.postMessage(
-        {specifier: specifier, baseURL: baseURL}, '*');
+        {action: "resolve", specifier: specifier, baseURL: baseURL}, '*');
+  });
+}
+
+// Returns a promise that is resolved with a serialized string of
+// a parsed import map JSON object.
+function getParsedImportMap(parsedImportMap) {
+  return new Promise((resolve, reject) => {
+    window.addEventListener('message', event => {
+        resolve(event.data.result);
+      },
+      {once: true});
+
+    parsedImportMap.contentWindow.postMessage(
+        {action: "getParsedImportMap"}, '*');
   });
 }
 
@@ -101,11 +123,27 @@ function assert_no_extra_properties(object, expectedProperties, description) {
   }
 }
 
+// Sort keys and then stringify for comparison.
+function stringifyImportMap(importMap) {
+  function getKeys(m) {
+    if (typeof m !== 'object')
+      return [];
+
+    let keys = [];
+    for (const key in m) {
+      keys.push(key);
+      keys = keys.concat(getKeys(m[key]));
+    }
+    return keys;
+  }
+  return JSON.stringify(importMap, getKeys(importMap).sort());
+}
+
 async function runTests(j) {
   const tests = j.tests;
   delete j.tests;
 
-  if (j.importMap) {
+  if (j.hasOwnProperty('importMap')) {
     assert_own_property(j, 'importMap');
     assert_own_property(j, 'importMapBaseURL');
     j.parsedImportMap = await parse(j.importMap, j.importMapBaseURL);
@@ -115,7 +153,8 @@ async function runTests(j) {
 
   assert_no_extra_properties(
       j,
-      ['expectedResults', 'baseURL', 'name', 'parsedImportMap',
+      ['expectedResults', 'expectedParsedImportMap',
+      'baseURL', 'name', 'parsedImportMap',
       'importMap', 'importMapBaseURL',
       'link', 'details'],
       j.name);
@@ -133,26 +172,51 @@ async function runTests(j) {
     }
   } else {
     // Leaf node.
-    for (const key of
-        ['expectedResults', 'parsedImportMap', 'baseURL', 'name']) {
+    for (const key of ['parsedImportMap', 'name']) {
       assert_own_property(j, key, j.name);
     }
+    assert_true(j.hasOwnProperty('expectedResults') ||
+                j.hasOwnProperty('expectedParsedImportMap'),
+                'expectedResults or expectedParsedImportMap should exist');
+
+    // Resolution tests.
+    if (j.hasOwnProperty('expectedResults')) {
+      assert_own_property(j, 'baseURL');
+      assert_equals(
+          j.parsedImportMap.parseImportMapResult,
+          "Success",
+          "Import map registration should be successful for resolution tests");
+      for (const specifier in j.expectedResults) {
+        const expected = j.expectedResults[specifier];
+        promise_test(async t => {
+            if (expected === null) {
+              return promise_rejects(t, new TypeError(),
+                  resolve(specifier, j.parsedImportMap, j.baseURL));
+            } else {
+              // Should be resolved to `expected`.
+              const actual = await resolve(
+                  specifier, j.parsedImportMap, j.baseURL);
+              assert_equals(actual, expected);
+            }
+          },
+          j.name + ': ' + specifier);
+      }
+    }
 
-    for (const specifier in j.expectedResults) {
-      const expected = j.expectedResults[specifier];
+    // Parsing tests.
+    if (j.hasOwnProperty('expectedParsedImportMap')) {
       promise_test(async t => {
-          if (expected === null) {
-            return promise_rejects(t, new TypeError(),
-                resolve(specifier, j.parsedImportMap, j.baseURL));
-          } else {
-            // Should be resolved to `expected`.
-            const actual = await resolve(
-                specifier, j.parsedImportMap, j.baseURL);
-            assert_equals(actual, expected);
-          }
-        },
-        j.name + ': ' + specifier);
+        if (j.expectedParsedImportMap === null) {
+          assert_equals(j.parsedImportMap.parseImportMapResult, "ParseError");
+        } else {
+          const actualParsedImportMap =
+              await getParsedImportMap(j.parsedImportMap);
+          assert_equals(stringifyImportMap(JSON.parse(actualParsedImportMap)),
+                        stringifyImportMap(j.expectedParsedImportMap));
+        }
+      }, j.name);
     }
+
   }
 }
 

diff --git a/import-maps/common/resources/parsing-addresses-absolute.json b/import-maps/common/resources/parsing-addresses-absolute.json
@@ -0,0 +1,63 @@
+{
+  "name": "Absolute URL addresses",
+  "tests": {
+    "should only accept absolute URL addresses with fetch schemes": {
+      "importMap": {
+        "imports": {
+          "about": "about:good",
+          "blob": "blob:good",
+          "data": "data:good",
+          "file": "file:///good",
+          "filesystem": "filesystem:http://example.com/good/",
+          "http": "http://good/",
+          "https": "https://good/",
+          "ftp": "ftp://good/",
+          "import": "import:bad",
+          "mailto": "mailto:bad",
+          "javascript": "javascript:bad",
+          "wss": "wss:bad"
+        }
+      },
+      "importMapBaseURL": "https://base.example/path1/path2/path3",
+      "expectedParsedImportMap": {
+        "imports": {
+          "about": "about:good",
+          "blob": "blob:good",
+          "data": "data:good",
+          "file": "file:///good",
+          "filesystem": "filesystem:http://example.com/good/",
+          "http": "http://good/",
+          "https": "https://good/",
+          "ftp": "ftp://good/",
+          "import": "import:bad",
+          "javascript": "javascript:bad",
+          "mailto": "mailto:bad",
+          "wss": "wss://bad/"
+        },
+        "scopes": {}
+      }
+    },
+    "should parse absolute URLs, ignoring unparseable ones": {
+      "importMap": {
+        "imports": {
+          "unparseable2": "https://example.com:demo",
+          "unparseable3": "http://[www.example.com]/",
+          "invalidButParseable1": "https:example.org",
+          "invalidButParseable2": "https://///example.com///",
+          "prettyNormal": "https://example.net",
+          "percentDecoding": "https://ex%41mple.com/"
+        }
+      },
+      "importMapBaseURL": "https://base.example/path1/path2/path3",
+      "expectedParsedImportMap": {
+        "imports": {
+          "invalidButParseable1": "https://example.org/",
+          "invalidButParseable2": "https://example.com///",
+          "prettyNormal": "https://example.net/",
+          "percentDecoding": "https://example.com/"
+        },
+        "scopes": {}
+      }
+    }
+  }
+}
diff --git a/import-maps/common/resources/parsing-addresses-invalid.json b/import-maps/common/resources/parsing-addresses-invalid.json
@@ -0,0 +1,21 @@
+{
+  "name": "Other invalid addresses",
+  "tests": {
+    "should ignore unprefixed strings that are not absolute URLs": {
+      "importMap": {
+        "imports": {
+          "foo1": "bar",
+          "foo2": "\\bar",
+          "foo3": "~bar",
+          "foo4": "#bar",
+          "foo5": "?bar"
+        }
+      },
+      "importMapBaseURL": "https://base.example/path1/path2/path3",
+      "expectedParsedImportMap": {
+        "imports": {},
+        "scopes": {}
+      }
+    }
+  }
+}