From c5028a8576080477d0e71f7fb2dd3819fc4c4301 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 20 Jun 2014 11:14:49 +0200 Subject: [PATCH 1/2] js: Fix console setup, allow mocking of the console --- docs/api/js.rst | 4 ++++ js/src/mopidy.js | 26 +++++++++++++++----------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/docs/api/js.rst b/docs/api/js.rst index ff333992..372e7f4e 100644 --- a/docs/api/js.rst +++ b/docs/api/js.rst @@ -138,6 +138,10 @@ When creating an instance, you can specify the following settings: .. versionadded:: 0.19 (Mopidy.js 0.4) +``console`` + If set, this object will be used to log errors from Mopidy.js. This is + mostly useful for testing Mopidy.js. + ``webSocket`` An existing WebSocket object to be used instead of creating a new WebSocket. Defaults to undefined. diff --git a/js/src/mopidy.js b/js/src/mopidy.js index 42762807..b5dd13f5 100644 --- a/js/src/mopidy.js +++ b/js/src/mopidy.js @@ -9,8 +9,8 @@ function Mopidy(settings) { return new Mopidy(settings); } + this._console = this._getConsole(settings || {}); this._settings = this._configure(settings || {}); - this._console = this._getConsole(); this._backoffDelay = this._settings.backoffDelayMin; this._pendingRequests = {}; @@ -40,6 +40,20 @@ Mopidy.ServerError.prototype.constructor = Mopidy.ServerError; Mopidy.WebSocket = websocket.Client; +Mopidy.prototype._getConsole = function (settings) { + if (typeof settings.console !== "undefined") { + return settings.console; + } + + var con = typeof console !== "undefined" && console || {}; + + con.log = con.log || function () {}; + con.warn = con.warn || function () {}; + con.error = con.error || function () {}; + + return con; +}; + Mopidy.prototype._configure = function (settings) { var currentHost = (typeof document !== "undefined" && document.location.host) || "localhost"; @@ -59,16 +73,6 @@ Mopidy.prototype._configure = function (settings) { return settings; }; -Mopidy.prototype._getConsole = function () { - var console = typeof console !== "undefined" && console || {}; - - console.log = console.log || function () {}; - console.warn = console.warn || function () {}; - console.error = console.error || function () {}; - - return console; -}; - Mopidy.prototype._delegateEvents = function () { // Remove existing event handlers this.off("websocket:close"); From d15c7d0797df07e30ea9357c36b510a6bc992ec8 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal Date: Fri, 20 Jun 2014 11:16:46 +0200 Subject: [PATCH 2/2] js: Log warning if calling convention is not explicitly set --- docs/changelog.rst | 4 +- js/README.md | 3 +- js/src/mopidy.js | 6 +++ js/test/mopidy-test.js | 90 +++++++++++++++++++++++++++++++++++------- 4 files changed, 87 insertions(+), 16 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 1708e98c..c49552aa 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -97,7 +97,9 @@ Feature release. - Add support for method calls with by-name arguments. The old calling convention, ``by-position-only``, is still the default, but this will - probably change in the future. See the :ref:`mopidy-js` docs for details. + change in the future. A warning is logged to the console if you don't + explicitly select a calling convention. See the :ref:`mopidy-js` docs for + details. **MPD frontend** diff --git a/js/README.md b/js/README.md index b6be8b76..45506d35 100644 --- a/js/README.md +++ b/js/README.md @@ -84,7 +84,8 @@ Changelog - Add support for method calls with by-name arguments. The old calling convention, "by-position-only", is still the default, but this will change in - the future. See the docs for details. + the future. A warning is printed to the console if you don't explicitly + select a calling convention. See the docs for details. ### 0.3.0 (2014-06-16) diff --git a/js/src/mopidy.js b/js/src/mopidy.js index b5dd13f5..7e019dd4 100644 --- a/js/src/mopidy.js +++ b/js/src/mopidy.js @@ -67,6 +67,12 @@ Mopidy.prototype._configure = function (settings) { settings.backoffDelayMin = settings.backoffDelayMin || 1000; settings.backoffDelayMax = settings.backoffDelayMax || 64000; + if (typeof settings.callingConvention === "undefined") { + this._console.warn( + "Mopidy.js is using the default calling convention. The " + + "default will change in the future. You should explicitly " + + "specify which calling convention you use."); + } settings.callingConvention = ( settings.callingConvention || "by-position-only"); diff --git a/js/test/mopidy-test.js b/js/test/mopidy-test.js index dae0f518..caf4ce21 100644 --- a/js/test/mopidy-test.js +++ b/js/test/mopidy-test.js @@ -33,7 +33,10 @@ buster.testCase("Mopidy", { close: this.stub(), send: this.stub() }; - this.mopidy = new Mopidy({webSocket: this.webSocket}); + this.mopidy = new Mopidy({ + callingConvention: "by-position-or-by-name", + webSocket: this.webSocket + }); }, tearDown: function () { @@ -42,7 +45,10 @@ buster.testCase("Mopidy", { "constructor": { "connects when autoConnect is true": function () { - new Mopidy({autoConnect: true}); + new Mopidy({ + autoConnect: true, + callingConvention: "by-position-or-by-name" + }); var currentHost = typeof document !== "undefined" && document.location.host || "localhost"; @@ -52,21 +58,73 @@ buster.testCase("Mopidy", { }, "does not connect when autoConnect is false": function () { - new Mopidy({autoConnect: false}); + new Mopidy({ + autoConnect: false, + callingConvention: "by-position-or-by-name" + }); refute.called(this.webSocketConstructorStub); }, "does not connect when passed a WebSocket": function () { - new Mopidy({webSocket: {}}); + new Mopidy({ + callingConvention: "by-position-or-by-name", + webSocket: {} + }); refute.called(this.webSocketConstructorStub); }, + "defaults to by-position-only calling convention": function () { + var console = { + warn: function () {} + }; + var mopidy = new Mopidy({ + console: console, + webSocket: this.webSocket, + }); + + assert.equals( + mopidy._settings.callingConvention, + "by-position-only"); + }, + + "warns if no calling convention explicitly selected": function () { + var console = { + warn: function () {} + }; + var stub = this.stub(console, "warn"); + + new Mopidy({console: console}); + + assert.calledOnceWith( + stub, + "Mopidy.js is using the default calling convention. The " + + "default will change in the future. You should explicitly " + + "specify which calling convention you use."); + }, + + "does not warn if calling convention chosen explicitly": function () { + var console = { + warn: function () {} + }; + var stub = this.stub(console, "warn"); + + new Mopidy({ + callingConvention: "by-position-or-by-name", + console: console + }); + + refute.called(stub); + }, + "works without 'new' keyword": function () { var mopidyConstructor = Mopidy; // To trick jshint into submission - var mopidy = mopidyConstructor({webSocket: {}}); + var mopidy = mopidyConstructor({ + callingConvention: "by-position-or-by-name", + webSocket: {} + }); assert.isObject(mopidy); assert(mopidy instanceof Mopidy); @@ -75,7 +133,10 @@ buster.testCase("Mopidy", { ".connect": { "connects when autoConnect is false": function () { - var mopidy = new Mopidy({autoConnect: false}); + var mopidy = new Mopidy({ + autoConnect: false, + callingConvention: "by-position-or-by-name" + }); refute.called(this.webSocketConstructorStub); mopidy.connect(); @@ -89,7 +150,10 @@ buster.testCase("Mopidy", { "does nothing when the WebSocket is open": function () { this.webSocket.readyState = Mopidy.WebSocket.OPEN; - var mopidy = new Mopidy({webSocket: this.webSocket}); + var mopidy = new Mopidy({ + callingConvention: "by-position-or-by-name", + webSocket: this.webSocket + }); mopidy.connect(); @@ -766,8 +830,12 @@ buster.testCase("Mopidy", { assert.calledOnceWith(spy); }, - "by-position calling convention": { + "by-position-only calling convention": { setUp: function () { + this.mopidy = new Mopidy({ + webSocket: this.webSocket, + callingConvention: "by-position-only" + }); this.mopidy._createApi({ foo: { params: ["bar", "baz"] @@ -777,12 +845,6 @@ buster.testCase("Mopidy", { }, - "is the default": function () { - assert.equals( - this.mopidy._settings.callingConvention, - "by-position-only"); - }, - "sends no params if no arguments passed to function": function () { this.mopidy.foo();