From f50275335d50758c28fc5e5d9ef1397c9aef47a7 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Wed, 22 Aug 2018 13:47:04 +0300 Subject: [PATCH 01/12] Checking valid package name / url --- src/pyodide.js | 7 +++++++ test/conftest.py | 5 +++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/pyodide.js b/src/pyodide.js index f5d85e074..cbc677b78 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -20,6 +20,13 @@ var languagePluginLoader = new Promise((resolve, reject) => { let toLoad = new Set(); while (queue.length) { const package = queue.pop(); + var valid_package_name_regexp = new RegExp('^[a-zA-Z0-9_\-]+$'); + console.log(package + valid_package_name_regexp.test(package)); + if (!valid_package_name_regexp.test(package)) { + console.log(`Invalid package name '${package}'`); + break; + } + if (!loadedPackages.has(package)) { toLoad.add(package); if (packages.hasOwnProperty(package)) { diff --git a/test/conftest.py b/test/conftest.py index a0e6e13ac..a904606c9 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -50,7 +50,8 @@ class SeleniumWrapper: @property def logs(self): - return self.driver.execute_script("return window.logs") + logs = self.driver.execute_script("return window.logs") + return '\n'.join(str(x) for x in logs) def run(self, code): return self.run_js( @@ -123,7 +124,7 @@ if pytest is not None: try: yield selenium finally: - print('\n'.join(str(x) for x in selenium.logs)) + print(selenium.logs) selenium.driver.quit() @pytest.fixture(params=['firefox', 'chrome'], scope='module') From 4f665e0a3a9217c40184d7630d46b3c6baf70930 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Wed, 22 Aug 2018 15:37:23 +0300 Subject: [PATCH 02/12] Validate package URI --- src/pyodide.js | 35 ++++++++++++++++++++++++++++++----- test/conftest.py | 8 +++++--- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/pyodide.js b/src/pyodide.js index cbc677b78..8b46a19a9 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -2,6 +2,13 @@ * The main bootstrap script for loading pyodide. */ +// Regexp for validating package name and URI +var package_name_regexp = '[a-zA-Z0-9_\-]+' +var package_uri_regexp = new RegExp( + '^(?:https?|file)://.*?(' + package_name_regexp + ').js$'); +var package_name_regexp = new RegExp('^' + package_name_regexp + '$'); + + var languagePluginLoader = new Promise((resolve, reject) => { // This is filled in by the Makefile to be either a local file or the // deployed location. TODO: This should be done in a less hacky @@ -13,20 +20,38 @@ var languagePluginLoader = new Promise((resolve, reject) => { var packages = undefined; let loadedPackages = new Set(); + let _uri_to_package_name = (package_uri) => { + // Generate a unique package name from URI + + if (package_name_regexp.test(package_uri)) { + return package_uri; + } else if (package_uri_regexp.test(package_uri)) { + var match = package_uri_regexp.exec(package_uri); + // Get the regexp group corresponding to the package name + return match[1]; + } else { + return null; + } + }; + + let loadPackage = (names) => { // DFS to find all dependencies of the requested packages let packages = window.pyodide.packages.dependencies; let queue = new Array(names); let toLoad = new Set(); while (queue.length) { - const package = queue.pop(); - var valid_package_name_regexp = new RegExp('^[a-zA-Z0-9_\-]+$'); - console.log(package + valid_package_name_regexp.test(package)); - if (!valid_package_name_regexp.test(package)) { - console.log(`Invalid package name '${package}'`); + const package_uri = queue.pop(); + + const package = _uri_to_package_name(package_uri); + + if (package == null) { + console.log(`Invalid package name or URI '${package_uri}'`); break; } + console.log(`Loading ${package} from ${package_uri}`); + if (!loadedPackages.has(package)) { toLoad.add(package); if (packages.hasOwnProperty(package)) { diff --git a/test/conftest.py b/test/conftest.py index a904606c9..1d1b93bf9 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -53,6 +53,9 @@ class SeleniumWrapper: logs = self.driver.execute_script("return window.logs") return '\n'.join(str(x) for x in logs) + def clean_logs(self): + self.driver.execute_script("window.logs = []") + def run(self, code): return self.run_js( 'return pyodide.runPython({!r})'.format(code)) @@ -145,11 +148,10 @@ if pytest is not None: def selenium(_selenium_cached): # selenium instance cached at the module level try: - # clean selenium logs for each test run - _selenium_cached.driver.execute_script("window.logs = []") + _selenium_cached.clean_logs() yield _selenium_cached finally: - print('\n'.join(str(x) for x in _selenium_cached.logs)) + print(_selenium_cached.logs) PORT = 0 From 2eeb42ebf1d55b0a8905efaa1327cd872e7ad31b Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Wed, 22 Aug 2018 16:28:45 +0300 Subject: [PATCH 03/12] Detect repeated loading of the same package from multiple URI --- src/pyodide.js | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/pyodide.js b/src/pyodide.js index 8b46a19a9..30538b0ed 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -18,7 +18,7 @@ var languagePluginLoader = new Promise((resolve, reject) => { //////////////////////////////////////////////////////////// // Package loading var packages = undefined; - let loadedPackages = new Set(); + let loadedPackages = new Array(); let _uri_to_package_name = (package_uri) => { // Generate a unique package name from URI @@ -41,22 +41,30 @@ var languagePluginLoader = new Promise((resolve, reject) => { let queue = new Array(names); let toLoad = new Set(); while (queue.length) { - const package_uri = queue.pop(); + var package_uri = queue.pop(); const package = _uri_to_package_name(package_uri); if (package == null) { console.log(`Invalid package name or URI '${package_uri}'`); break; + } else if (package == package_uri) { + package_uri = 'packages.json'; } console.log(`Loading ${package} from ${package_uri}`); - if (!loadedPackages.has(package)) { + if (package in loadedPackages) { + if (package_uri != loadedPackages[package]) { + console.log(`Error: URI mismatch, attempting to load package ` + + `${package} from ${package_uri} while is already ` + + `loaded from ${loadedPackages[package]}!`); + } + } else { toLoad.add(package); if (packages.hasOwnProperty(package)) { packages[package].forEach((subpackage) => { - if (!loadedPackages.has(subpackage) && !toLoad.has(subpackage)) { + if (!(subpackage in loadedPackages) && !toLoad.has(subpackage)) { queue.push(subpackage); } }); @@ -73,7 +81,9 @@ var languagePluginLoader = new Promise((resolve, reject) => { pyodide.monitorRunDependencies = (n) => { if (n === 0) { - toLoad.forEach((package) => loadedPackages.add(package)); + toLoad.forEach((package) => { + loadedPackages[package] = package; + }); delete pyodide.monitorRunDependencies; const packageList = Array.from(toLoad.keys()).join(', '); resolve(`Loaded ${packageList}`); From b2156caa94040205b1356c5100bac398c3160a24 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Wed, 22 Aug 2018 16:46:31 +0300 Subject: [PATCH 04/12] Loading packages from custom URLs --- src/pyodide.js | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/pyodide.js b/src/pyodide.js index 30538b0ed..0fb24fc69 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -26,7 +26,7 @@ var languagePluginLoader = new Promise((resolve, reject) => { if (package_name_regexp.test(package_uri)) { return package_uri; } else if (package_uri_regexp.test(package_uri)) { - var match = package_uri_regexp.exec(package_uri); + let match = package_uri_regexp.exec(package_uri); // Get the regexp group corresponding to the package name return match[1]; } else { @@ -39,17 +39,17 @@ var languagePluginLoader = new Promise((resolve, reject) => { // DFS to find all dependencies of the requested packages let packages = window.pyodide.packages.dependencies; let queue = new Array(names); - let toLoad = new Set(); + let toLoad = new Array(); while (queue.length) { var package_uri = queue.pop(); const package = _uri_to_package_name(package_uri); if (package == null) { - console.log(`Invalid package name or URI '${package_uri}'`); - break; - } else if (package == package_uri) { - package_uri = 'packages.json'; + console.log(`Invalid package name or URI '${package_uri}'`); + break; + } else if (package == package_uri) { + package_uri = 'packages.json'; } console.log(`Loading ${package} from ${package_uri}`); @@ -61,10 +61,10 @@ var languagePluginLoader = new Promise((resolve, reject) => { `loaded from ${loadedPackages[package]}!`); } } else { - toLoad.add(package); + toLoad[package] = package_uri; if (packages.hasOwnProperty(package)) { packages[package].forEach((subpackage) => { - if (!(subpackage in loadedPackages) && !toLoad.has(subpackage)) { + if (!(subpackage in loadedPackages) && !(subpackage in toLoad)) { queue.push(subpackage); } }); @@ -75,27 +75,32 @@ var languagePluginLoader = new Promise((resolve, reject) => { } let promise = new Promise((resolve, reject) => { - if (toLoad.size === 0) { + if (Object.keys(toLoad).length === 0) { resolve('No new packages to load'); } pyodide.monitorRunDependencies = (n) => { if (n === 0) { - toLoad.forEach((package) => { - loadedPackages[package] = package; - }); + for (let package in toLoad) { + loadedPackages[package] = toLoad[package]; + } delete pyodide.monitorRunDependencies; - const packageList = Array.from(toLoad.keys()).join(', '); + const packageList = Array.from(Object.keys(toLoad)).join(', '); resolve(`Loaded ${packageList}`); } }; - toLoad.forEach((package) => { + for (let package in toLoad) { let script = document.createElement('script'); - script.src = `${baseURL}${package}.js`; + let package_uri = toLoad[package]; + if (package_uri == 'packages.json') { + script.src = `${baseURL}${package}.js`; + } else { + script.src = `${package_uri}`; + } script.onerror = (e) => { reject(e); }; document.body.appendChild(script); - }); + } // We have to invalidate Python's import caches, or it won't // see the new files. This is done here so it happens in parallel From 068906bbf7aa69b7e08ee787dd994c2b9a0057f4 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Wed, 22 Aug 2018 16:48:47 +0300 Subject: [PATCH 05/12] Unit tests for loading packages from user defined URLs --- src/pyodide.js | 6 ++--- test/test_package_loading.py | 50 ++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 test/test_package_loading.py diff --git a/src/pyodide.js b/src/pyodide.js index 0fb24fc69..910b98907 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -5,7 +5,7 @@ // Regexp for validating package name and URI var package_name_regexp = '[a-zA-Z0-9_\-]+' var package_uri_regexp = new RegExp( - '^(?:https?|file)://.*?(' + package_name_regexp + ').js$'); + '^https?://.*?(' + package_name_regexp + ').js$'); var package_name_regexp = new RegExp('^' + package_name_regexp + '$'); @@ -41,7 +41,7 @@ var languagePluginLoader = new Promise((resolve, reject) => { let queue = new Array(names); let toLoad = new Array(); while (queue.length) { - var package_uri = queue.pop(); + let package_uri = queue.pop(); const package = _uri_to_package_name(package_uri); @@ -57,7 +57,7 @@ var languagePluginLoader = new Promise((resolve, reject) => { if (package in loadedPackages) { if (package_uri != loadedPackages[package]) { console.log(`Error: URI mismatch, attempting to load package ` + - `${package} from ${package_uri} while is already ` + + `${package} from ${package_uri} while it is already ` + `loaded from ${loadedPackages[package]}!`); } } else { diff --git a/test/test_package_loading.py b/test/test_package_loading.py new file mode 100644 index 000000000..f403103cd --- /dev/null +++ b/test/test_package_loading.py @@ -0,0 +1,50 @@ +import os +from pathlib import Path +import time + +from http.server import HTTPServer, SimpleHTTPRequestHandler +import threading + +import pytest + + +@pytest.fixture +def static_web_server(): + # serve artefacts from a different port + build_dir = Path(__file__).parent.parent / 'build' + os.chdir(build_dir) + try: + url, port = ('127.0.0.1', 8888) + server = HTTPServer((url, port), SimpleHTTPRequestHandler) + thread = threading.Thread(target=server.serve_forever) + thread.start() + yield url, port + finally: + server.shutdown() + + +def test_load_from_url(selenium_standalone, static_web_server): + + url, port = static_web_server + + selenium_standalone.load_package(f"http://{url}:{port}/pyparsing.js") + assert "Invalid package name or URI" not in selenium_standalone.logs + + selenium_standalone.run("from pyparsing import Word, alphas") + selenium_standalone.run("Word(alphas).parseString('hello')") + + +def test_uri_mismatch(selenium_standalone): + selenium_standalone.load_package('pyparsing') + selenium_standalone.load_package('http://some_url/pyparsing.js') + assert "Invalid package name or URI" not in selenium_standalone.logs + assert ("URI mismatch, attempting " + "to load package pyparsing") in selenium_standalone.logs + + +def test_invalid_package_name(selenium): + selenium.load_package('wrong name+$') + assert "Invalid package name or URI" in selenium.logs + selenium.clean_logs() + selenium.load_package('tcp://some_url') + assert "Invalid package name or URI" in selenium.logs From 99772a9ac81d8d227c0621f0ec8f0b529423e824 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Wed, 22 Aug 2018 17:35:40 +0300 Subject: [PATCH 06/12] Case insensitive packages / URLs --- src/pyodide.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pyodide.js b/src/pyodide.js index 910b98907..3a28862ec 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -3,10 +3,10 @@ */ // Regexp for validating package name and URI -var package_name_regexp = '[a-zA-Z0-9_\-]+' +var package_name_regexp = '[a-z0-9_\-]+' var package_uri_regexp = new RegExp( - '^https?://.*?(' + package_name_regexp + ').js$'); -var package_name_regexp = new RegExp('^' + package_name_regexp + '$'); + '^https?://.*?(' + package_name_regexp + ').js$', 'i'); +var package_name_regexp = new RegExp('^' + package_name_regexp + '$', 'i'); var languagePluginLoader = new Promise((resolve, reject) => { From ff45a550a6193fcae7645c40b08358d46976b1b3 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Wed, 22 Aug 2018 17:52:03 +0300 Subject: [PATCH 07/12] Flake8 and store artefacts in CircleCI --- .circleci/config.yml | 3 +++ test/test_package_loading.py | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index f5ccd8a0d..ed0c80730 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -111,6 +111,9 @@ jobs: export PATH=$PWD/firefox:$PATH pytest test -v -k chrome + - store_artifacts: + path: /home/circleci/repo/build/ + deploy: machine: enabled: true diff --git a/test/test_package_loading.py b/test/test_package_loading.py index f403103cd..590d27e90 100644 --- a/test/test_package_loading.py +++ b/test/test_package_loading.py @@ -1,6 +1,5 @@ import os from pathlib import Path -import time from http.server import HTTPServer, SimpleHTTPRequestHandler import threading From 5030eae5b716e1f778e479650689a450a52a0fa0 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Thu, 23 Aug 2018 11:32:52 +0300 Subject: [PATCH 08/12] Re-use built-in web server --- test/conftest.py | 5 +++++ test/test_package_loading.py | 29 +++++------------------------ 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index 1d1b93bf9..9d11f7307 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -205,5 +205,10 @@ def run_web_server(q): httpd.serve_forever() +@pytest.fixture +def web_server(): + return '127.0.0.1', PORT + + if multiprocessing.current_process().name == 'MainProcess': spawn_web_server() diff --git a/test/test_package_loading.py b/test/test_package_loading.py index 590d27e90..52f0d8338 100644 --- a/test/test_package_loading.py +++ b/test/test_package_loading.py @@ -1,30 +1,8 @@ -import os -from pathlib import Path - -from http.server import HTTPServer, SimpleHTTPRequestHandler -import threading - -import pytest -@pytest.fixture -def static_web_server(): - # serve artefacts from a different port - build_dir = Path(__file__).parent.parent / 'build' - os.chdir(build_dir) - try: - url, port = ('127.0.0.1', 8888) - server = HTTPServer((url, port), SimpleHTTPRequestHandler) - thread = threading.Thread(target=server.serve_forever) - thread.start() - yield url, port - finally: - server.shutdown() +def test_load_from_url(selenium_standalone, web_server): - -def test_load_from_url(selenium_standalone, static_web_server): - - url, port = static_web_server + url, port = web_server selenium_standalone.load_package(f"http://{url}:{port}/pyparsing.js") assert "Invalid package name or URI" not in selenium_standalone.logs @@ -32,6 +10,9 @@ def test_load_from_url(selenium_standalone, static_web_server): selenium_standalone.run("from pyparsing import Word, alphas") selenium_standalone.run("Word(alphas).parseString('hello')") + selenium_standalone.load_package(f"http://{url}:{port}/numpy.js") + selenium_standalone.run("import numpy as np") + def test_uri_mismatch(selenium_standalone): selenium_standalone.load_package('pyparsing') From f35aa2eede70a92bf7c7b30d9bf5954ab0a5ad7f Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Fri, 24 Aug 2018 18:43:59 +0300 Subject: [PATCH 09/12] More review comments --- src/pyodide.js | 2 +- test/test_python.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/pyodide.js b/src/pyodide.js index 3a28862ec..b922fdf0e 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -49,7 +49,7 @@ var languagePluginLoader = new Promise((resolve, reject) => { console.log(`Invalid package name or URI '${package_uri}'`); break; } else if (package == package_uri) { - package_uri = 'packages.json'; + package_uri = 'default channel'; } console.log(`Loading ${package} from ${package_uri}`); diff --git a/test/test_python.py b/test/test_python.py index 0e2e9d294..2b3be4367 100644 --- a/test/test_python.py +++ b/test/test_python.py @@ -7,7 +7,8 @@ import pytest def test_init(selenium_standalone): - assert 'Python initialization complete' in selenium_standalone.logs + assert ('Python initialization complete' + in selenium_standalone.logs.splitlines()) assert len(selenium_standalone.driver.window_handles) == 1 @@ -19,7 +20,7 @@ def test_webbrowser(selenium): def test_print(selenium): selenium.run("print('This should be logged')") - assert 'This should be logged' in selenium.logs + assert 'This should be logged' in selenium.logs.splitlines() def test_python2js(selenium): From c9d5ba3c8ed4d9906ef71256232ba18b3e4817d6 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Tue, 28 Aug 2018 17:30:46 +0300 Subject: [PATCH 10/12] Better logs when PyodideInited() or PackageLoaded() fails on Chrome --- src/pyodide.js | 14 ++++++-------- test/conftest.py | 26 ++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/pyodide.js b/src/pyodide.js index b922fdf0e..25691ffd6 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -4,11 +4,10 @@ // Regexp for validating package name and URI var package_name_regexp = '[a-z0-9_\-]+' -var package_uri_regexp = new RegExp( - '^https?://.*?(' + package_name_regexp + ').js$', 'i'); +var package_uri_regexp = + new RegExp('^https?://.*?(' + package_name_regexp + ').js$', 'i'); var package_name_regexp = new RegExp('^' + package_name_regexp + '$', 'i'); - var languagePluginLoader = new Promise((resolve, reject) => { // This is filled in by the Makefile to be either a local file or the // deployed location. TODO: This should be done in a less hacky @@ -34,7 +33,6 @@ var languagePluginLoader = new Promise((resolve, reject) => { } }; - let loadPackage = (names) => { // DFS to find all dependencies of the requested packages let packages = window.pyodide.packages.dependencies; @@ -82,7 +80,7 @@ var languagePluginLoader = new Promise((resolve, reject) => { pyodide.monitorRunDependencies = (n) => { if (n === 0) { for (let package in toLoad) { - loadedPackages[package] = toLoad[package]; + loadedPackages[package] = toLoad[package]; } delete pyodide.monitorRunDependencies; const packageList = Array.from(Object.keys(toLoad)).join(', '); @@ -93,10 +91,10 @@ var languagePluginLoader = new Promise((resolve, reject) => { for (let package in toLoad) { let script = document.createElement('script'); let package_uri = toLoad[package]; - if (package_uri == 'packages.json') { - script.src = `${baseURL}${package}.js`; + if (package_uri == 'default channel') { + script.src = `${baseURL}${package}.js`; } else { - script.src = `${package_uri}`; + script.src = `${package_uri}`; } script.onerror = (e) => { reject(e); }; document.body.appendChild(script); diff --git a/test/conftest.py b/test/conftest.py index 9d11f7307..7d988ede0 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -33,9 +33,20 @@ class PackageLoaded: return bool(inited) +def _display_driver_logs(browser, driver): + if browser == 'chrome': + print('# Selenium browser logs') + print(driver.get_log("browser")) + elif browser == 'firefox': + # browser logs are not available in GeckoDriver + # https://github.com/mozilla/geckodriver/issues/284 + print('Cannot access browser logs for Firefox.') + + class SeleniumWrapper: def __init__(self): from selenium.webdriver.support.wait import WebDriverWait + from selenium.common.exceptions import TimeoutException driver = self.get_driver() wait = WebDriverWait(driver, timeout=20) @@ -44,7 +55,11 @@ class SeleniumWrapper: raise ValueError(f"{(BUILD_PATH / 'test.html').resolve()} " f"does not exist!") driver.get(f'http://127.0.0.1:{PORT}/test.html') - wait.until(PyodideInited()) + try: + wait.until(PyodideInited()) + except TimeoutException as exc: + _display_driver_logs(self.browser, driver) + raise TimeoutException() self.wait = wait self.driver = driver @@ -68,11 +83,18 @@ 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; })') - self.wait.until(PackageLoaded()) + try: + self.wait.until(PackageLoaded()) + except TimeoutException as exc: + _display_driver_logs(self.browser, self.driver) + print(self.logs) + raise TimeoutException() @property def urls(self): From 11fcaf8da64ed816e858f3a9b1e179b19231b9de Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Tue, 28 Aug 2018 18:45:42 +0300 Subject: [PATCH 11/12] Throw error on invalid packages --- src/pyodide.js | 14 +++++++------- test/conftest.py | 3 ++- test/test_package_loading.py | 20 +++++++++++++------- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/pyodide.js b/src/pyodide.js index 25691ffd6..c2f1050a9 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -3,7 +3,7 @@ */ // Regexp for validating package name and URI -var package_name_regexp = '[a-z0-9_\-]+' +var package_name_regexp = '[a-z0-9_][a-z0-9_\-]*' var package_uri_regexp = new RegExp('^https?://.*?(' + package_name_regexp + ').js$', 'i'); var package_name_regexp = new RegExp('^' + package_name_regexp + '$', 'i'); @@ -44,8 +44,7 @@ var languagePluginLoader = new Promise((resolve, reject) => { const package = _uri_to_package_name(package_uri); if (package == null) { - console.log(`Invalid package name or URI '${package_uri}'`); - break; + throw new Error(`Invalid package name or URI '${package_uri}'`); } else if (package == package_uri) { package_uri = 'default channel'; } @@ -54,9 +53,10 @@ var languagePluginLoader = new Promise((resolve, reject) => { if (package in loadedPackages) { if (package_uri != loadedPackages[package]) { - console.log(`Error: URI mismatch, attempting to load package ` + - `${package} from ${package_uri} while it is already ` + - `loaded from ${loadedPackages[package]}!`); + throw new Error( + `URI mismatch, attempting to load package ` + + `${package} from ${package_uri} while it is already ` + + `loaded from ${loadedPackages[package]}!`); } } else { toLoad[package] = package_uri; @@ -67,7 +67,7 @@ var languagePluginLoader = new Promise((resolve, reject) => { } }); } else { - console.log(`Unknown package '${package}'`); + log.console(`Unknown package '${package}'`); } } } diff --git a/test/conftest.py b/test/conftest.py index 7d988ede0..9abca9395 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -40,7 +40,8 @@ def _display_driver_logs(browser, driver): elif browser == 'firefox': # browser logs are not available in GeckoDriver # https://github.com/mozilla/geckodriver/issues/284 - print('Cannot access browser logs for Firefox.') + print('Accessing raw browser logs with Selenium is not ' + 'supported by Firefox.') class SeleniumWrapper: diff --git a/test/test_package_loading.py b/test/test_package_loading.py index 52f0d8338..47e87d3df 100644 --- a/test/test_package_loading.py +++ b/test/test_package_loading.py @@ -1,3 +1,5 @@ +import pytest +from selenium.common.exceptions import WebDriverException def test_load_from_url(selenium_standalone, web_server): @@ -16,15 +18,19 @@ def test_load_from_url(selenium_standalone, web_server): def test_uri_mismatch(selenium_standalone): selenium_standalone.load_package('pyparsing') - selenium_standalone.load_package('http://some_url/pyparsing.js') + with pytest.raises(WebDriverException, + match="URI mismatch, attempting " + "to load package pyparsing"): + selenium_standalone.load_package('http://some_url/pyparsing.js') assert "Invalid package name or URI" not in selenium_standalone.logs - assert ("URI mismatch, attempting " - "to load package pyparsing") in selenium_standalone.logs def test_invalid_package_name(selenium): - selenium.load_package('wrong name+$') - assert "Invalid package name or URI" in selenium.logs + with pytest.raises(WebDriverException, + match="Invalid package name or URI"): + selenium.load_package('wrong name+$') selenium.clean_logs() - selenium.load_package('tcp://some_url') - assert "Invalid package name or URI" in selenium.logs + + with pytest.raises(WebDriverException, + match="Invalid package name or URI"): + selenium.load_package('tcp://some_url') From 2aa98eb6c576235bc026a9f00d1e383aa254b3e3 Mon Sep 17 00:00:00 2001 From: Roman Yurchak Date: Wed, 29 Aug 2018 11:40:10 +0300 Subject: [PATCH 12/12] Document loading packages from custom URLs --- docs/new_packages.md | 3 ++- docs/using_pyodide_from_iodide.md | 10 +++++++++- docs/using_pyodide_from_javascript.md | 10 +++++++++- src/pyodide.js | 2 +- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/docs/new_packages.md b/docs/new_packages.md index 940bd89fd..8d6cd7888 100644 --- a/docs/new_packages.md +++ b/docs/new_packages.md @@ -50,7 +50,8 @@ The supported keys in the `meta.yaml` file are described below. The name of the package. It must match the name of the package used when expanding the tarball, which is sometimes different from the name of the package in the Python namespace when installed. It must also match the name of the -directory in which the `meta.yaml` file is placed. +directory in which the `meta.yaml` file is placed. It can only contain +alpha-numeric characters and `-`, `_`. #### `package/version` diff --git a/docs/using_pyodide_from_iodide.md b/docs/using_pyodide_from_iodide.md index 58a24b471..89ee3a816 100644 --- a/docs/using_pyodide_from_iodide.md +++ b/docs/using_pyodide_from_iodide.md @@ -51,7 +51,15 @@ Pyodide. To use other libraries, you'll need to load their package using from a Javascript cell. This downloads the file data over the network (as a `.data` and `.js` index file) and installs the files in the virtual filesystem. -When you request a package, all of that package's dependencies are also loaded. +Packages can be loaded by name, for those included in the official pyodide +repository (e.g. `pyodide.loadPackage('numpy')`). It is also possible to load +packages from custom URLs (e.g. +`pyodide.loadPackage('https://foo/bar/numpy.js')`), in which case the URL must +end with `.js`. + +When you request a package from the official repository, all of that package's +dependencies are also loaded. Dependency resolution is not yet implemented +when loading packages from custom URLs. `pyodide.loadPackage` returns a `Promise`. diff --git a/docs/using_pyodide_from_javascript.md b/docs/using_pyodide_from_javascript.md index 7d9c1d0fa..4e0bc67df 100644 --- a/docs/using_pyodide_from_javascript.md +++ b/docs/using_pyodide_from_javascript.md @@ -39,7 +39,15 @@ Pyodide. To use other libraries, you'll need to load their package using `pyodide.loadPackage`. This downloads the file data over the network (as a `.data` and `.js` index file) and installs the files in the virtual filesystem. -When you request a package, all of that package's dependencies are also loaded. +Packages can be loaded by name, for those included in the official pyodide +repository (e.g. `pyodide.loadPackage('numpy')`). It is also possible to load +packages from custom URLs (e.g. +`pyodide.loadPackage('https://foo/bar/numpy.js')`), in which case the URL must +end with `.js`. + +When you request a package from the official repository, all of that package's +dependencies are also loaded. Dependency resolution is not yet implemented +when loading packages from custom URLs. `pyodide.loadPackage` returns a `Promise`. diff --git a/src/pyodide.js b/src/pyodide.js index c2f1050a9..42988deed 100644 --- a/src/pyodide.js +++ b/src/pyodide.js @@ -67,7 +67,7 @@ var languagePluginLoader = new Promise((resolve, reject) => { } }); } else { - log.console(`Unknown package '${package}'`); + console.log(`Unknown package '${package}'`); } } }