From f2fdbcc511b775dbf6de8a86e6b9cb8a1263aa9c Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 26 Nov 2015 10:00:09 +0000 Subject: [PATCH 1/9] Lookup baseURI getter on Node.prototype, not Node Fix regression in baseURI override, spotted by the Karma tests. The getter should be looked up on Node.prototype, not Node. --- pywb/static/wombat.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pywb/static/wombat.js b/pywb/static/wombat.js index e86ae3d3..62f9bbb6 100644 --- a/pywb/static/wombat.js +++ b/pywb/static/wombat.js @@ -762,7 +762,7 @@ var wombat_internal = function($wbwindow) { def_prop($wbwindow.HTMLBaseElement.prototype, "href", undefined, base_href_get); // Shared baseURI - var orig_getter = get_orig_getter($wbwindow.Node, "baseURI"); + var orig_getter = get_orig_getter($wbwindow.Node.prototype, "baseURI"); if (orig_getter) { var get_baseURI = function() { var res = orig_getter.call(this); From 9c88b1fd209b7d7d19c14cff7df6cf1b8aa08c53 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 26 Nov 2015 10:15:35 +0000 Subject: [PATCH 2/9] Enable running Karma tests against Safari Detect in the test whether overriding of DOM properties is supported in the current environment and skip testing for the baseURI override in that case. --- karma-tests/wombat.spec.js | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/karma-tests/wombat.spec.js b/karma-tests/wombat.spec.js index 7012da83..8bebb8de 100644 --- a/karma-tests/wombat.spec.js +++ b/karma-tests/wombat.spec.js @@ -44,8 +44,21 @@ function runWombatTest(testCase, done) { }; // expose chai's assertion testing API to the test script - assert = window.parent.assert; - reportError = window.parent.reportError; + window.assert = window.parent.assert; + window.reportError = window.parent.reportError; + + // helpers which check whether DOM property overrides are supported + // in the current browser + window.domTests = { + areDOMPropertiesConfigurable: function () { + var descriptor = Object.getOwnPropertyDescriptor(Node.prototype, 'baseURI'); + if (descriptor && !descriptor.configurable) { + return false; + } else { + return true; + } + } + }; }); try { @@ -123,7 +136,9 @@ describe('WombatJS', function () { if (typeof baseURI !== 'string') { throw new Error('baseURI is not a string'); } - assert.equal(baseURI, 'http:///dummy.html'); + if (domTests.areDOMPropertiesConfigurable()) { + assert.equal(baseURI, 'http:///dummy.html'); + } }, }, done); }); @@ -141,7 +156,9 @@ describe('WombatJS', function () { html: 'A link', testScript: function () { var link = document.getElementById('link'); - assert.equal(link.href, 'http:///foobar.html'); + if (domTests.areDOMPropertiesConfigurable()) { + assert.equal(link.href, 'http:///foobar.html'); + } }, }, done); }); From 4f86a895c53453e3ea369518d06f73d881c05179 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 26 Nov 2015 10:16:23 +0000 Subject: [PATCH 3/9] Enable testing against Safari on Sauce Labs * Provide a fallback mode in the Karma tests which tests against a local browser (defaults to Firefox) if Sauce Labs credentials are not set. This is useful for local testing for contributors who might not have a Sauce Labs account. * Add Safari under OS X to the set of Sauce Labs browsers that the Karma tests are run against, following the merge of the WombatJS fixes for Safari and Edge. Although Edge now works under manual testing, automated testing against Sauce Labs is not yet working for reasons yet to be determined. --- karma-tests/karma.conf.js | 39 +++++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/karma-tests/karma.conf.js b/karma-tests/karma.conf.js index b40190f2..af573a5a 100644 --- a/karma-tests/karma.conf.js +++ b/karma-tests/karma.conf.js @@ -1,8 +1,3 @@ -if (!process.env['SAUCE_USERNAME'] || !process.env['SAUCE_ACCESS_KEY']) { - console.error('Sauce Labs account details not set, skipping Karma tests'); - process.exit(0); -} - var sauceLabsConfig = { testName: 'PyWB Client Tests', }; @@ -15,7 +10,7 @@ if (process.env.TRAVIS_JOB_NUMBER) { var WOMBAT_JS_PATH = 'pywb/static/wombat.js'; -var customLaunchers = { +var sauceLaunchers = { sl_chrome: { base: 'SauceLabs', browserName: 'chrome', @@ -26,18 +21,20 @@ var customLaunchers = { browserName: 'firefox', }, -/* Safari and Edge are currently broken in - pywb. - - See: https://github.com/ikreymer/pywb/issues/148 (Edge) - https://github.com/ikreymer/pywb/issues/147 (Safari) - sl_safari: { base: 'SauceLabs', browserName: 'safari', platform: 'OS X 10.11', version: '9.0', }, + +/* Edge is currently broken in + pywb. + + See: https://github.com/ikreymer/pywb/issues/148 (Edge) + https://github.com/ikreymer/pywb/issues/147 (Safari) + + sl_edge: { base: 'SauceLabs', browserName: 'MicrosoftEdge', @@ -45,6 +42,24 @@ var customLaunchers = { */ }; +var localLaunchers = { + localFirefox: { + base: 'Firefox', + }, +}; + +var customLaunchers = {}; + +if (process.env['SAUCE_USERNAME'] && process.env['SAUCE_ACCESS_KEY']) { + customLaunchers = sauceLaunchers; +} else { + console.error('Sauce Labs account details not set, ' + + 'Karma tests will be run only against local browsers.' + + 'Set SAUCE_USERNAME and SAUCE_ACCESS_KEY environment variables to ' + + 'run tests against Sauce Labs browsers'); + customLaunchers = localLaunchers; +} + module.exports = function(config) { config.set({ basePath: '../', From a153ada08eae45104f2138374d336b851557fb92 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 26 Nov 2015 10:27:37 +0000 Subject: [PATCH 4/9] Configure Travis to make Firefox available for Karma tests For some Karma tests, we can run them faster by running against a local browser in the Travis CI instance rather than Sauce Labs. This is also useful for: - Contributors wishing to run the Travis tests against their own forks and have not set up Sauce Labs credentials. - Running Karma tests against Pull Request builds where Sauce Connect is not available. --- .travis.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.travis.yml b/.travis.yml index 559895df..7ea4d126 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,6 +22,10 @@ install: - pip install coverage pytest-cov coveralls --use-mirrors - npm install +before_script: + - export DISPLAY=:99.0 + - sh -e /etc/init.d/xvfb start + script: - python setup.py test - cd karma-tests && make test From 70a098cbd45c29fe402ddb5e417f3c22a0cf187e Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 26 Nov 2015 11:07:07 +0000 Subject: [PATCH 5/9] Enable Sauce Connect in Travis Rather than specifying the username and encrypted key in the Travis config, we just enable Sauce Connect and require SAUCE_USERNAME and SAUCE_ACCESS_KEY env vars to be set. This is so that the Karma tests have the same env vars available to them which they can use to check whether to run against Sauce Labs or not. --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.travis.yml b/.travis.yml index 7ea4d126..3ddcdff5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,6 +7,9 @@ python: os: - linux +addons: + sauce_connect: true + cache: directories: - $HOME/.cache/pip From 1484b06da61a886a6517c34f0b0b2d610cb557c1 Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 26 Nov 2015 13:54:47 +0000 Subject: [PATCH 6/9] Avoid changing the writability of the 'href' attr of Wombat overrides document.baseURI and .href in order to return the original URL rather than the proxied URL. The .href override however ended up making a writable attribute read-only, which could trigger script errors in strict-mode JS. Fix this by avoiding replacing the setter for a DOM property if no replacement setter is provided. Fixes an error loading Hypothesis under Microsoft Edge. --- karma-tests/wombat.spec.js | 97 +++++++++++++++++++++++--------------- pywb/static/wombat.js | 15 ++++-- 2 files changed, 69 insertions(+), 43 deletions(-) diff --git a/karma-tests/wombat.spec.js b/karma-tests/wombat.spec.js index 8bebb8de..62fe9f72 100644 --- a/karma-tests/wombat.spec.js +++ b/karma-tests/wombat.spec.js @@ -121,45 +121,66 @@ describe('WombatJS', function () { }, done); }); - it('should rewrite document.baseURI', function (done) { - runWombatTest({ - initScript: function () { - wbinfo = { - wombat_opts: {}, - prefix: window.location.origin, - wombat_ts: '', - }; - }, - wombatScript: wombatScript, - testScript: function () { - var baseURI = document.baseURI; - if (typeof baseURI !== 'string') { - throw new Error('baseURI is not a string'); - } - if (domTests.areDOMPropertiesConfigurable()) { - assert.equal(baseURI, 'http:///dummy.html'); - } - }, - }, done); + describe('anchor rewriting', function () { + it('should rewrite links in dynamically injected tags', function (done) { + runWombatTest({ + initScript: function () { + wbinfo = { + wombat_opts: {}, + prefix: window.location.origin, + wombat_ts: '', + }; + }, + wombatScript: wombatScript, + html: 'A link', + testScript: function () { + var link = document.getElementById('link'); + if (domTests.areDOMPropertiesConfigurable()) { + assert.equal(link.href, 'http:///foobar.html'); + } + }, + }, done); + }); }); - it('should rewrite links in dynamically injected tags', function (done) { - runWombatTest({ - initScript: function () { - wbinfo = { - wombat_opts: {}, - prefix: window.location.origin, - wombat_ts: '', - }; - }, - wombatScript: wombatScript, - html: 'A link', - testScript: function () { - var link = document.getElementById('link'); - if (domTests.areDOMPropertiesConfigurable()) { - assert.equal(link.href, 'http:///foobar.html'); - } - }, - }, done); + describe('base URL overrides', function () { + it('document.baseURI should return the original URL', function (done) { + runWombatTest({ + initScript: function () { + wbinfo = { + wombat_opts: {}, + prefix: window.location.origin, + wombat_ts: '', + }; + }, + wombatScript: wombatScript, + testScript: function () { + var baseURI = document.baseURI; + if (typeof baseURI !== 'string') { + throw new Error('baseURI is not a string'); + } + if (domTests.areDOMPropertiesConfigurable()) { + assert.equal(baseURI, 'http:///dummy.html'); + } + }, + }, done); + }); + + it('should allow base.href to be assigned', function (done) { + runWombatTest({ + initScript: function () { + wbinfo = { + wombat_opts: {}, + }; + }, + wombatScript: wombatScript, + testScript: function () { + 'use strict'; + var baseElement = document.createElement('base'); + baseElement.href = 'http://foobar.com/base'; + assert.equal(baseElement.href, 'http://foobar.com/base'); + }, + }, done); + }); }); }); diff --git a/pywb/static/wombat.js b/pywb/static/wombat.js index 62f9bbb6..09ad23c4 100644 --- a/pywb/static/wombat.js +++ b/pywb/static/wombat.js @@ -379,11 +379,16 @@ var wombat_internal = function($wbwindow) { } try { - Object.defineProperty(obj, prop, { - configurable: false, - set: set_func, - get: get_func - }); + var descriptor = { + configurable: true, + get: get_func, + }; + + if (set_func) { + descriptor.set = set_func; + } + + Object.defineProperty(obj, prop, descriptor); return true; } catch (e) { From e4e3de85e219d72e121bd6e6826227fd1982e36b Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 26 Nov 2015 13:57:35 +0000 Subject: [PATCH 7/9] Increase the browser inactivity timeout Increase the inactivity timeout when running CI tests in case as recommended at https://docs.travis-ci.com/user/gui-and-headless-browsers/#Karma-and-Firefox-inactivity-timeouts That guidance applies to Travis but also to Sauce Labs. --- karma-tests/karma.conf.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/karma-tests/karma.conf.js b/karma-tests/karma.conf.js index af573a5a..2d482d5c 100644 --- a/karma-tests/karma.conf.js +++ b/karma-tests/karma.conf.js @@ -91,9 +91,12 @@ module.exports = function(config) { sauceLabs: sauceLabsConfig, // use an extended timeout for capturing Sauce Labs - // browsers in case the service is busy + // browsers and waiting for activity + // in case the service is busy captureTimeout: 3 * 60000, + browserNoActivityTimeout: 30 * 1000, + customLaunchers: customLaunchers, browsers: Object.keys(customLaunchers), From 1997c4a18046304d8b879631841034c63e5a15ad Mon Sep 17 00:00:00 2001 From: Robert Knight Date: Thu, 26 Nov 2015 16:56:23 +0000 Subject: [PATCH 8/9] Run Karma tests against Microsoft Edge * Increase the default timeouts to account for the relative slowness of setting up connections to remote browsers. * Change the URL into which Wombat JS is loaded for tests to be a valid URL. Under Microsoft Edge, the JS code in the page is not run if the URL fetch returns a 404. * Change assert.equal() implementation to avoid confusion due to Karma's reformatting of URLs in exception error messages. --- karma-tests/dummy.html | 9 +++++++++ karma-tests/karma.conf.js | 24 ++++++++++++------------ karma-tests/wombat.spec.js | 23 +++++++++++++++++------ 3 files changed, 38 insertions(+), 18 deletions(-) create mode 100644 karma-tests/dummy.html diff --git a/karma-tests/dummy.html b/karma-tests/dummy.html new file mode 100644 index 00000000..52e695fa --- /dev/null +++ b/karma-tests/dummy.html @@ -0,0 +1,9 @@ + + + + + + diff --git a/karma-tests/karma.conf.js b/karma-tests/karma.conf.js index 2d482d5c..9972fac9 100644 --- a/karma-tests/karma.conf.js +++ b/karma-tests/karma.conf.js @@ -28,18 +28,10 @@ var sauceLaunchers = { version: '9.0', }, -/* Edge is currently broken in - pywb. - - See: https://github.com/ikreymer/pywb/issues/148 (Edge) - https://github.com/ikreymer/pywb/issues/147 (Safari) - - sl_edge: { base: 'SauceLabs', browserName: 'MicrosoftEdge', }, -*/ }; var localLaunchers = { @@ -73,6 +65,11 @@ module.exports = function(config) { included: false, served: true, }, + { + pattern: 'karma-tests/dummy.html', + included: false, + served: true, + }, 'karma-tests/*.spec.js', ], @@ -90,12 +87,15 @@ module.exports = function(config) { sauceLabs: sauceLabsConfig, - // use an extended timeout for capturing Sauce Labs - // browsers and waiting for activity - // in case the service is busy + // Set extended timeouts to account for the slowness + // in connecting to remote browsers (eg. when using + // Sauce Labs) + // + // See https://oligofren.wordpress.com/2014/05/27/running-karma-tests-on-browserstack/ captureTimeout: 3 * 60000, - browserNoActivityTimeout: 30 * 1000, + browserDisconnectTimeout: 10 * 1000, + browserDisconnectTolerance: 1, customLaunchers: customLaunchers, diff --git a/karma-tests/wombat.spec.js b/karma-tests/wombat.spec.js index 62fe9f72..fbb9b33d 100644 --- a/karma-tests/wombat.spec.js +++ b/karma-tests/wombat.spec.js @@ -1,4 +1,3 @@ -var WOMBAT_SRC = '../pywb/static/wombat.js'; var DEFAULT_TIMEOUT = 20000; // creates a new document in an