Merge pull request #145 from mdboom/sequence-promises

Ensure that package loading runs in sequence

src/pyodide.js

@@ -18,6 +18,7 @@ var languagePluginLoader = new Promise((resolve, reject) => {
   // Package loading
   var packages = undefined;
   let loadedPackages = new Array();
+  var loadPackagePromise = new Promise((resolve) => resolve());
 
   let _uri_to_package_name = (package_uri) => {
     // Generate a unique package name from URI
@@ -33,7 +34,7 @@ var languagePluginLoader = new Promise((resolve, reject) => {
     }
   };
 
-  let loadPackage = (names) => {
+  let _loadPackage = (names) => {
     // DFS to find all dependencies of the requested packages
     let packages = window.pyodide.packages.dependencies;
     let queue = [].concat(names || []);
@@ -44,24 +45,25 @@ var languagePluginLoader = new Promise((resolve, reject) => {
       const package = _uri_to_package_name(package_uri);
 
       if (package == null) {
-        throw new Error(`Invalid package name or URI '${package_uri}'`);
+        console.error(`Invalid package name or URI '${package_uri}'`);
+        return;
       } else if (package == package_uri) {
         package_uri = 'default channel';
       }
 
       if (package in loadedPackages) {
         if (package_uri != loadedPackages[package]) {
-          throw new Error(
-              `URI mismatch, attempting to load package ` +
-              `${package} from ${package_uri} while it is already ` +
-              `loaded from ${loadedPackages[package]}!`);
+          console.error(`URI mismatch, attempting to load package ` +
+                        `${package} from ${package_uri} while it is already ` +
+                        `loaded from ${loadedPackages[package]}!`);
+          return;
         }
       } else if (package in toLoad) {
         if (package_uri != toLoad[package]) {
-          throw new Error(
-              `URI mismatch, attempting to load package ` +
-              `${package} from ${package_uri} while it is already ` +
-              `being loaded from ${toLoad[package]}!`);
+          console.error(`URI mismatch, attempting to load package ` +
+                        `${package} from ${package_uri} while it is already ` +
+                        `being loaded from ${toLoad[package]}!`);
+          return;
         }
       } else {
         console.log(`Loading ${package} from ${package_uri}`);
@@ -121,6 +123,13 @@ var languagePluginLoader = new Promise((resolve, reject) => {
     return promise;
   };
 
+  let loadPackage = (names) => {
+    /* We want to make sure that only one loadPackage invocation runs at any
+     * given time, so this creates a "chain" of promises. */
+    loadPackagePromise = loadPackagePromise.then(() => _loadPackage(names));
+    return loadPackagePromise;
+  };
+
   function fixRecursionLimit(pyodide) {
     // The Javascript/Wasm call stack may be too small to handle the default
     // Python call stack limit of 1000 frames. This is generally the case on

src/test.html

@@ -13,6 +13,9 @@
          console.info = function(message) {
              window.logs.push(message);
          }
+         console.error = function(message) {
+             window.logs.push(message);
+         }
         </script>
         <script src="pyodide_dev.js"></script>
     </head>

test/conftest.py

@@ -91,12 +91,15 @@ class SeleniumWrapper:
         return self.driver.execute_script(catch)
 
     def load_package(self, packages):
-        from selenium.common.exceptions import TimeoutException
-
         self.run_js(
             'window.done = false\n' +
             'pyodide.loadPackage({!r})'.format(packages) +
-            '.then(function() { window.done = true; })')
+            '.finally(function() { window.done = true; })')
+        self.wait_until_packages_loaded()
+
+    def wait_until_packages_loaded(self):
+        from selenium.common.exceptions import TimeoutException
+
         try:
             self.wait.until(PackageLoaded())
         except TimeoutException as exc:

test/test_package_loading.py

 import pytest
-from selenium.common.exceptions import WebDriverException
 
 
 def test_load_from_url(selenium_standalone, web_server):
@@ -18,22 +17,19 @@ def test_load_from_url(selenium_standalone, web_server):
 
 def test_uri_mismatch(selenium_standalone):
     selenium_standalone.load_package('pyparsing')
-    with pytest.raises(WebDriverException,
-                       match="URI mismatch, attempting "
-                             "to load package pyparsing"):
-        selenium_standalone.load_package('http://some_url/pyparsing.js')
+    selenium_standalone.load_package('http://some_url/pyparsing.js')
+    assert ("URI mismatch, attempting to load package pyparsing" in
+            selenium_standalone.logs)
     assert "Invalid package name or URI" not in selenium_standalone.logs
 
 
 def test_invalid_package_name(selenium):
-    with pytest.raises(WebDriverException,
-                       match="Invalid package name or URI"):
-        selenium.load_package('wrong name+$')
+    selenium.load_package('wrong name+$')
+    assert "Invalid package name or URI" in selenium.logs
     selenium.clean_logs()
 
-    with pytest.raises(WebDriverException,
-                       match="Invalid package name or URI"):
-        selenium.load_package('tcp://some_url')
+    selenium.load_package('tcp://some_url')
+    assert "Invalid package name or URI" in selenium.logs
 
 
 @pytest.mark.parametrize('packages', [['pyparsing', 'pytz'],
@@ -44,7 +40,29 @@ def test_load_packages_multiple(selenium_standalone, packages):
     selenium.load_package(packages)
     selenium.run(f'import {packages[0]}')
     selenium.run(f'import {packages[1]}')
-    # The long must show that each package is loaded exactly once,
+    # The log must show that each package is loaded exactly once,
+    # including when one package is a dependency of the other
+    # ('pyparsing' and 'matplotlib')
+    assert selenium.logs.count(f'Loading {packages[0]}') == 1
+    assert selenium.logs.count(f'Loading {packages[1]}') == 1
+
+
+@pytest.mark.parametrize('packages', [['pyparsing', 'pytz'],
+                                      ['pyparsing', 'matplotlib']],
+                         ids='-'.join)
+def test_load_packages_sequential(selenium_standalone, packages):
+    selenium = selenium_standalone
+    promises = ','.join(
+        'pyodide.loadPackage("{}")'.format(x) for x in packages
+    )
+    selenium.run_js(
+        'window.done = false\n' +
+        'Promise.all([{}])'.format(promises) +
+        '.finally(function() { window.done = true; })')
+    selenium.wait_until_packages_loaded()
+    selenium.run(f'import {packages[0]}')
+    selenium.run(f'import {packages[1]}')
+    # The log must show that each package is loaded exactly once,
     # including when one package is a dependency of the other
     # ('pyparsing' and 'matplotlib')
     assert selenium.logs.count(f'Loading {packages[0]}') == 1