From 45cc59d804e96bb41ff2f5c3dcc5666fd2d2e48d Mon Sep 17 00:00:00 2001 From: David LaPalomento Date: Wed, 29 Apr 2015 12:10:36 -0700 Subject: [PATCH 1/3] Throw an error if techOrder isn't in options. For #1998 Bail with a friendly error if techOrder isn't defined during player construction. Add in a qunit test page at the root of the tests directory for simpler browser debugging. --- Gruntfile.js | 37 ++++++++++++++++++++++++------------- src/js/player.js | 11 +++++++++++ test/globals-shim.js | 10 ++++++++++ test/index.html | 16 ++++++++++++++++ test/karma-qunit-shim.js | 6 ------ test/karma.conf.js | 9 ++------- test/unit/player.js | 11 +++++++++++ 7 files changed, 74 insertions(+), 26 deletions(-) create mode 100644 test/globals-shim.js create mode 100644 test/index.html delete mode 100644 test/karma-qunit-shim.js diff --git a/Gruntfile.js b/Gruntfile.js index 0225662566..c26bc5c1ca 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -263,6 +263,21 @@ module.exports = function(grunt) { } }, browserify: { + options: { + transform: [ + require('babelify').configure({ + sourceMapRelative: './src/js' + }), + ['browserify-versionify', { + placeholder: '__VERSION__', + version: pkg.version + }], + ['browserify-versionify', { + placeholder: '__VERSION_NO_PATCH__', + version: version.majorMinor + }] + ] + }, build: { files: { 'build/temp/video.js': ['src/js/video.js'] @@ -275,19 +290,15 @@ module.exports = function(grunt) { banner: license, plugin: [ [ 'browserify-derequire' ] - ], - transform: [ - require('babelify').configure({ - sourceMapRelative: './src/js' - }), - ['browserify-versionify', { - placeholder: '__VERSION__', - version: pkg.version - }], - ['browserify-versionify', { - placeholder: '__VERSION_NO_PATCH__', - version: version.majorMinor - }] + ] + } + }, + test: { + files: { + 'build/temp/tests.js': [ + 'test/globals-shim.js', + 'test/unit/**/*.js', + 'test/api/**.js' ] } } diff --git a/src/js/player.js b/src/js/player.js index cab97a5417..52012cab5a 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -76,6 +76,17 @@ class Player extends Component { // Run base component initializing with new options super(null, options, ready); + + // if the global option object was accidentally blown away by + // someone, bail early with an informative error + if (!this.options_ || + !this.options_.techOrder || + !this.options_.techOrder.length) { + throw new Error('No techOrder specified. Did you overwrite ' + + 'videojs.options instead of just changing the ' + + 'properties you want to override?'); + } + this.tag = tag; // Store the original tag used to set options // Store the tag attributes used to restore html5 element diff --git a/test/globals-shim.js b/test/globals-shim.js new file mode 100644 index 0000000000..cbcf7b034c --- /dev/null +++ b/test/globals-shim.js @@ -0,0 +1,10 @@ +import window from 'global/window'; +import sinon from 'sinon'; + +window.q = QUnit; +window.sinon = sinon; + +// This may not be needed anymore, but double check before removing +window.fixture = document.createElement('div'); +fixture.id = 'qunit-fixture'; +document.body.appendChild(fixture); diff --git a/test/index.html b/test/index.html new file mode 100644 index 0000000000..1efc6d36e2 --- /dev/null +++ b/test/index.html @@ -0,0 +1,16 @@ + + + + + VideoJS Tests + + + +
+
+ + + + + diff --git a/test/karma-qunit-shim.js b/test/karma-qunit-shim.js deleted file mode 100644 index 18611a8d2f..0000000000 --- a/test/karma-qunit-shim.js +++ /dev/null @@ -1,6 +0,0 @@ -var q = QUnit; - -// This may not be needed anymore, but double check before removing -var fixture = document.createElement('div'); -fixture.id = 'qunit-fixture'; -document.body.appendChild(fixture); \ No newline at end of file diff --git a/test/karma.conf.js b/test/karma.conf.js index 40b9773a63..9c5876f03c 100644 --- a/test/karma.conf.js +++ b/test/karma.conf.js @@ -50,20 +50,16 @@ module.exports = function(config) { config.set({ basePath: '', - frameworks: ['browserify', 'qunit', 'sinon'], + frameworks: ['browserify', 'qunit'], autoWatch: false, singleRun: true, - // customLaunchers: customLaunchers, - files: [ '../build/temp/video-js.min.css', - '../test/karma-qunit-shim.js', - '../test/unit/**/*.js', '../build/temp/video.min.js', - '../test/api/**/*.js', + '../build/temp/tests.js', ], preprocessors: { @@ -84,7 +80,6 @@ module.exports = function(config) { 'karma-phantomjs-launcher', 'karma-safari-launcher', 'karma-sauce-launcher', - 'karma-sinon', 'karma-browserify', 'karma-coverage' ], diff --git a/test/unit/player.js b/test/unit/player.js index 7f7376e5eb..aa63119f09 100644 --- a/test/unit/player.js +++ b/test/unit/player.js @@ -717,3 +717,14 @@ test('should be scrubbing while seeking', function(){ equal(player.scrubbing(), true, 'player is scrubbing'); ok(player.el().className.indexOf('scrubbing') !== -1, 'scrubbing class is present'); }); + +test('should throw on startup no techs are specified', function() { + const techOrder = Options.techOrder; + + Options.techOrder = null; + q.throws(function() { + videojs(TestHelpers.makeTag()); + }, 'a falsey techOrder should throw'); + + Options.techOrder = techOrder; +}); From 8f77be1f6c0917c4ebdfff845d2b33a8eb77c769 Mon Sep 17 00:00:00 2001 From: David LaPalomento Date: Wed, 29 Apr 2015 13:51:26 -0700 Subject: [PATCH 2/3] Don't fail the build for coveralls Coveralls is behaving unreliably so treat a failure as a warning. --- Gruntfile.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Gruntfile.js b/Gruntfile.js index c26bc5c1ca..ebbb1d3d9f 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -312,6 +312,9 @@ module.exports = function(grunt) { } }, coveralls: { + options: { + force: true + }, all: { src: 'test/coverage/lcov.info' } From 67bb555862ac6cae49e398bb772a809c6ac49ec5 Mon Sep 17 00:00:00 2001 From: David LaPalomento Date: Wed, 29 Apr 2015 13:52:42 -0700 Subject: [PATCH 3/3] Remove extra div. Add styles. The test fixture is created in the globals shim now so remove the inline element. Add the default video.js styles to the page until we turn on the script-based injection. --- test/index.html | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/index.html b/test/index.html index 1efc6d36e2..41b1268b67 100644 --- a/test/index.html +++ b/test/index.html @@ -4,13 +4,12 @@ VideoJS Tests +
-
- +