Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail tests on any un-spied warnings #4223

Merged
merged 1 commit into from
Jun 29, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 22 additions & 27 deletions src/isomorphic/modern/class/__tests__/ReactTypeScriptClass-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,13 +311,12 @@ describe('ReactTypeScriptClass', function() {
});

it('throws if no render function is defined', function() {
var warn = jest.genMockFn();
console.error = warn;
spyOn(console, 'error');

expect(() => React.render(React.createElement(Empty), container)).toThrow();

expect(warn.mock.calls.length).toBe(1);
expect(warn.mock.calls[0][0]).toBe(
expect((<any>console.error).argsForCall.length).toBe(1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to typecast this to make TypeScript happy?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah.

expect((<any>console.error).argsForCall[0][0]).toBe(
'Warning: Empty(...): ' +
'No `render` method found on the returned component instance: you may ' +
'have forgotten to define `render` in your component or you may have ' +
Expand Down Expand Up @@ -434,38 +433,36 @@ describe('ReactTypeScriptClass', function() {

it('warns when classic properties are defined on the instance, ' +
'but does not invoke them.', function() {
var warn = jest.genMockFn();
console.error = warn;
spyOn(console, 'error');
getInitialStateWasCalled = false;
getDefaultPropsWasCalled = false;
test(React.createElement(ClassicProperties), 'SPAN', 'foo');
expect(getInitialStateWasCalled).toBe(false);
expect(getDefaultPropsWasCalled).toBe(false);
expect(warn.mock.calls.length).toBe(4);
expect(warn.mock.calls[0][0]).toContain(
expect((<any>console.error).argsForCall.length).toBe(4);
expect((<any>console.error).argsForCall[0][0]).toContain(
'getInitialState was defined on ClassicProperties, ' +
'a plain JavaScript class.'
);
expect(warn.mock.calls[1][0]).toContain(
expect((<any>console.error).argsForCall[1][0]).toContain(
'getDefaultProps was defined on ClassicProperties, ' +
'a plain JavaScript class.'
);
expect(warn.mock.calls[2][0]).toContain(
expect((<any>console.error).argsForCall[2][0]).toContain(
'propTypes was defined as an instance property on ClassicProperties.'
);
expect(warn.mock.calls[3][0]).toContain(
expect((<any>console.error).argsForCall[3][0]).toContain(
'contextTypes was defined as an instance property on ClassicProperties.'
);
});

it('should warn when misspelling shouldComponentUpdate', function() {
var warn = jest.genMockFn();
console.error = warn;
spyOn(console, 'error');

test(React.createElement(MisspelledComponent1), 'SPAN', 'foo');

expect(warn.mock.calls.length).toBe(1);
expect(warn.mock.calls[0][0]).toBe(
expect((<any>console.error).argsForCall.length).toBe(1);
expect((<any>console.error).argsForCall[0][0]).toBe(
'Warning: ' +
'MisspelledComponent1 has a method called componentShouldUpdate(). Did ' +
'you mean shouldComponentUpdate()? The name is phrased as a question ' +
Expand All @@ -474,22 +471,20 @@ describe('ReactTypeScriptClass', function() {
});

it('should warn when misspelling componentWillReceiveProps', function() {
var warn = jest.genMockFn();
console.error = warn;
spyOn(console, 'error');

test(React.createElement(MisspelledComponent2), 'SPAN', 'foo');

expect(warn.mock.calls.length).toBe(1);
expect(warn.mock.calls[0][0]).toBe(
expect((<any>console.error).argsForCall.length).toBe(1);
expect((<any>console.error).argsForCall[0][0]).toBe(
'Warning: ' +
'MisspelledComponent2 has a method called componentWillRecieveProps(). ' +
'Did you mean componentWillReceiveProps()?'
);
});

it('should throw AND warn when trying to access classic APIs', function() {
var warn = jest.genMockFn();
console.error = warn;
spyOn(console, 'error');
var instance = test(
React.createElement(Inner, {name: 'foo'}),
'DIV','foo'
Expand All @@ -499,20 +494,20 @@ describe('ReactTypeScriptClass', function() {
expect(() => instance.isMounted()).toThrow();
expect(() => instance.setProps({ name: 'bar' })).toThrow();
expect(() => instance.replaceProps({ name: 'bar' })).toThrow();
expect(warn.mock.calls.length).toBe(5);
expect(warn.mock.calls[0][0]).toContain(
expect((<any>console.error).argsForCall.length).toBe(5);
expect((<any>console.error).argsForCall[0][0]).toContain(
'getDOMNode(...) is deprecated in plain JavaScript React classes'
);
expect(warn.mock.calls[1][0]).toContain(
expect((<any>console.error).argsForCall[1][0]).toContain(
'replaceState(...) is deprecated in plain JavaScript React classes'
);
expect(warn.mock.calls[2][0]).toContain(
expect((<any>console.error).argsForCall[2][0]).toContain(
'isMounted(...) is deprecated in plain JavaScript React classes'
);
expect(warn.mock.calls[3][0]).toContain(
expect((<any>console.error).argsForCall[3][0]).toContain(
'setProps(...) is deprecated in plain JavaScript React classes'
);
expect(warn.mock.calls[4][0]).toContain(
expect((<any>console.error).argsForCall[4][0]).toContain(
'replaceProps(...) is deprecated in plain JavaScript React classes'
);
});
Expand Down
11 changes: 5 additions & 6 deletions src/renderers/dom/client/__tests__/ReactMount-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,24 +124,23 @@ describe('ReactMount', function() {
var container = document.createElement('container');
container.innerHTML = React.renderToString(<div />) + ' ';

console.error = mocks.getMockFunction();
spyOn(console, 'error');
ReactMount.render(<div />, container);
expect(console.error.mock.calls.length).toBe(1);
expect(console.error.calls.length).toBe(1);

container.innerHTML = ' ' + React.renderToString(<div />);

console.error = mocks.getMockFunction();
ReactMount.render(<div />, container);
expect(console.error.mock.calls.length).toBe(1);
expect(console.error.calls.length).toBe(2);
});

it('should not warn if mounting into non-empty node', function() {
var container = document.createElement('container');
container.innerHTML = '<div></div>';

console.error = mocks.getMockFunction();
spyOn(console, 'error');
ReactMount.render(<div />, container);
expect(console.error.mock.calls.length).toBe(0);
expect(console.error.calls.length).toBe(0);
});

it('should warn when mounting into document.body', function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,10 @@ var ReactTestUtils;
var ReactUpdates;

var reactComponentExpect;
var mocks;

describe('ReactCompositeComponent', function() {

beforeEach(function() {
mocks = require('mocks');

reactComponentExpect = require('reactComponentExpect');
React = require('React');
ReactCurrentOwner = require('ReactCurrentOwner');
Expand Down Expand Up @@ -71,7 +68,6 @@ describe('ReactCompositeComponent', function() {
},
});

console.error = mocks.getMockFunction();
spyOn(console, 'error');
});

Expand Down
3 changes: 3 additions & 0 deletions src/shared/utils/__tests__/traverseAllChildren-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ describe('traverseAllChildren', function() {
});

it('should be called for each child in an iterable without keys', function() {
spyOn(console, 'error');
var threeDivIterable = {
'@@iterator': function() {
var i = 0;
Expand Down Expand Up @@ -332,6 +333,8 @@ describe('traverseAllChildren', function() {
'.2'
);

expect(console.error.calls.length).toBe(1);
expect(console.error.calls[0].args[0]).toContain('Warning: Each child in an array or iterator should have a unique "key" prop.');
});

it('should be called for each child in an iterable with keys', function() {
Expand Down
8 changes: 0 additions & 8 deletions src/test/__tests__/ReactTestUtils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ var React;
var ReactTestUtils;

var mocks;
var warn;

describe('ReactTestUtils', function() {

Expand All @@ -24,13 +23,6 @@ describe('ReactTestUtils', function() {

React = require('React');
ReactTestUtils = require('ReactTestUtils');

warn = console.error;
console.error = mocks.getMockFunction();
});

afterEach(function() {
console.error = warn;
});

it('should have shallow rendering', function() {
Expand Down
4 changes: 2 additions & 2 deletions test/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@

'lib/postDataToURL.browser.js',
'lib/reportTestResults.browser.js',
'lib/jasmine-execute.js',

'../build/react.js',
'../build/react-test.js',
'the-files-to-test.generated.js',
'lib/jasmine-execute.js'
'the-files-to-test.generated.js'
];

if (typeof Function.prototype.bind == 'undefined') {
Expand Down
28 changes: 28 additions & 0 deletions test/lib/jasmine-execute.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,32 @@ document.write('<style> @import \'../vendor/jasmine/jasmine.css?_=' + (+new Date
window.onload = function() {
env.execute();
};

var oldError = console.error;
var newError = function() {
oldError.apply(this, arguments);
var spec = env.currentSpec;
if (spec) {
var expectationResult = new jasmine.ExpectationResult({
passed: false,
message:
'Expected test not to warn. If the warning is expected, mock it ' +
'out using spyOn(console, \'error\'); and test that the warning ' +
'occurs.',
});
spec.addMatcherResult(expectationResult);
}
};
console.error = newError;
// Make sure console.error is set back at the end of each test, or else the
// above logic won't work
env.afterEach(function() {
if (console.error !== newError && !console.error.isSpy) {
var expectationResult = new jasmine.ExpectationResult({
passed: false,
message: 'Test did not tear down console.error mock properly.',
});
env.currentSpec.addMatcherResult(expectationResult);
}
});
})(jasmine.getEnv());