From f0ea2b59797f4abf3149cdcb2f995c2e4164d8a2 Mon Sep 17 00:00:00 2001 From: Kevin Old Date: Sun, 25 Jan 2015 22:38:05 -0600 Subject: [PATCH] update to use spyOn for console.warn #2749 --- .../__tests__/ReactCSSTransitionGroup-test.js | 5 +- .../__tests__/ReactServerRendering-test.js | 6 +- .../__tests__/ReactDOMInput-test.js | 115 ++++++-------- .../__tests__/ReactContextValidator-test.js | 24 +-- .../class/__tests__/ReactClass-test.js | 141 ++++++++---------- .../__tests__/ReactCompositeComponent-test.js | 74 ++++----- src/utils/__tests__/cloneWithProps-test.js | 16 +- 7 files changed, 157 insertions(+), 224 deletions(-) diff --git a/src/addons/transitions/__tests__/ReactCSSTransitionGroup-test.js b/src/addons/transitions/__tests__/ReactCSSTransitionGroup-test.js index dabd3c5819..3c08014110 100644 --- a/src/addons/transitions/__tests__/ReactCSSTransitionGroup-test.js +++ b/src/addons/transitions/__tests__/ReactCSSTransitionGroup-test.js @@ -26,6 +26,7 @@ describe('ReactCSSTransitionGroup', function() { mocks = require('mocks'); container = document.createElement('div'); + spyOn(console, 'warn'); }); it('should warn after time with no transitionend', function() { @@ -49,8 +50,6 @@ describe('ReactCSSTransitionGroup', function() { expect(a.getDOMNode().childNodes[0].id).toBe('two'); expect(a.getDOMNode().childNodes[1].id).toBe('one'); - console.warn = mocks.getMockFunction(); - // For some reason jst is adding extra setTimeout()s and grunt test isn't, // so we need to do this disgusting hack. for (var i = 0 ; i < setTimeout.mock.calls.length; i++) { @@ -61,7 +60,7 @@ describe('ReactCSSTransitionGroup', function() { } expect(a.getDOMNode().childNodes.length).toBe(2); - expect(console.warn.mock.calls.length).toBe(1); + expect(console.warn.argsForCall.length).toBe(1); }); it('should keep both sets of DOM nodes around', function() { diff --git a/src/browser/server/__tests__/ReactServerRendering-test.js b/src/browser/server/__tests__/ReactServerRendering-test.js index cee8ba19e8..50b03234af 100644 --- a/src/browser/server/__tests__/ReactServerRendering-test.js +++ b/src/browser/server/__tests__/ReactServerRendering-test.js @@ -48,6 +48,7 @@ describe('ReactServerRendering', function() { var DOMProperty = require('DOMProperty'); ID_ATTRIBUTE_NAME = DOMProperty.ID_ATTRIBUTE_NAME; + spyOn(console, 'warn'); }); describe('renderToString', function() { @@ -215,15 +216,12 @@ describe('ReactServerRendering', function() { // Now simulate a situation where the app is not idempotent. React should // warn but do the right thing. - var _warn = console.warn; - console.warn = mocks.getMockFunction(); element.innerHTML = lastMarkup; var instance = React.render(, element); expect(mountCount).toEqual(4); - expect(console.warn.mock.calls.length).toBe(1); + expect(console.warn.argsForCall.length).toBe(1); expect(element.innerHTML.length > 0).toBe(true); expect(element.innerHTML).not.toEqual(lastMarkup); - console.warn = _warn; // Ensure the events system works expect(numClicks).toEqual(0); diff --git a/src/browser/ui/dom/components/__tests__/ReactDOMInput-test.js b/src/browser/ui/dom/components/__tests__/ReactDOMInput-test.js index b20f3b60a2..ae1f3777f5 100644 --- a/src/browser/ui/dom/components/__tests__/ReactDOMInput-test.js +++ b/src/browser/ui/dom/components/__tests__/ReactDOMInput-test.js @@ -25,6 +25,7 @@ describe('ReactDOMInput', function() { React = require('React'); ReactLink = require('ReactLink'); ReactTestUtils = require('ReactTestUtils'); + spyOn(console, 'warn'); }); it('should display `defaultValue` of number 0', function() { @@ -212,42 +213,28 @@ describe('ReactDOMInput', function() { }); it('should warn with value and no onChange handler', function() { - var oldWarn = console.warn; - try { - console.warn = mocks.getMockFunction(); + var link = new ReactLink('yolo', mocks.getMockFunction()); + ReactTestUtils.renderIntoDocument(); + expect(console.warn.argsForCall.length).toBe(0); - var link = new ReactLink('yolo', mocks.getMockFunction()); - ReactTestUtils.renderIntoDocument(); - expect(console.warn.mock.calls.length).toBe(0); - - ReactTestUtils.renderIntoDocument( - - ); - expect(console.warn.mock.calls.length).toBe(0); - ReactTestUtils.renderIntoDocument(); - expect(console.warn.mock.calls.length).toBe(1); - } finally { - console.warn = oldWarn; - } + ReactTestUtils.renderIntoDocument( + + ); + expect(console.warn.argsForCall.length).toBe(0); + ReactTestUtils.renderIntoDocument(); + expect(console.warn.argsForCall.length).toBe(1); }); it('should warn with value and no onChange handler and readOnly specified', function() { - var oldWarn = console.warn; - try { - console.warn = mocks.getMockFunction(); + ReactTestUtils.renderIntoDocument( + + ); + expect(console.warn.argsForCall.length).toBe(0); - ReactTestUtils.renderIntoDocument( - - ); - expect(console.warn.mock.calls.length).toBe(0); - - ReactTestUtils.renderIntoDocument( - - ); - expect(console.warn.mock.calls.length).toBe(1); - } finally { - console.warn = oldWarn; - } + ReactTestUtils.renderIntoDocument( + + ); + expect(console.warn.argsForCall.length).toBe(1); }); it('should throw if both value and valueLink are provided', function() { @@ -294,53 +281,39 @@ describe('ReactDOMInput', function() { }); it('should warn with checked and no onChange handler', function() { - var oldWarn = console.warn; - try { - console.warn = mocks.getMockFunction(); + var node = document.createElement('div'); + var link = new ReactLink(true, mocks.getMockFunction()); + React.render(, node); + expect(console.warn.argsForCall.length).toBe(0); - var node = document.createElement('div'); - var link = new ReactLink(true, mocks.getMockFunction()); - React.render(, node); - expect(console.warn.mock.calls.length).toBe(0); + ReactTestUtils.renderIntoDocument( + + ); + expect(console.warn.argsForCall.length).toBe(0); - ReactTestUtils.renderIntoDocument( - - ); - expect(console.warn.mock.calls.length).toBe(0); + ReactTestUtils.renderIntoDocument( + + ); + expect(console.warn.argsForCall.length).toBe(0); - ReactTestUtils.renderIntoDocument( - - ); - expect(console.warn.mock.calls.length).toBe(0); - - ReactTestUtils.renderIntoDocument(); - expect(console.warn.mock.calls.length).toBe(1); - } finally { - console.warn = oldWarn; - } + ReactTestUtils.renderIntoDocument(); + expect(console.warn.argsForCall.length).toBe(1); }); it('should warn with checked and no onChange handler with readOnly specified', function() { - var oldWarn = console.warn; - try { - console.warn = mocks.getMockFunction(); + ReactTestUtils.renderIntoDocument( + + ); + expect(console.warn.argsForCall.length).toBe(0); - ReactTestUtils.renderIntoDocument( - - ); - expect(console.warn.mock.calls.length).toBe(0); - - ReactTestUtils.renderIntoDocument( - - ); - expect(console.warn.mock.calls.length).toBe(1); - } finally { - console.warn = oldWarn; - } + ReactTestUtils.renderIntoDocument( + + ); + expect(console.warn.argsForCall.length).toBe(1); }); it('should throw if both checked and checkedLink are provided', function() { diff --git a/src/classic/__tests__/ReactContextValidator-test.js b/src/classic/__tests__/ReactContextValidator-test.js index bc504a3531..097d8ad99f 100644 --- a/src/classic/__tests__/ReactContextValidator-test.js +++ b/src/classic/__tests__/ReactContextValidator-test.js @@ -32,7 +32,7 @@ describe('ReactContextValidator', function() { reactComponentExpect = require('reactComponentExpect'); mocks = require('mocks'); - console.warn = mocks.getMockFunction(); + spyOn(console, 'warn'); }); // TODO: This behavior creates a runtime dependency on propTypes. We should @@ -145,8 +145,8 @@ describe('ReactContextValidator', function() { ReactTestUtils.renderIntoDocument(); - expect(console.warn.mock.calls.length).toBe(1); - expect(console.warn.mock.calls[0][0]).toBe( + expect(console.warn.argsForCall.length).toBe(1); + expect(console.warn.argsForCall[0][0]).toBe( 'Warning: Required context `foo` was not specified in `Component`.' ); @@ -171,7 +171,7 @@ describe('ReactContextValidator', function() { ); // Previous call should not error - expect(console.warn.mock.calls.length).toBe(1); + expect(console.warn.argsForCall.length).toBe(1); var ComponentInFooNumberContext = React.createClass({ childContextTypes: { @@ -191,8 +191,8 @@ describe('ReactContextValidator', function() { ReactTestUtils.renderIntoDocument(); - expect(console.warn.mock.calls.length).toBe(2); - expect(console.warn.mock.calls[1][0]).toBe( + expect(console.warn.argsForCall.length).toBe(2); + expect(console.warn.argsForCall[1][0]).toBe( 'Warning: Invalid context `foo` of type `number` supplied ' + 'to `Component`, expected `string`.' + ' Check the render method of `ComponentInFooNumberContext`.' @@ -216,18 +216,18 @@ describe('ReactContextValidator', function() { }); ReactTestUtils.renderIntoDocument(); - expect(console.warn.mock.calls.length).toBe(2); - expect(console.warn.mock.calls[0][0]).toBe( + expect(console.warn.argsForCall.length).toBe(2); + expect(console.warn.argsForCall[0][0]).toBe( 'Warning: Required child context `foo` was not specified in `Component`.' ); - expect(console.warn.mock.calls[1][0]).toBe( + expect(console.warn.argsForCall[1][0]).toBe( 'Warning: Required child context `foo` was not specified in `Component`.' ); ReactTestUtils.renderIntoDocument(); - expect(console.warn.mock.calls.length).toBe(4); - expect(console.warn.mock.calls[3][0]).toBe( + expect(console.warn.argsForCall.length).toBe(4); + expect(console.warn.argsForCall[3][0]).toBe( 'Warning: Invalid child context `foo` of type `number` ' + 'supplied to `Component`, expected `string`.' ); @@ -241,7 +241,7 @@ describe('ReactContextValidator', function() { ); // Previous calls should not log errors - expect(console.warn.mock.calls.length).toBe(4); + expect(console.warn.argsForCall.length).toBe(4); }); }); diff --git a/src/classic/class/__tests__/ReactClass-test.js b/src/classic/class/__tests__/ReactClass-test.js index b0d3b45b5a..64543e6c60 100644 --- a/src/classic/class/__tests__/ReactClass-test.js +++ b/src/classic/class/__tests__/ReactClass-test.js @@ -21,6 +21,7 @@ describe('ReactClass-spec', function() { beforeEach(function() { React = require('React'); ReactTestUtils = require('ReactTestUtils'); + spyOn(console, 'warn'); }); it('should throw when `render` is not specified', function() { @@ -49,10 +50,9 @@ describe('ReactClass-spec', function() { return
; } }); - console.warn = mocks.getMockFunction(); expect(TestComponent.type).toBe(TestComponent); - expect(console.warn.mock.calls.length).toBe(1); - expect(console.warn.mock.calls[0][0]).toBe( + expect(console.warn.argsForCall.length).toBe(1); + expect(console.warn.argsForCall[0][0]).toBe( 'Warning: TestComponent.type is deprecated. Use TestComponent ' + 'directly to access the class.' ); @@ -126,44 +126,37 @@ describe('ReactClass-spec', function() { }); it('should warn when mispelling shouldComponentUpdate', function() { - var warn = console.warn; - console.warn = mocks.getMockFunction(); + React.createClass({ + componentShouldUpdate: function() { + return false; + }, + render: function() { + return
; + } + }); + expect(console.warn.argsForCall.length).toBe(1); + expect(console.warn.argsForCall[0][0]).toBe( + 'A component has a method called componentShouldUpdate(). Did you ' + + 'mean shouldComponentUpdate()? The name is phrased as a question ' + + 'because the function is expected to return a value.' + ); - try { - React.createClass({ - componentShouldUpdate: function() { - return false; - }, - render: function() { - return
; - } - }); - expect(console.warn.mock.calls.length).toBe(1); - expect(console.warn.mock.calls[0][0]).toBe( - 'A component has a method called componentShouldUpdate(). Did you ' + - 'mean shouldComponentUpdate()? The name is phrased as a question ' + - 'because the function is expected to return a value.' - ); + var NamedComponent = React.createClass({ + componentShouldUpdate: function() { + return false; + }, + render: function() { + return
; + } + }); + expect(console.warn.argsForCall.length).toBe(2); + expect(console.warn.argsForCall[1][0]).toBe( + 'NamedComponent has a method called componentShouldUpdate(). Did you ' + + 'mean shouldComponentUpdate()? The name is phrased as a question ' + + 'because the function is expected to return a value.' + ); - var NamedComponent = React.createClass({ - componentShouldUpdate: function() { - return false; - }, - render: function() { - return
; - } - }); - expect(console.warn.mock.calls.length).toBe(2); - expect(console.warn.mock.calls[1][0]).toBe( - 'NamedComponent has a method called componentShouldUpdate(). Did you ' + - 'mean shouldComponentUpdate()? The name is phrased as a question ' + - 'because the function is expected to return a value.' - ); - - ; // Shut up lint - } finally { - console.warn = warn; - } + ; // Shut up lint }); it('should throw if a reserved property is in statics', function() { @@ -192,44 +185,38 @@ describe('ReactClass-spec', function() { // TODO: Consider actually moving these to statics or drop this unit test. xit('should warn when using deprecated non-static spec keys', function() { - var warn = console.warn; - console.warn = mocks.getMockFunction(); - try { - React.createClass({ - mixins: [{}], - propTypes: { - foo: React.PropTypes.string - }, - contextTypes: { - foo: React.PropTypes.string - }, - childContextTypes: { - foo: React.PropTypes.string - }, - render: function() { - return
; - } - }); - expect(console.warn.mock.calls.length).toBe(4); - expect(console.warn.mock.calls[0][0]).toBe( - 'createClass(...): `mixins` is now a static property and should ' + - 'be defined inside "statics".' - ); - expect(console.warn.mock.calls[1][0]).toBe( - 'createClass(...): `propTypes` is now a static property and should ' + - 'be defined inside "statics".' - ); - expect(console.warn.mock.calls[2][0]).toBe( - 'createClass(...): `contextTypes` is now a static property and ' + - 'should be defined inside "statics".' - ); - expect(console.warn.mock.calls[3][0]).toBe( - 'createClass(...): `childContextTypes` is now a static property and ' + - 'should be defined inside "statics".' - ); - } finally { - console.warn = warn; - } + React.createClass({ + mixins: [{}], + propTypes: { + foo: React.PropTypes.string + }, + contextTypes: { + foo: React.PropTypes.string + }, + childContextTypes: { + foo: React.PropTypes.string + }, + render: function() { + return
; + } + }); + expect(console.warn.argsForCall.length).toBe(4); + expect(console.warn.argsForCall[0][0]).toBe( + 'createClass(...): `mixins` is now a static property and should ' + + 'be defined inside "statics".' + ); + expect(console.warn.argsForCall[1][0]).toBe( + 'createClass(...): `propTypes` is now a static property and should ' + + 'be defined inside "statics".' + ); + expect(console.warn.argsForCall[2][0]).toBe( + 'createClass(...): `contextTypes` is now a static property and ' + + 'should be defined inside "statics".' + ); + expect(console.warn.argsForCall[3][0]).toBe( + 'createClass(...): `childContextTypes` is now a static property and ' + + 'should be defined inside "statics".' + ); }); it('should support statics', function() { diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index ee705d4e2f..c01444cea5 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -24,8 +24,6 @@ var ReactTestUtils; var cx; var reactComponentExpect; var mocks; -var warn; - describe('ReactCompositeComponent', function() { @@ -75,12 +73,7 @@ describe('ReactCompositeComponent', function() { } }); - warn = console.warn; - console.warn = mocks.getMockFunction(); - }); - - afterEach(function() { - console.warn = warn; + spyOn(console, 'warn'); }); it('should support rendering to different child types over time', function() { @@ -118,7 +111,6 @@ describe('ReactCompositeComponent', function() { var container = document.createElement('div'); container.innerHTML = markup; - spyOn(console, 'warn'); React.render(, container); expect(console.warn).not.toHaveBeenCalled(); }); @@ -161,8 +153,6 @@ describe('ReactCompositeComponent', function() { }); it('should auto bind methods and values correctly', function() { - spyOn(console, 'warn'); - var ComponentClass = React.createClass({ getInitialState: function() { return {valueToReturn: 'hi'}; @@ -470,35 +460,28 @@ describe('ReactCompositeComponent', function() { }); it('should warn when shouldComponentUpdate() returns undefined', function() { - var warn = console.warn; - console.warn = mocks.getMockFunction(); + var Component = React.createClass({ + getInitialState: function () { + return {bogus: false}; + }, - try { - var Component = React.createClass({ - getInitialState: function () { - return {bogus: false}; - }, + shouldComponentUpdate: function() { + return undefined; + }, - shouldComponentUpdate: function() { - return undefined; - }, + render: function() { + return
; + } + }); - render: function() { - return
; - } - }); + var instance = ReactTestUtils.renderIntoDocument(); + instance.setState({bogus: true}); - var instance = ReactTestUtils.renderIntoDocument(); - instance.setState({bogus: true}); - - expect(console.warn.mock.calls.length).toBe(1); - expect(console.warn.mock.calls[0][0]).toBe( - 'Component.shouldComponentUpdate(): Returned undefined instead of a ' + - 'boolean value. Make sure to return true or false.' - ); - } finally { - console.warn = warn; - } + expect(console.warn.argsForCall.length).toBe(1); + expect(console.warn.argsForCall[0][0]).toBe( + 'Component.shouldComponentUpdate(): Returned undefined instead of a ' + + 'boolean value. Make sure to return true or false.' + ); }); it('should pass context', function() { @@ -579,8 +562,8 @@ describe('ReactCompositeComponent', function() { // Two warnings, one for the component and one for the div // We may want to make this expect one warning in the future - expect(console.warn.mock.calls.length).toBe(1); - expect(console.warn.mock.calls[0][0]).toBe( + expect(console.warn.argsForCall.length).toBe(1); + expect(console.warn.argsForCall[0][0]).toBe( 'Warning: owner-based and parent-based contexts differ '+ '(values: `bar` vs `undefined`) for key (foo) '+ 'while mounting Component (see: http://fb.me/react-context-by-parent)' @@ -622,8 +605,8 @@ describe('ReactCompositeComponent', function() { // Two warnings, one for the component and one for the div // We may want to make this expect one warning in the future - expect(console.warn.mock.calls.length).toBe(1); - expect(console.warn.mock.calls[0][0]).toBe( + expect(console.warn.argsForCall.length).toBe(1); + expect(console.warn.argsForCall[0][0]).toBe( 'Warning: owner-based and parent-based contexts differ ' + '(values: `noise` vs `bar`) for key (foo) while mounting Component ' + '(see: http://fb.me/react-context-by-parent)' @@ -665,7 +648,7 @@ describe('ReactCompositeComponent', function() { }); React.render({componentWithSameContext}, div); - expect(console.warn.mock.calls.length).toBe(0); + expect(console.warn.argsForCall.length).toBe(0); var componentWithDifferentContext = React.withContext({foo: 'noise'}, function() { return ; @@ -674,8 +657,8 @@ describe('ReactCompositeComponent', function() { // Two warnings, one for the component and one for the div // We may want to make this expect one warning in the future - expect(console.warn.mock.calls.length).toBe(1); - expect(console.warn.mock.calls[0][0]).toBe( + expect(console.warn.argsForCall.length).toBe(1); + expect(console.warn.argsForCall[0][0]).toBe( 'Warning: owner-based and parent-based contexts differ ' + '(values: `noise` vs `bar`) for key (foo) while mounting Component ' + '(see: http://fb.me/react-context-by-parent)' @@ -728,8 +711,8 @@ describe('ReactCompositeComponent', function() { // Two warnings, one for the component and one for the div // We may want to make this expect one warning in the future - expect(console.warn.mock.calls.length).toBe(1); - expect(console.warn.mock.calls[0][0]).toBe( + expect(console.warn.argsForCall.length).toBe(1); + expect(console.warn.argsForCall[0][0]).toBe( 'Warning: owner-based and parent-based contexts differ ' + '(values: `noise` vs `bar`) for key (foo) while mounting Component ' + '(see: http://fb.me/react-context-by-parent)' @@ -810,7 +793,6 @@ describe('ReactCompositeComponent', function() { }); it('should disallow nested render calls', function() { - spyOn(console, 'warn'); var Inner = React.createClass({ render: function() { return
; diff --git a/src/utils/__tests__/cloneWithProps-test.js b/src/utils/__tests__/cloneWithProps-test.js index e4712ea00b..f6bd903325 100644 --- a/src/utils/__tests__/cloneWithProps-test.js +++ b/src/utils/__tests__/cloneWithProps-test.js @@ -82,6 +82,8 @@ describe('cloneWithProps', function() { }); it('should warn when cloning with refs', function() { + spyOn(console, 'warn'); + var Grandparent = React.createClass({ render: function() { return
; @@ -97,17 +99,9 @@ describe('cloneWithProps', function() { } }); - var _warn = console.warn; - - try { - console.warn = mocks.getMockFunction(); - - var component = ReactTestUtils.renderIntoDocument(); - expect(component.refs).toBe(emptyObject); - expect(console.warn.mock.calls.length).toBe(1); - } finally { - console.warn = _warn; - } + var component = ReactTestUtils.renderIntoDocument(); + expect(component.refs).toBe(emptyObject); + expect(console.warn.argsForCall.length).toBe(1); }); it('should transfer the key property', function() {