From 9aed55bb91d84534f6191395b8af01cf04198d5d Mon Sep 17 00:00:00 2001 From: Steve Gravrock Date: Sat, 18 Jan 2020 10:39:51 -0800 Subject: [PATCH] Improved readability of matcher-related deprecations * Include stack traces. This makes it easier to find the matcher that needs to be updated, particularly when it comes from a library rather than the user's own code. * Show each deprecation only once unless `config.verboseDeprecations` is set. Since matchers are often added in a global `beforeEach`, logging deprecations every time can be overwhelming. --- lib/jasmine-core/jasmine.js | 66 +++++++++++++++---- spec/core/EnvSpec.js | 54 +++++++++++++++ .../integration/CustomAsyncMatchersSpec.js | 23 +++++-- spec/core/integration/CustomMatchersSpec.js | 23 +++++-- spec/core/matchers/matchersUtilSpec.js | 21 ++++-- spec/helpers/resetEnv.js | 4 ++ spec/support/jasmine-browser.js | 3 +- spec/support/jasmine.json | 3 +- src/core/Env.js | 46 ++++++++++++- src/core/matchers/matchersUtil.js | 16 ++--- src/core/requireCore.js | 4 +- 11 files changed, 216 insertions(+), 47 deletions(-) create mode 100644 spec/helpers/resetEnv.js diff --git a/lib/jasmine-core/jasmine.js b/lib/jasmine-core/jasmine.js index b5ca6195..9465e287 100644 --- a/lib/jasmine-core/jasmine.js +++ b/lib/jasmine-core/jasmine.js @@ -80,7 +80,7 @@ var getJasmineRequireObj = (function(jasmineGlobal) { j$.basicPrettyPrinter_ = j$.makePrettyPrinter(); Object.defineProperty(j$, 'pp', { get: function() { - j$.getEnv().deprecated( + j$.getEnv().deprecatedOnceWithStack( 'jasmine.pp is deprecated and will be removed in a future release. ' + 'Use the pp method of the matchersUtil passed to the matcher factory ' + "or the asymmetric equality tester's `asymmetricMatch` method " + @@ -97,7 +97,7 @@ var getJasmineRequireObj = (function(jasmineGlobal) { }); Object.defineProperty(j$, 'matchersUtil', { get: function() { - j$.getEnv().deprecated( + j$.getEnv().deprecatedOnceWithStack( 'jasmine.matchersUtil is deprecated and will be removed ' + 'in a future release. Use the instance passed to the matcher factory or ' + "the asymmetric equality tester's `asymmetricMatch` method instead. " + @@ -981,6 +981,7 @@ getJasmineRequireObj().Env = function(j$) { var currentlyExecutingSuites = []; var currentDeclarationSuite = null; var hasFailures = false; + var deprecationsToSuppress = []; /** * This represents the available options to configure Jasmine. @@ -1060,7 +1061,19 @@ getJasmineRequireObj().Env = function(j$) { * @type function * @default undefined */ - Promise: undefined + Promise: undefined, + /** + * Whether or not to issue warnings for certain deprecated functionality + * every time it's used. If not set or set to false, deprecation warnings + * for methods that tend to be called frequently will be issued only once + * or otherwise throttled to to prevent the suite output from being flooded + * with warnings. + * @name Configuration#verboseDeprecations + * @since 3.6.0 + * @type Boolean + * @default false + */ + verboseDeprecations: false }; var currentSuite = function() { @@ -1154,6 +1167,10 @@ getJasmineRequireObj().Env = function(j$) { ); } } + + if (configuration.hasOwnProperty('verboseDeprecations')) { + config.verboseDeprecations = configuration.verboseDeprecations; + } }; /** @@ -1228,7 +1245,7 @@ getJasmineRequireObj().Env = function(j$) { for (var matcherName in matchersToAdd) { if (matchersToAdd[matcherName].length > 1) { - self.deprecated( + self.deprecatedOnceWithStack( 'The matcher factory for "' + matcherName + '" ' + @@ -1253,7 +1270,7 @@ getJasmineRequireObj().Env = function(j$) { for (var matcherName in matchersToAdd) { if (matchersToAdd[matcherName].length > 1) { - self.deprecated( + self.deprecatedOnceWithStack( 'The matcher factory for "' + matcherName + '" ' + @@ -1561,6 +1578,29 @@ getJasmineRequireObj().Env = function(j$) { } }; + this.deprecatedOnceWithStack = function(deprecation) { + var formatter = new j$.ExceptionFormatter(), + stackTrace = formatter + .stack(j$.util.errorWithStack()) + .replace(/^Error\n/m, ''); + + if (config.verboseDeprecations) { + this.deprecated(deprecation + '\n' + stackTrace); + } else { + if (deprecationsToSuppress.indexOf(deprecation) === -1) { + this.deprecated( + deprecation + + '\n' + + 'Note: This message will be shown only once. ' + + 'Set config.verboseDeprecations to true to see every occurrence.\n' + + stackTrace + ); + } + + deprecationsToSuppress.push(deprecation); + } + }; + var queueRunnerFactory = function(options, args) { var failFast = false; if (options.isLeaf) { @@ -4489,9 +4529,9 @@ getJasmineRequireObj().MatchersUtil = function(j$) { */ MatchersUtil.prototype.contains = function(haystack, needle, customTesters) { if (customTesters) { - j$.getEnv().deprecated('Passing custom equality testers to ' + - 'MatchersUtil#contains is deprecated. See ' + - ' for details.'); + j$.getEnv().deprecatedOnceWithStack('Passing custom equality testers ' + + 'to MatchersUtil#contains is deprecated. ' + + 'See for details.'); } if (j$.isSet(haystack)) { @@ -4593,14 +4633,14 @@ getJasmineRequireObj().MatchersUtil = function(j$) { diffBuilder = customTestersOrDiffBuilder; } else { if (customTestersOrDiffBuilder) { - j$.getEnv().deprecated('Passing custom equality testers to ' + - 'MatchersUtil#equals is deprecated. See ' + - ' for details.'); + j$.getEnv().deprecatedOnceWithStack('Passing custom equality testers ' + + 'to MatchersUtil#equals is deprecated. ' + + 'See for details.'); } if (diffBuilderOrNothing) { - j$.getEnv().deprecated('Diff builder should be passed as the third argument ' + - 'to MatchersUtil#equals, not the fourth. ' + + j$.getEnv().deprecatedOnceWithStack('Diff builder should be passed ' + + 'as the third argument to MatchersUtil#equals, not the fourth. ' + 'See for details.'); } diff --git a/spec/core/EnvSpec.js b/spec/core/EnvSpec.js index e32d8eda..1e50b0b8 100644 --- a/spec/core/EnvSpec.js +++ b/spec/core/EnvSpec.js @@ -281,6 +281,60 @@ describe('Env', function() { }); }); + describe('#deprecatedOnceWithStack', function() { + it('includes a stack trace', function() { + spyOn(env, 'deprecated'); + + env.deprecatedOnceWithStack('msg'); + + expect(env.deprecated).toHaveBeenCalled(); + var msg = env.deprecated.calls.argsFor(0)[0]; + expect(msg).toContain('msg'); + expect(msg).toContain('EnvSpec.js'); + expect(msg).not.toContain('Error'); + }); + + describe('When verboseDeprecations is true', function() { + it('calls #deprecated every time', function() { + env.configure({ verboseDeprecations: true }); + spyOn(env, 'deprecated'); + + env.deprecatedOnceWithStack('msg'); + env.deprecatedOnceWithStack('msg'); + expect(env.deprecated).toHaveBeenCalledWith( + jasmine.stringMatching(/msg/) + ); + expect(env.deprecated).toHaveBeenCalledTimes(2); + expect(env.deprecated).not.toHaveBeenCalledWith( + jasmine.stringMatching(/only once/) + ); + }); + }); + + describe('When verboseDeprecations is false', function() { + it('calls #deprecated once per unique message', function() { + env.configure({ verboseDeprecations: false }); + spyOn(env, 'deprecated'); + + env.deprecatedOnceWithStack('foo'); + env.deprecatedOnceWithStack('bar'); + env.deprecatedOnceWithStack('foo'); + + expect(env.deprecated).toHaveBeenCalledWith( + jasmine.stringMatching( + /foo\nNote: This message will be shown only once. Set config.verboseDeprecations to true to see every occurrence/m + ) + ); + expect(env.deprecated).toHaveBeenCalledWith( + jasmine.stringMatching( + /bar\nNote: This message will be shown only once. Set config.verboseDeprecations to true to see every occurrence/m + ) + ); + expect(env.deprecated).toHaveBeenCalledTimes(2); + }); + }); + }); + describe('when not constructed with suppressLoadErrors: true', function() { it('installs a global error handler on construction', function() { var globalErrors = jasmine.createSpyObj('globalErrors', [ diff --git a/spec/core/integration/CustomAsyncMatchersSpec.js b/spec/core/integration/CustomAsyncMatchersSpec.js index 4acffb7a..bd618c8b 100644 --- a/spec/core/integration/CustomAsyncMatchersSpec.js +++ b/spec/core/integration/CustomAsyncMatchersSpec.js @@ -148,22 +148,33 @@ describe('Custom Async Matchers (Integration)', function() { env.execute(); }); - it('logs a deprecation warning if the matcher factory takes two arguments', function (done) { + it('logs a deprecation once per matcher if the matcher factory takes two arguments', function (done) { var matcherFactory = function (matchersUtil, customEqualityTesters) { return { compare: function () {} }; }; spyOn(env, 'deprecated'); - env.it('a spec', function() { + env.beforeEach(function() { env.addAsyncMatchers({toBeFoo: matcherFactory}); + env.addAsyncMatchers({toBeBar: matcherFactory}); }); + env.it('a spec', function() {}); + env.it('another spec', function() {}); + function jasmineDone() { - expect(env.deprecated).toHaveBeenCalledWith('The matcher factory for "toBeFoo" ' + - 'accepts custom equality testers, but this parameter will no longer be passed ' + - 'in a future release. ' + - 'See for details.'); + expect(env.deprecated).toHaveBeenCalledWith(jasmine.stringMatching( + 'The matcher factory for "toBeFoo" accepts custom equality testers, ' + + 'but this parameter will no longer be passed in a future release. ' + + 'See for details.' + )); + expect(env.deprecated).toHaveBeenCalledWith(jasmine.stringMatching( + 'The matcher factory for "toBeBar" accepts custom equality testers, ' + + 'but this parameter will no longer be passed in a future release. ' + + 'See for details.' + )); + expect(env.deprecated).toHaveBeenCalledTimes(2); done(); } diff --git a/spec/core/integration/CustomMatchersSpec.js b/spec/core/integration/CustomMatchersSpec.js index 5e71a5da..b8424e69 100644 --- a/spec/core/integration/CustomMatchersSpec.js +++ b/spec/core/integration/CustomMatchersSpec.js @@ -267,22 +267,33 @@ describe("Custom Matchers (Integration)", function () { env.execute(); }); - it('logs a deprecation warning if the matcher factory takes two arguments', function(done) { + it('logs a deprecation once per matcher if the matcher factory takes two arguments', function(done) { var matcherFactory = function (matchersUtil, customEqualityTesters) { return { compare: function() {} }; }; spyOn(env, 'deprecated'); - env.it('a spec', function() { + env.beforeEach(function() { env.addMatchers({toBeFoo: matcherFactory}); + env.addMatchers({toBeBar: matcherFactory}); }); + env.it('a spec', function() {}); + env.it('another spec', function() {}); + function jasmineDone() { - expect(env.deprecated).toHaveBeenCalledWith('The matcher factory for "toBeFoo" ' + - 'accepts custom equality testers, but this parameter will no longer be passed ' + - 'in a future release. ' + - 'See for details.'); + expect(env.deprecated).toHaveBeenCalledWith(jasmine.stringMatching( + 'The matcher factory for "toBeFoo" accepts custom equality testers, ' + + 'but this parameter will no longer be passed in a future release. ' + + 'See for details.' + )); + expect(env.deprecated).toHaveBeenCalledWith(jasmine.stringMatching( + 'The matcher factory for "toBeBar" accepts custom equality testers, ' + + 'but this parameter will no longer be passed in a future release. ' + + 'See for details.' + )); + expect(env.deprecated).toHaveBeenCalledTimes(2); done(); } diff --git a/spec/core/matchers/matchersUtilSpec.js b/spec/core/matchers/matchersUtilSpec.js index 82aebaf4..6e7d7178 100644 --- a/spec/core/matchers/matchersUtilSpec.js +++ b/spec/core/matchers/matchersUtilSpec.js @@ -856,20 +856,26 @@ describe("matchersUtil", function() { matchersUtil.equals(0, 0, []); - expect(deprecated).toHaveBeenCalledWith('Passing custom equality testers ' + + expect(deprecated).toHaveBeenCalledWith(jasmine.stringMatching( + 'Passing custom equality testers ' + 'to MatchersUtil#equals is deprecated. ' + - 'See for details.'); + 'See for details.' + )); }); it('logs a deprecation warning when a diffBuilder is provided as the fourth argument', function() { var matchersUtil = new jasmineUnderTest.MatchersUtil(), deprecated = spyOn(jasmineUnderTest.getEnv(), 'deprecated'); + + debugger; matchersUtil.equals(0, 0, null, new jasmineUnderTest.NullDiffBuilder()); - expect(deprecated).toHaveBeenCalledWith('Diff builder should be passed as the ' + + expect(deprecated).toHaveBeenCalledWith(jasmine.stringMatching( + 'Diff builder should be passed as the ' + 'third argument to MatchersUtil#equals, not the fourth. ' + - 'See for details.'); + 'See for details.' + )); }); it('uses a diffBuilder if one is provided as the fourth argument', function() { @@ -936,9 +942,10 @@ describe("matchersUtil", function() { expect(matchersUtil.contains([1, 2], 3, [customTester])).toBe(true); - expect(deprecated).toHaveBeenCalledWith('Passing custom equality testers ' + - 'to MatchersUtil#contains is deprecated. ' + - 'See for details.'); + expect(deprecated).toHaveBeenCalledWith(jasmine.stringMatching( + 'Passing custom equality testers to MatchersUtil#contains is deprecated. ' + + 'See for details.' + )); }); it("uses custom equality testers if passed to the constructor and actual is an Array", function() { diff --git a/spec/helpers/resetEnv.js b/spec/helpers/resetEnv.js new file mode 100644 index 00000000..cf12a0a9 --- /dev/null +++ b/spec/helpers/resetEnv.js @@ -0,0 +1,4 @@ +beforeEach(function() { + // env is stateful. Ensure that it does not leak between tests. + jasmineUnderTest.currentEnv_ = null; +}); diff --git a/spec/support/jasmine-browser.js b/spec/support/jasmine-browser.js index 61076dbb..9ff33cb8 100644 --- a/spec/support/jasmine-browser.js +++ b/spec/support/jasmine-browser.js @@ -27,7 +27,8 @@ module.exports = { 'helpers/integrationMatchers.js', 'helpers/promises.js', 'helpers/requireFastCheck.js', - 'helpers/defineJasmineUnderTest.js' + 'helpers/defineJasmineUnderTest.js', + 'helpers/resetEnv.js' ], random: true, browser: { diff --git a/spec/support/jasmine.json b/spec/support/jasmine.json index b48efde5..5b720636 100644 --- a/spec/support/jasmine.json +++ b/spec/support/jasmine.json @@ -14,7 +14,8 @@ "helpers/integrationMatchers.js", "helpers/promises.js", "helpers/requireFastCheck.js", - "helpers/nodeDefineJasmineUnderTest.js" + "helpers/nodeDefineJasmineUnderTest.js", + "helpers/resetEnv.js" ], "random": true } diff --git a/src/core/Env.js b/src/core/Env.js index 8c887d32..eeeece86 100644 --- a/src/core/Env.js +++ b/src/core/Env.js @@ -32,6 +32,7 @@ getJasmineRequireObj().Env = function(j$) { var currentlyExecutingSuites = []; var currentDeclarationSuite = null; var hasFailures = false; + var deprecationsToSuppress = []; /** * This represents the available options to configure Jasmine. @@ -111,7 +112,19 @@ getJasmineRequireObj().Env = function(j$) { * @type function * @default undefined */ - Promise: undefined + Promise: undefined, + /** + * Whether or not to issue warnings for certain deprecated functionality + * every time it's used. If not set or set to false, deprecation warnings + * for methods that tend to be called frequently will be issued only once + * or otherwise throttled to to prevent the suite output from being flooded + * with warnings. + * @name Configuration#verboseDeprecations + * @since 3.6.0 + * @type Boolean + * @default false + */ + verboseDeprecations: false }; var currentSuite = function() { @@ -205,6 +218,10 @@ getJasmineRequireObj().Env = function(j$) { ); } } + + if (configuration.hasOwnProperty('verboseDeprecations')) { + config.verboseDeprecations = configuration.verboseDeprecations; + } }; /** @@ -279,7 +296,7 @@ getJasmineRequireObj().Env = function(j$) { for (var matcherName in matchersToAdd) { if (matchersToAdd[matcherName].length > 1) { - self.deprecated( + self.deprecatedOnceWithStack( 'The matcher factory for "' + matcherName + '" ' + @@ -304,7 +321,7 @@ getJasmineRequireObj().Env = function(j$) { for (var matcherName in matchersToAdd) { if (matchersToAdd[matcherName].length > 1) { - self.deprecated( + self.deprecatedOnceWithStack( 'The matcher factory for "' + matcherName + '" ' + @@ -612,6 +629,29 @@ getJasmineRequireObj().Env = function(j$) { } }; + this.deprecatedOnceWithStack = function(deprecation) { + var formatter = new j$.ExceptionFormatter(), + stackTrace = formatter + .stack(j$.util.errorWithStack()) + .replace(/^Error\n/m, ''); + + if (config.verboseDeprecations) { + this.deprecated(deprecation + '\n' + stackTrace); + } else { + if (deprecationsToSuppress.indexOf(deprecation) === -1) { + this.deprecated( + deprecation + + '\n' + + 'Note: This message will be shown only once. ' + + 'Set config.verboseDeprecations to true to see every occurrence.\n' + + stackTrace + ); + } + + deprecationsToSuppress.push(deprecation); + } + }; + var queueRunnerFactory = function(options, args) { var failFast = false; if (options.isLeaf) { diff --git a/src/core/matchers/matchersUtil.js b/src/core/matchers/matchersUtil.js index b2367026..d20b990a 100644 --- a/src/core/matchers/matchersUtil.js +++ b/src/core/matchers/matchersUtil.js @@ -34,9 +34,9 @@ getJasmineRequireObj().MatchersUtil = function(j$) { */ MatchersUtil.prototype.contains = function(haystack, needle, customTesters) { if (customTesters) { - j$.getEnv().deprecated('Passing custom equality testers to ' + - 'MatchersUtil#contains is deprecated. See ' + - ' for details.'); + j$.getEnv().deprecatedOnceWithStack('Passing custom equality testers ' + + 'to MatchersUtil#contains is deprecated. ' + + 'See for details.'); } if (j$.isSet(haystack)) { @@ -138,14 +138,14 @@ getJasmineRequireObj().MatchersUtil = function(j$) { diffBuilder = customTestersOrDiffBuilder; } else { if (customTestersOrDiffBuilder) { - j$.getEnv().deprecated('Passing custom equality testers to ' + - 'MatchersUtil#equals is deprecated. See ' + - ' for details.'); + j$.getEnv().deprecatedOnceWithStack('Passing custom equality testers ' + + 'to MatchersUtil#equals is deprecated. ' + + 'See for details.'); } if (diffBuilderOrNothing) { - j$.getEnv().deprecated('Diff builder should be passed as the third argument ' + - 'to MatchersUtil#equals, not the fourth. ' + + j$.getEnv().deprecatedOnceWithStack('Diff builder should be passed ' + + 'as the third argument to MatchersUtil#equals, not the fourth. ' + 'See for details.'); } diff --git a/src/core/requireCore.js b/src/core/requireCore.js index f73b62c2..1bace753 100644 --- a/src/core/requireCore.js +++ b/src/core/requireCore.js @@ -58,7 +58,7 @@ var getJasmineRequireObj = (function(jasmineGlobal) { j$.basicPrettyPrinter_ = j$.makePrettyPrinter(); Object.defineProperty(j$, 'pp', { get: function() { - j$.getEnv().deprecated( + j$.getEnv().deprecatedOnceWithStack( 'jasmine.pp is deprecated and will be removed in a future release. ' + 'Use the pp method of the matchersUtil passed to the matcher factory ' + "or the asymmetric equality tester's `asymmetricMatch` method " + @@ -75,7 +75,7 @@ var getJasmineRequireObj = (function(jasmineGlobal) { }); Object.defineProperty(j$, 'matchersUtil', { get: function() { - j$.getEnv().deprecated( + j$.getEnv().deprecatedOnceWithStack( 'jasmine.matchersUtil is deprecated and will be removed ' + 'in a future release. Use the instance passed to the matcher factory or ' + "the asymmetric equality tester's `asymmetricMatch` method instead. " +