Merge pull request #511 from Polymer/flaky-esm-404-test

Make esm-amd-loader dynamic require 404 test not flaky.

packages/esm-amd-loader/src/esm-amd-loader.ts

@@ -16,7 +16,8 @@ interface Window {
   define: ((deps: string[], moduleBody: OnExecutedCallback) => void)&{
     _reset?: () => void;
   };
-  HTMLImports?: { importForElement: (element: Element) => HTMLLinkElement | undefined };
+  HTMLImports?:
+      {importForElement: (element: Element) => HTMLLinkElement | undefined};
 }
 
 type OnExecutedCallback = (...args: Array<{}>) => void;
@@ -268,9 +269,9 @@ function loadDeps(
       args.push({
         // We append "#<script index>" to top-level scripts so that they have
         // unique keys in the registry. We don't want to see that here.
-        url: (module.isTopLevel === true)
-          ? module.url.substring(0, module.url.lastIndexOf('#'))
-          : module.url
+        url: (module.isTopLevel === true) ?
+            module.url.substring(0, module.url.lastIndexOf('#')) :
+            module.url
       });
       continue;
     }
@@ -555,33 +556,36 @@ function getBaseUrl(): NormalizedUrl {
 }
 
 /**
- * Get the url of the current document. If the document is the main document, the base
- * url is returned. Otherwise if the module was imported by a HTML import we need to
- * resolve the URL relative to the HTML import.
+ * Get the url of the current document. If the document is the main document,
+ * the base url is returned. Otherwise if the module was imported by a HTML
+ * import we need to resolve the URL relative to the HTML import.
  *
- * document.currentScript does not work in IE11, but the HTML import polyfill mocks it
- * when executing an import so for this case that's ok
+ * document.currentScript does not work in IE11, but the HTML import polyfill
+ * mocks it when executing an import so for this case that's ok
  */
 function getDocumentUrl() {
-  const { currentScript } = document;
+  const {currentScript} = document;
   // On IE11 document.currentScript is not defined when not in a HTML import
   if (!currentScript) {
     return baseUrl;
   }
 
   if (window.HTMLImports) {
-    // When the HTMLImports polyfill is active, we can take the path from the link element
+    // When the HTMLImports polyfill is active, we can take the path from the
+    // link element
     const htmlImport = window.HTMLImports.importForElement(currentScript);
     if (!htmlImport) {
-      // If there is no import for the current script, we are in the index.html. Take the base url.
+      // If there is no import for the current script, we are in the index.html.
+      // Take the base url.
       return baseUrl;
     }
 
     // Return the import href
     return htmlImport.href;
   } else {
-    // On chrome's native implementation it's not possible to get a direct reference to the link element,
-    // create an anchor and let the browser resolve the url.
+    // On chrome's native implementation it's not possible to get a direct
+    // reference to the link element, create an anchor and let the browser
+    // resolve the url.
     const a = currentScript.ownerDocument.createElement('a');
     a.href = '';
     return a.href;

packages/esm-amd-loader/test/src/suite.ts

@@ -268,19 +268,29 @@ suite('dynamic require', () => {
   });
 
   test('calls error callback only once on multiple 404s', (done) => {
-    let numErrors = 0;
+    let num404s = 0;
+    let numCallbackCalls = 0;
+
+    window.addEventListener('error', on404, true);
+
+    function on404() {
+      num404s++;
+      if (num404s === 2) {
+        window.removeEventListener('error', on404);
+        // Need a tick to ensure the loader error handlers have fired.
+        setTimeout(() => {
+          assert.equal(numCallbackCalls, 1);
+          done();
+        });
+      }
+    }
 
     define(['require'], (require: any) => {
       require(
           ['./not-found-a.js', './not-found-b.js'],
           () => assert.fail(),
-          () => numErrors++);
+          () => numCallbackCalls++);
     });
-
-    setTimeout(() => {
-      assert.equal(numErrors, 1);
-      done();
-    }, 1000);
   });
 });
 
@@ -407,16 +417,16 @@ suite('html imports', () => {
   }
 
   function testImport(href: string, expectedOrder: string[], done: () => void) {
-     // Each time an amd module in the chain is executed, it registers itself.
-     // If we've reached the length of modules we are expecing to be loaded,
-     // we check if the right modules were loaded in the expected order
-     window.addExecutedForImport = (name: string) => {
-       window.executionOrder.push(name);
-       if (window.executionOrder.length === expectedOrder.length) {
-         assert.deepEqual(window.executionOrder, expectedOrder);
-         done();
-       }
-     };
+    // Each time an amd module in the chain is executed, it registers itself.
+    // If we've reached the length of modules we are expecing to be loaded,
+    // we check if the right modules were loaded in the expected order
+    window.addExecutedForImport = (name: string) => {
+      window.executionOrder.push(name);
+      if (window.executionOrder.length === expectedOrder.length) {
+        assert.deepEqual(window.executionOrder, expectedOrder);
+        done();
+      }
+    };
 
     importHref(href);
   }
@@ -426,11 +436,17 @@ suite('html imports', () => {
   });
 
   test('modules inside deeper level html import', (done) => {
-    testImport('../html-import/y/deep-import.html', ['x', 'z', 'y', 'deep-import'], done);
+    testImport(
+        '../html-import/y/deep-import.html',
+        ['x', 'z', 'y', 'deep-import'],
+        done);
   });
 
   test('imports with child imports', (done) => {
-    testImport('../html-import/parent-import.html', ['z', 'y', 'child-import', 'x', 'parent-import'], done);
+    testImport(
+        '../html-import/parent-import.html',
+        ['z', 'y', 'child-import', 'x', 'parent-import'],
+        done);
   });
 
   test('import with meta', (done) => {