Skip to content

Commit

Permalink
Update to Jest 22 (#11956)
Browse files Browse the repository at this point in the history 
* Bump deps to Jest 22

* Prevent jsdom from logging intentionally thrown errors

This relies on our existing special field that we use to mute errors.
Perhaps, it would be better to instead rely on preventDefault() directly.
I outlined a possible strategy here: #11098 (comment)

* Update snapshots

* Mock out a method called by ReactART that now throws

* Calling .click() no longer works, dispatch event instead

* Fix incorrect SVG element creation in test

* Render SVG elements inside <svg> to avoid extra warnings

* Fix range input test to use numeric value

* Fix creating SVG element in test

* Replace brittle test that relied on jsdom behavior

The test passed in jsdom due to its implementation details.

The original intention was to test the mutation method, but it was removed a while ago.

Following @nhunzaker's suggestion, I moved the tests to ReactDOMInput and adjusted them to not rely on implementation details.

* Add a workaround for the expected extra client-side warning

This is a bit ugly but it's just two places. I think we can live with this.

* Only warn once for mismatches caused by bad attribute casing

We used to warn both about bad casing and about a mismatch.
The mismatch warning was a bit confusing. We didn't know we warned twice because jsdom didn't faithfully emulate SVG.

This changes the behavior to only leave the warning about bad casing if that's what caused the mismatch.
It also adjusts the test to have an expectation that matches the real world behavior.

* Add an expected warning per comment in the same test
  • Loading branch information
@gaearon
gaearon authored Jan 4, 2018
1 parent 4d37040 commit d289d4b
Show file tree
Hide file tree
Showing 14 changed files with 724 additions and 321 deletions.
7 changes: 2 additions & 5 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"babel-code-frame": "^6.26.0",
"babel-core": "^6.0.0",
"babel-eslint": "^7.1.0",
"babel-jest": "^21.3.0-beta.4",
"babel-jest": "^22.0.4",
"babel-plugin-check-es2015-constants": "^6.5.0",
"babel-plugin-external-helpers": "^6.22.0",
"babel-plugin-syntax-trailing-function-commas": "^6.5.0",
Expand Down Expand Up @@ -67,10 +67,7 @@
"gzip-js": "~0.3.2",
"gzip-size": "^3.0.0",
"jasmine-check": "^1.0.0-rc.0",
"jest": "^21.3.0-beta.4",
"jest-config": "^21.3.0-beta.4",
"jest-jasmine2": "^21.3.0-beta.4",
"jest-runtime": "^21.3.0-beta.4",
"jest": "^22.0.4",
"merge-stream": "^1.0.0",
"minimatch": "^3.0.4",
"minimist": "^1.2.0",
Expand Down
24 changes: 15 additions & 9 deletions packages/react-art/src/__tests__/ReactART-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@ function testDOMNodeStructure(domNode, expectedStructure) {
}

describe('ReactART', () => {
let container;

beforeEach(() => {
container = document.createElement('div');
document.body.appendChild(container);

ARTCurrentMode.setCurrent(ARTSVGMode);

Group = ReactART.Group;
Expand Down Expand Up @@ -104,6 +109,11 @@ describe('ReactART', () => {
};
});

afterEach(() => {
document.body.removeChild(container);
container = null;
});

it('should have the correct lifecycle state', () => {
let instance = <TestComponent />;
instance = ReactTestUtils.renderIntoDocument(instance);
Expand Down Expand Up @@ -142,7 +152,6 @@ describe('ReactART', () => {
});

it('should be able to reorder components', () => {
const container = document.createElement('div');
const instance = ReactDOM.render(
<TestComponent flipped={false} />,
container,
Expand Down Expand Up @@ -189,8 +198,6 @@ describe('ReactART', () => {
});

it('should be able to reorder many components', () => {
const container = document.createElement('div');

class Component extends React.Component {
render() {
const chars = this.props.chars.split('');
Expand Down Expand Up @@ -296,17 +303,13 @@ describe('ReactART', () => {
);
}
}

const container = document.createElement('div');
ReactDOM.render(<Outer />, container);
expect(ref).not.toBeDefined();
ReactDOM.render(<Outer mountCustomShape={true} />, container);
expect(ref.constructor).toBe(CustomShape);
});

it('adds and updates event handlers', () => {
const container = document.createElement('div');

function render(onClick) {
return ReactDOM.render(
<Surface>
Expand All @@ -319,8 +322,11 @@ describe('ReactART', () => {
function doClick(instance) {
const path = ReactDOM.findDOMNode(instance).querySelector('path');

// ReactTestUtils.Simulate.click doesn't work with SVG elements
path.click();
path.dispatchEvent(
new MouseEvent('click', {
bubbles: true,
}),
);
}

const onClick1 = jest.fn();
Expand Down
6 changes: 3 additions & 3 deletions packages/react-art/src/__tests__/__snapshots__/ReactART-test.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`ReactARTComponents should generate a <Shape> with props for drawing the Circle 1`] = `
<Shape
d={
{
Object {
"_pivotX": 0,
"_pivotY": -10,
"path": Array [
Expand All @@ -28,7 +28,7 @@ exports[`ReactARTComponents should generate a <Shape> with props for drawing the
exports[`ReactARTComponents should generate a <Shape> with props for drawing the Rectangle 1`] = `
<Shape
d={
{
Object {
"_pivotX": 0,
"_pivotY": 0,
"path": Array [
Expand All @@ -54,7 +54,7 @@ exports[`ReactARTComponents should generate a <Shape> with props for drawing the
exports[`ReactARTComponents should generate a <Shape> with props for drawing the Wedge 1`] = `
<Shape
d={
{
Object {
"_pivotX": 0,
"_pivotY": 50,
"path": Array [
Expand Down
21 changes: 4 additions & 17 deletions packages/react-dom/src/__tests__/DOMPropertyOperations-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ describe('DOMPropertyOperations', () => {
});

it('should set values as namespace attributes if necessary', () => {
const container = document.createElement('svg');
const container = document.createElementNS(
'http://www.w3.org/2000/svg',
'svg',
);
ReactDOM.render(<image xlinkHref="about:blank" />, container);
expect(
container.firstChild.getAttributeNS(
Expand Down Expand Up @@ -113,22 +116,6 @@ describe('DOMPropertyOperations', () => {
ReactDOM.render(<div hidden={false} />, container);
expect(container.firstChild.hasAttribute('hidden')).toBe(false);
});
});

describe('value mutation method', function() {
it('should update an empty attribute to zero', function() {
const container = document.createElement('div');
ReactDOM.render(
<input type="radio" value="" onChange={function() {}} />,
container,
);
spyOnDevAndProd(container.firstChild, 'setAttribute');
ReactDOM.render(
<input type="radio" value={0} onChange={function() {}} />,
container,
);
expect(container.firstChild.setAttribute.calls.count()).toBe(1);
});

it('should always assign the value attribute for non-inputs', function() {
const container = document.createElement('div');
Expand Down
14 changes: 8 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMComponent-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1162,12 +1162,14 @@ describe('ReactDOMComponent', () => {
expect(returnedValue).toContain('</menuitem>');

expect(function() {
ReactDOM.render(
<menu>
<menuitem>children</menuitem>
</menu>,
container,
);
expect(() => {
ReactDOM.render(
<menu>
<menuitem>children</menuitem>
</menu>,
container,
);
}).toWarnDev('The tag <menuitem> is unrecognized in this browser.');
}).toThrowError(
'menuitem is a void element tag and must neither have `children` nor use ' +
'`dangerouslySetInnerHTML`.',
Expand Down
28 changes: 28 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMInput-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,34 @@ describe('ReactDOMInput', () => {
expect(node.value).toEqual('0');
});

it('updates the value on radio buttons from "" to 0', function() {
const container = document.createElement('div');
ReactDOM.render(
<input type="radio" value="" onChange={function() {}} />,
container,
);
ReactDOM.render(
<input type="radio" value={0} onChange={function() {}} />,
container,
);
expect(container.firstChild.value).toBe('0');
expect(container.firstChild.getAttribute('value')).toBe('0');
});

it('updates the value on checkboxes from "" to 0', function() {
const container = document.createElement('div');
ReactDOM.render(
<input type="checkbox" value="" onChange={function() {}} />,
container,
);
ReactDOM.render(
<input type="checkbox" value={0} onChange={function() {}} />,
container,
);
expect(container.firstChild.value).toBe('0');
expect(container.firstChild.getAttribute('value')).toBe('0');
});

it('distinguishes precision for extra zeroes in string number values', () => {
class Stub extends React.Component {
state = {
Expand Down
58 changes: 50 additions & 8 deletions packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@ function initModules() {
};
}

const {resetModules, itRenders} = ReactDOMServerIntegrationUtils(initModules);
const {
resetModules,
itRenders,
clientCleanRender,
} = ReactDOMServerIntegrationUtils(initModules);

describe('ReactDOMServerIntegration', () => {
beforeEach(() => {
Expand Down Expand Up @@ -488,21 +492,49 @@ describe('ReactDOMServerIntegration', () => {
);

itRenders('badly cased SVG attribute with a warning', async render => {
const e = await render(<text textlength="10" />, 1);
expect(e.getAttribute('textLength')).toBe('10');
const e = await render(
<svg>
<text textlength="10" />
</svg>,
1,
);
// The discrepancy is expected as long as we emit a warning
// both on the client and the server.
if (render === clientCleanRender) {
// On the client, "textlength" is treated as a case-sensitive
// SVG attribute so the wrong attribute ("textlength") gets set.
expect(e.firstChild.getAttribute('textlength')).toBe('10');
expect(e.firstChild.hasAttribute('textLength')).toBe(false);
} else {
// When parsing HTML (including the hydration case), the browser
// correctly maps "textlength" to "textLength" SVG attribute.
// So it happens to work on the initial render.
expect(e.firstChild.getAttribute('textLength')).toBe('10');
expect(e.firstChild.hasAttribute('textlength')).toBe(false);
}
});

itRenders('no badly cased aliased SVG attribute alias', async render => {
const e = await render(<text strokedasharray="10 10" />, 1);
expect(e.hasAttribute('stroke-dasharray')).toBe(false);
expect(e.getAttribute('strokedasharray')).toBe('10 10');
const e = await render(
<svg>
<text strokedasharray="10 10" />
</svg>,
1,
);
expect(e.firstChild.hasAttribute('stroke-dasharray')).toBe(false);
expect(e.firstChild.getAttribute('strokedasharray')).toBe('10 10');
});

itRenders(
'no badly cased original SVG attribute that is aliased',
async render => {
const e = await render(<text stroke-dasharray="10 10" />, 1);
expect(e.getAttribute('stroke-dasharray')).toBe('10 10');
const e = await render(
<svg>
<text stroke-dasharray="10 10" />
</svg>,
1,
);
expect(e.firstChild.getAttribute('stroke-dasharray')).toBe('10 10');
},
);
});
Expand Down Expand Up @@ -558,6 +590,16 @@ describe('ReactDOMServerIntegration', () => {
);

itRenders('custom attributes for non-standard elements', async render => {
// This test suite generally assumes that we get exactly
// the same warnings (or none) for all scenarios including
// SSR + innerHTML, hydration, and client-side rendering.
// However this particular warning fires only when creating
// DOM nodes on the client side. We force it to fire early
// so that it gets deduplicated later, and doesn't fail the test.
expect(() => {
ReactDOM.render(<nonstandard />, document.createElement('div'));
}).toWarnDev('The tag <nonstandard> is unrecognized in this browser.');

const e = await render(<nonstandard foo="bar" />);
expect(e.getAttribute('foo')).toBe('bar');
});
Expand Down
10 changes: 10 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,16 @@ describe('ReactDOMServerIntegration', () => {
});

itRenders('a non-standard element with text', async render => {
// This test suite generally assumes that we get exactly
// the same warnings (or none) for all scenarios including
// SSR + innerHTML, hydration, and client-side rendering.
// However this particular warning fires only when creating
// DOM nodes on the client side. We force it to fire early
// so that it gets deduplicated later, and doesn't fail the test.
expect(() => {
ReactDOM.render(<nonstandard />, document.createElement('div'));
}).toWarnDev('The tag <nonstandard> is unrecognized in this browser.');

const e = await render(<nonstandard>Text</nonstandard>);
expect(e.tagName).toBe('NONSTANDARD');
expect(e.childNodes.length).toBe(1);
Expand Down
1 change: 1 addition & 0 deletions packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ module.exports = function(initModules) {
itThrowsWhenRendering,
asyncReactDOMRender,
serverRender,
clientCleanRender,
clientRenderOnServerString,
renderIntoDom,
streamRender,
Expand Down
8 changes: 6 additions & 2 deletions packages/react-dom/src/__tests__/validateDOMNesting-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ function expectWarnings(tags, warnings = []) {
warnings = [...warnings];

let element = null;
const container = document.createElement(tags.splice(0, 1));
const containerTag = tags.shift();
const container =
containerTag === 'svg'
? document.createElementNS('http://www.w3.org/2000/svg', containerTag)
: document.createElement(containerTag);

while (tags.length) {
const Tag = tags.pop();
element = <Tag>{element}</Tag>;
Expand Down Expand Up @@ -108,7 +113,6 @@ describe('validateDOMNesting', () => {
'validateDOMNesting(...): <body> cannot appear as a child of <foreignObject>.\n' +
' in body (at **)\n' +
' in foreignObject (at **)',
'<foreignObject /> is using uppercase HTML',
],
);
});
Expand Down
Loading

0 comments on commit d289d4b

Please sign in to comment.