From f8db8be71f5b7d6eea8493b2c794017a036a26a9 Mon Sep 17 00:00:00 2001 From: jcass Date: Thu, 10 Mar 2016 17:21:13 +0200 Subject: [PATCH] Add Javascript test and test coverage frameworks. --- .gitignore | 1 + .travis.yml | 5 +- karma.conf.js | 81 +++++++ mopidy_musicbox_webclient/static/js/gui.js | 2 +- mopidy_musicbox_webclient/static/js/images.js | 154 ++++++++------ .../static/js/process_ws.js | 6 +- .../static/js/progress_timer.js | 7 +- package.json | 36 +++- tests/test_images.js | 201 ++++++++++++++++++ tox.ini | 14 +- 10 files changed, 421 insertions(+), 86 deletions(-) create mode 100644 karma.conf.js create mode 100644 tests/test_images.js diff --git a/.gitignore b/.gitignore index b754965..e2cf956 100644 --- a/.gitignore +++ b/.gitignore @@ -8,6 +8,7 @@ MANIFEST build/ cover/ +coverage/ coverage.xml dist/ docs/_build/ diff --git a/.travis.yml b/.travis.yml index 5ce0d3d..c1749cf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,6 +8,7 @@ python: env: - TOX_ENV=py27 - TOX_ENV=flake8 + - TOX_ENV=test - TOX_ENV=eslint - TOX_ENV=csslint - TOX_ENV=tidy @@ -19,4 +20,6 @@ script: - "tox -e $TOX_ENV" after_success: - - "if [ $TOX_ENV == 'py27' ]; then pip install coveralls; coveralls; fi" + # TODO: find a way to combine .py and .js coverage reports. + # - "if [ $TOX_ENV == 'py27' ]; then pip install coveralls; coveralls; fi" + - "if [ $TOX_ENV == 'test' ]; then cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js; fi" diff --git a/karma.conf.js b/karma.conf.js new file mode 100644 index 0000000..6104b24 --- /dev/null +++ b/karma.conf.js @@ -0,0 +1,81 @@ +// Karma configuration + +module.exports = function (config) { + config.set({ + + // base path that will be used to resolve all patterns (eg. files, exclude) + basePath: '', + + // frameworks to use + // available frameworks: https://npmjs.org/browse/keyword/karma-adapter + frameworks: ['browserify', 'mocha'], + + // list of files / patterns to load in the browser + files: [ + 'mopidy_musicbox_webclient/static/vendors/**/*.js', + // TODO: can remove the next line once JavaScript codebase has been modularized. + 'mopidy_musicbox_webclient/static/js/**/*.js', + 'tests/**/test_*.js' + ], + + // list of files to exclude + exclude: [ + ], + + // preprocess matching files before serving them to the browser + // available preprocessors: https://npmjs.org/browse/keyword/karma-preprocessor + preprocessors: { + 'tests/**/test_*.js': [ 'browserify' ], + 'mopidy_musicbox_webclient/static/js/**/*.js': ['coverage'] + }, + + // test results reporter to use + // possible values: 'dots', 'progress' + // available reporters: https://npmjs.org/browse/keyword/karma-reporter + reporters: ['progress', 'coverage'], + + // web server port + port: 9876, + + // enable / disable colors in the output (reporters and logs) + colors: true, + + // level of logging + // possible values: config.LOG_DISABLE || config.LOG_ERROR || config.LOG_WARN || config.LOG_INFO || config.LOG_DEBUG + logLevel: config.LOG_INFO, + + // enable / disable watching file and executing tests whenever any file changes + autoWatch: false, + + // start these browsers + // available browser launchers: https://npmjs.org/browse/keyword/karma-launcher + browsers: ['PhantomJS'], + + // Continuous Integration mode + // if true, Karma captures browsers, runs the tests and exits + singleRun: true, + + // Concurrency level + // how many browser should be started simultaneous + concurrency: Infinity, + + // add additional browserify configuration properties here + // such as transform and/or debug=true to generate source maps + browserify: { + debug: true, + transform: [ + 'babelify', + ['browserify-istanbul', { instrumenter: require('isparta') }] + ] + }, + + coverageReporter: { + // specify a common output directory + dir: 'coverage/', + reporters: [ + { type: 'lcov', subdir: '.' }, + { type: 'text'} + ] + } + }) +} diff --git a/mopidy_musicbox_webclient/static/js/gui.js b/mopidy_musicbox_webclient/static/js/gui.js index 104732e..7eff64c 100644 --- a/mopidy_musicbox_webclient/static/js/gui.js +++ b/mopidy_musicbox_webclient/static/js/gui.js @@ -128,7 +128,7 @@ function setSongInfo (data) { } if (data.track.album && data.track.album.name) { $('#modalalbum').html('' + data.track.album.name + '') - getCover(data.track.uri, '#infocover, #controlspopupimage', 'extralarge') + coverArt.getCover(data.track.uri, '#infocover, #controlspopupimage', 'extralarge') } else { $('#modalalbum').html('') $('#infocover').attr('src', 'images/default_cover.png') diff --git a/mopidy_musicbox_webclient/static/js/images.js b/mopidy_musicbox_webclient/static/js/images.js index 5c16f18..0a1b531 100644 --- a/mopidy_musicbox_webclient/static/js/images.js +++ b/mopidy_musicbox_webclient/static/js/images.js @@ -5,88 +5,106 @@ API_KEY = 'b6d34c3af91d62ab0ae00ab1b6fa8733' API_SECRET = '2c631802c2285d5d5d1502462fe42a2b' -var fmcache -var lastfm - -$(window).load(function () { - // create a Cache object - fmcache = new LastFMCache() - // create a LastFM object - lastfm = new LastFM({ +var coverArt = { + fmcache: new LastFMCache(), + lastfm: new LastFM({ apiKey: API_KEY, apiSecret: API_SECRET, - cache: fmcache - }) -}) + cache: this.fmcache + }), -function getCover (uri, images, size) { - var defUrl = 'images/default_cover.png' - $(images).attr('src', defUrl) - if (!uri) { - return - } - - mopidy.library.getImages({'uris': [uri]}).then(function (imageResults) { - var uri = Object.keys(imageResults)[0] - if (imageResults[uri].length > 0) { - $(images).attr('src', imageResults[uri][0].uri) - } else { - // Also check deprecated 'album.images' in case backend does not - // implement mopidy.library.getImages yet... - getCoverFromAlbum(uri, images, size) + getCover: function (uri, images, size) { + var defUrl = 'images/default_cover.png' + $(images).attr('src', defUrl) + if (!uri) { + return } - }) -} + mopidy.library.getImages({'uris': [uri]}).then(function (imageResults) { + var uri = Object.keys(imageResults)[0] + if (imageResults[uri].length > 0) { + $(images).attr('src', imageResults[uri][0].uri) + } else { + // Also check deprecated 'album.images' in case backend does not + // implement mopidy.library.getImages yet... + coverArt.getCoverFromAlbum(uri, images, size) + } + }) + }, -// Note that this approach has been deprecated in Mopidy -// TODO: Remove when Mopidy no longer supports getting images -// with 'album.images'. -function getCoverFromAlbum (uri, images, size) { - mopidy.library.lookup({'uris': [uri]}).then(function (resultDict) { - var uri = Object.keys(resultDict)[0] - var track = resultDict[uri][0] - if (track.album && track.album.images && (track.album.images.length > 0)) { - $(images).attr('src', track.album.images[0]) - } else { - // Fallback to last.fm - getCoverFromLastFm(track, images, size) + // Note that this approach has been deprecated in Mopidy + // TODO: Remove when Mopidy no longer supports getting images + // with 'album.images'. + getCoverFromAlbum: function (uri, images, size) { + var defUrl = 'images/default_cover.png' + $(images).attr('src', defUrl) + if (!uri) { + return } - }) -} + mopidy.library.lookup({'uris': [uri]}).then(function (resultDict) { + var uri = Object.keys(resultDict)[0] + var track = resultDict[uri][0] + if (track && track.album && track.album.images && track.album.images.length > 0) { + $(images).attr('src', track.album.images[0]) + } else if (track && (track.album || track.artist)) { + // Fallback to last.fm + coverArt.getCoverFromLastFm(track, images, size) + } else { + return + } + }) + }, -function getCoverFromLastFm (track, images, size) { - var defUrl = 'images/default_cover.png' - if (!(track.album || track.artist)) { - return - } - var albumname = track.album.name || '' - var artistname = '' - if (track.album.artists && (track.album.artists.length > 0)) { - // First look for the artist in the album - artistname = track.album.artists[0].name - } else if (track.artists && (track.artists.length > 0)) { - // Fallback to using artists for specific track - artistname = track.artists[0].name - } + getCoverFromLastFm: function (track, images, size) { + var defUrl = 'images/default_cover.png' + $(images).attr('src', defUrl) + if (!track || !(track.album || track.artists)) { + return + } + var albumname = (track.album && track.album.name) ? track.album.name : '' + var artistname = '' + if (track.album && track.album.artists && track.album.artists.length > 0) { + // First look for the artist in the album + artistname = track.album.artists[0].name + } else if (track.artists && (track.artists.length > 0)) { + // Fallback to using artists for specific track + artistname = track.artists[0].name + } - lastfm.album.getInfo({artist: artistname, album: albumname}, - { success: function (data) { + this.lastfm.album.getInfo({artist: artistname, album: albumname}, {success: function (data) { for (var i = 0; i < data.album.image.length; i++) { if (data.album.image[i].size === size) { $(images).attr('src', data.album.image[i]['#text'] || defUrl) } } + }, error: function (code, message) { + console.log('Error retrieving album info from last.fm', code, message) + }}) + }, + + getArtistImage: function (artist, images, size) { + var defUrl = 'images/user_24x32.png' + $(images).attr('src', defUrl) + if (!artist || artist.length === 0) { + return } - }, $(images).attr('src', defUrl)) + this.lastfm.artist.getInfo({artist: artist}, {success: function (data) { + for (var i = 0; i < data.artist.image.length; i++) { + if (data.artist.image[i].size === size) { + $(images).attr('src', data.artist.image[i]['#text'] || defUrl) + } + } + }, error: function (code, message) { + console.log('Error retrieving artist info from last.fm', code, message) + }}) + } } -function getArtistImage (nwartist, image, size) { - var defUrl = 'images/user_24x32.png' - lastfm.artist.getInfo({artist: nwartist}, {success: function (data) { - for (var i = 0; i < data.artist.image.length; i++) { - if (data.artist.image[i].size === size) { - $(image).attr('src', data.artist.image[i]['#text'] || defUrl) - } - } - }}, $(images).attr('src', defUrl)) +$(document).ready(coverArt.init) + +// TODO: Remove this once JavaScript codebase has been completely modularized +// in favour of bundling everything using 'browserify'. +if (typeof exports !== 'undefined') { + if (typeof module !== 'undefined' && module.exports) { + module.exports = coverArt + } } diff --git a/mopidy_musicbox_webclient/static/js/process_ws.js b/mopidy_musicbox_webclient/static/js/process_ws.js index e4228af..4c0d287 100644 --- a/mopidy_musicbox_webclient/static/js/process_ws.js +++ b/mopidy_musicbox_webclient/static/js/process_ws.js @@ -220,7 +220,7 @@ function processCurrentPlaylist (resultArr) { function processArtistResults (resultArr) { if (!resultArr || (resultArr.length === 0)) { $('#h_artistname').text('Artist not found...') - getCover('', '#artistviewimage, #artistpopupimage', 'extralarge') + coverArt.getCover('', '#artistviewimage, #artistpopupimage', 'extralarge') showLoading(false) return } @@ -239,7 +239,7 @@ function processArtistResults (resultArr) { function processAlbumResults (resultArr) { if (!resultArr || (resultArr.length === 0)) { $('#h_albumname').text('Album not found...') - getCover('', '#albumviewcover, #coverpopupimage', 'extralarge') + coverArt.getCover('', '#albumviewcover, #coverpopupimage', 'extralarge') showLoading(false) return } @@ -252,6 +252,6 @@ function processAlbumResults (resultArr) { $('#h_albumartist').html(artistname) $('#coverpopupalbumname').html(albumname) $('#coverpopupartist').html(artistname) - getCover(resultArr[0].uri, '#albumviewcover, #coverpopupimage', 'extralarge') + coverArt.getCover(resultArr[0].uri, '#albumviewcover, #coverpopupimage', 'extralarge') showLoading(false) } diff --git a/mopidy_musicbox_webclient/static/js/progress_timer.js b/mopidy_musicbox_webclient/static/js/progress_timer.js index 0b371c7..00ff6e9 100644 --- a/mopidy_musicbox_webclient/static/js/progress_timer.js +++ b/mopidy_musicbox_webclient/static/js/progress_timer.js @@ -1,5 +1,4 @@ var progressTimer -var progressElement = document.getElementById('trackslider') var positionNode = document.createTextNode('') var durationNode = document.createTextNode('') @@ -13,8 +12,10 @@ var syncsLeft = MAX_SYNCS var synced = false var consecutiveSyncs = 0 -document.getElementById('songelapsed').appendChild(positionNode) -document.getElementById('songlength').appendChild(durationNode) +$(document).ready(function () { + $('#songelapsed').append(positionNode) + $('#songlength').append(durationNode) +}) function timerCallback (position, duration, isRunning) { updateTimers(position, duration, isRunning) diff --git a/package.json b/package.json index ee6c8e3..97fb1ac 100644 --- a/package.json +++ b/package.json @@ -7,8 +7,8 @@ "test": "tests" }, "scripts": { - "test": "echo \"Error: no test specified\" && exit 1", - "eslint": "eslint mopidy_musicbox_webclient/static/js/**.js", + "test": "karma start karma.conf.js", + "eslint": "eslint mopidy_musicbox_webclient/static/js/**/**.js tests/**/test_*.js", "csslint": "csslint mopidy_musicbox_webclient/static/css/**.css", "tidy": "node tidy.js" }, @@ -22,12 +22,32 @@ "url": "https://github.com/pimusicbox/mopidy-musicbox-webclient/issues" }, "devDependencies": { - "eslint": "latest", - "eslint-config-standard": "latest", - "eslint-plugin-standard": "latest", - "eslint-plugin-promise": "latest", - "csslint": "latest", - "tidy-html5": "latest" + "babelify": "^7.2.0", + "browserify": "^13.0.0", + "browserify-istanbul": "^2.0.0", + "chai": "^3.5.0", + "chai-as-promised": "^5.2.0", + "chai-jquery": "^2.0.0", + "chai-string": "^1.2.0", + "coveralls": "^2.11.8", + "csslint": "^0.10.0", + "eslint": "^2.3.0", + "eslint-config-standard": "^5.1.0", + "eslint-plugin-promise": "^1.1.0", + "eslint-plugin-standard": "^1.3.2", + "install": "^0.5.6", + "isparta": "^4.0.0", + "karma": "^0.13.22", + "karma-browserify": "^5.0.2", + "karma-cli": "^0.1.2", + "karma-coverage": "^0.5.5", + "karma-mocha": "^0.2.2", + "karma-phantomjs-launcher": "^1.0.0", + "mocha": "^2.4.5", + "phantomjs-prebuilt": "^2.1.5", + "sinon": "^1.17.3", + "tidy-html5": "latest", + "watchify": "^3.7.0" }, "homepage": "https://github.com/pimusicbox/mopidy-musicbox-webclient#readme" } diff --git a/tests/test_images.js b/tests/test_images.js new file mode 100644 index 0000000..8d52232 --- /dev/null +++ b/tests/test_images.js @@ -0,0 +1,201 @@ +var chai = require('chai') +var should = chai.should() +var expect = chai.expect +var assert = chai.assert +chai.use(require('chai-string')) +chai.use(require('chai-jquery')) + +var sinon = require('sinon') + +var coverArt = require('../mopidy_musicbox_webclient/static/js/images.js') + +var images + +before(function () { + html = + '' + + '' + $(document).ready(function () { + $(document.body).add(html) + }) + mopidy = sinon.stub(new Mopidy({callingConvention: 'by-position-or-by-name'})) + images = $('') +}) + +describe('CoverArt', function () { + describe('#getCover()', function () { + beforeEach(function () { + $(images).removeAttr('src') + }) + + it('should use default image if no track URI is provided', function () { + coverArt.getCover('', images, '') + $(images).prop('src').should.endWith('images/default_cover.png') + }) + + it('should get image from Mopidy, if available', function () { + var getImagesResultMock = {'mock:track:uri': [{uri: 'mockImageUri'}]} + var library = { getImages: function () { return $.when(getImagesResultMock) } } + mopidy.library = library + + var getImagesSpy = sinon.spy(mopidy.library, 'getImages') + coverArt.getCover('mock:track:uri', images, '') + + assert(getImagesSpy.calledOnce) + $(images).prop('src').should.endWith('mockImageUri') + }) + + it('should fall back to retrieving image from deprecated track.album.images', function () { + var getImagesResultMock = {'mock:track:uri': []} + var lookupResultMock = {'mock:track:uri': [{album: {images: ['mockAlbumImageUri']}}]} + var library = { + getImages: function () { return $.when(getImagesResultMock) }, + lookup: function () { return $.when(lookupResultMock) } + } + mopidy.library = library + + var getImagesSpy = sinon.spy(mopidy.library, 'getImages') + var getCoverFromAlbumSpy = sinon.spy(coverArt, 'getCoverFromAlbum') + + coverArt.getCover('mock:track:uri', images, '') + + assert(getImagesSpy.calledOnce) + assert(getCoverFromAlbumSpy.calledOnce) + }) + }) + + describe('#getCoverFromAlbum()', function () { + beforeEach(function () { + $(images).removeAttr('src') + }) + + it('should use default image if no track URI is provided', function () { + coverArt.getCoverFromAlbum('', images, '') + $(images).prop('src').should.endWith('images/default_cover.png') + }) + + it('should get image from Mopidy track.album.images, if available', function () { + var lookupResultMock = {'mock:track:uri': [{album: {images: ['mockAlbumImageUri']}}]} + var library = { + lookup: function () { return $.when(lookupResultMock) } + } + mopidy.library = library + + var lookupSpy = sinon.spy(mopidy.library, 'lookup') + coverArt.getCoverFromAlbum('mock:track:uri', images, '') + + assert(lookupSpy.calledOnce) + $(images).prop('src').should.endWith('mockAlbumImageUri') + }) + + it('should use default image if track.album or track.artist is not available', function () { + var lookupResultMock = {'mock:track:uri': []} + var library = { + lookup: function () { return $.when(lookupResultMock) } + } + mopidy.library = library + + var lookupSpy = sinon.spy(mopidy.library, 'lookup') + coverArt.getCoverFromAlbum('mock:track:uri', images, '') + + assert(lookupSpy.calledOnce) + $(images).prop('src').should.endWith('images/default_cover.png') + }) + + it('should fall back to retrieving image from last.fm if none provided by Mopidy', function () { + var lookupResultMock = {'mock:track:uri': [{album: {images: []}}]} + var library = { + lookup: function () { return $.when(lookupResultMock) } + } + mopidy.library = library + + var getCoverFromLastFmSpy = sinon.spy(coverArt, 'getCoverFromLastFm') + coverArt.getCoverFromAlbum('mock:track:uri', images, '') + + assert(getCoverFromLastFmSpy.calledOnce) + }) + }) + + describe('#getCoverFromLastFm()', function () { + beforeEach(function () { + $(images).removeAttr('src') + }) + + it('should use default image if no track is provided', function () { + coverArt.getCoverFromLastFm(undefined, images, '') + $(images).prop('src').should.endWith('images/default_cover.png') + }) + + it('should fall back to using track artist if album artist is not available', function () { + var track = {artists: [{name: 'artistMock'}]} + var getInfoResultMock = {album: {image: []}} + + var getInfoStub = sinon.stub(coverArt.lastfm.album, 'getInfo') + getInfoStub.returns($.when(getInfoResultMock)) + + coverArt.getCoverFromLastFm(track, images, '') + var args = getInfoStub.args + assert(args[0][0].artist === 'artistMock') + getInfoStub.restore() + }) + + it('should get album info from last.fm', function () { + var track = {album: {artists: [{name: 'albumMock'}]}} + var getInfoResultMock = {album: {image: [{'#text': 'mockAlbumImageUri', size: 'small'}]}} + + var getInfoStub = sinon.stub(coverArt.lastfm.album, 'getInfo') + getInfoStub.yieldsTo('success', getInfoResultMock) + + coverArt.getCoverFromLastFm(track, images, 'small') + $(images).prop('src').should.endWith('mockAlbumImageUri') + getInfoStub.restore() + }) + + it('should log errors', function () { + var track = {album: {artists: [{name: 'albumMock'}]}} + var getInfoStub = sinon.stub(coverArt.lastfm.album, 'getInfo') + getInfoStub.yieldsTo('error', 'code', 'message') + + var consoleSpy = sinon.spy(console, 'log') + coverArt.getCoverFromLastFm(track, images, '') + + assert(consoleSpy.calledOnce) + getInfoStub.restore() + consoleSpy.restore() + }) + }) + + describe('#getArtistImage()', function () { + beforeEach(function () { + $(images).removeAttr('src') + }) + + it('should use default image if no artist is provided', function () { + coverArt.getArtistImage('', images, '') + $(images).prop('src').should.endWith('images/user_24x32.png') + }) + + it('should get artist info from last.fm', function () { + var getInfoResultMock = {artist: {image: [{'#text': 'mockArtistImageUri', size: 'small'}]}} + + var getInfoStub = sinon.stub(coverArt.lastfm.artist, 'getInfo') + getInfoStub.yieldsTo('success', getInfoResultMock) + + coverArt.getArtistImage('mockArtist', images, 'small') + $(images).prop('src').should.endWith('mockArtistImageUri') + getInfoStub.restore() + }) + + it('should log errors', function () { + var getInfoStub = sinon.stub(coverArt.lastfm.artist, 'getInfo') + getInfoStub.yieldsTo('error', 'code', 'message') + + var consoleSpy = sinon.spy(console, 'log') + + coverArt.getArtistImage('mockArtist', images, 'small') + assert(consoleSpy.calledOnce) + getInfoStub.restore() + consoleSpy.restore() + }) + }) +}) diff --git a/tox.ini b/tox.ini index 96a5099..b6a3ba3 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py27, flake8, eslint, csslint, tidy +envlist = py27, flake8, test, eslint, csslint, tidy [testenv] deps = @@ -20,7 +20,17 @@ deps = flake8 flake8-import-order skip_install = true -commands = flake8 {posargs:mopidy_musicbox_webclient} +commands = flake8 {posargs:mopidy_musicbox_webclient tests} + +[testenv:test] +whitelist_externals = + /bin/bash +deps = + nodeenv +skip_install = true +commands = + - nodeenv --prebuilt {toxworkdir}/node_env + bash -c '. {toxworkdir}/node_env/bin/activate; npm install; npm test' [testenv:eslint] whitelist_externals =