Skip to content

Commit

Permalink
Merge pull request #1168 from IBM/investigate-jest-failures
Browse files Browse the repository at this point in the history
refactor(tests): fix memory leak, intermittent failures
  • Loading branch information
davidicus committed May 12, 2020
2 parents 5c35b1a + d87c2e8 commit 3bd2f51
Show file tree
Hide file tree
Showing 39 changed files with 1,568 additions and 1,025 deletions.
5 changes: 4 additions & 1 deletion .eslintrc
Expand Up @@ -73,7 +73,10 @@
// https://nolanlawson.com/2018/03/20/smaller-lodash-bundles-with-webpack-and-babel/
"lodash/chaining": [2, "never"],
"lodash/import-scope": [2, "method"],
"react/destructuring-assignment": [2, "always", { "ignoreClassFields": true }]
"react/destructuring-assignment": [2, "always", { "ignoreClassFields": true }],
"jest/expect-expect": "error",
"jest/prefer-hooks-on-top": "error"
// "jest/consistent-test-it": ["error", { "fn": "test", "withinDescribe": "it" }]
},
// Testcases have less eslint rules applied to them
"overrides": [
Expand Down
2 changes: 1 addition & 1 deletion .travis.yml
Expand Up @@ -23,7 +23,7 @@ install:

script:
- yarn lint
- travis_wait yarn test --runInBand --ci --coverage && cat ./coverage/lcov.info | yarn run coveralls && rm -rf ./coverage # report coveralls status
- travis_wait yarn test --maxWorkers=2 --ci --logHeapUsage --coverage && cat ./coverage/lcov.info | yarn run coveralls && rm -rf ./coverage # report coveralls status

after_success:
- yarn publish-npm
3 changes: 3 additions & 0 deletions .vscode/launch.json
Expand Up @@ -6,12 +6,14 @@
"configurations": [
{
"type": "node",
"runtimeVersion": "10.15.3",
"request": "launch",
"name": "Launch Program",
"program": "${workspaceFolder}/lib/index.js"
},
{
"type": "node",
"runtimeVersion": "10.15.3",
"request": "launch",
"name": "Jest All",
"program": "${workspaceFolder}/node_modules/.bin/jest",
Expand All @@ -26,6 +28,7 @@
},
{
"type": "node",
"runtimeVersion": "10.15.3",
"request": "launch",
"name": "Jest Current File",
"program": "${workspaceFolder}/node_modules/.bin/jest",
Expand Down
2 changes: 1 addition & 1 deletion config/jest/cssTransform.js
@@ -1,7 +1,7 @@
'use strict';

// This is a custom Jest transformer turning style imports into empty objects.
// http://facebook.github.io/jest/docs/tutorial-webpack.html
// https://jestjs.io/docs/en/webpack
module.exports = {
process() {
return 'module.exports = {};';
Expand Down
2 changes: 1 addition & 1 deletion config/jest/fileTransform.js
Expand Up @@ -3,7 +3,7 @@
const path = require('path');

// This is a custom Jest transformer turning file imports into filenames.
// http://facebook.github.io/jest/docs/tutorial-webpack.html
// https://jestjs.io/docs/en/webpack
module.exports = {
process(src, filename) {
return `module.exports = ${JSON.stringify(path.basename(filename))};`;
Expand Down
29 changes: 0 additions & 29 deletions config/jest/setupJSDom.js

This file was deleted.

6 changes: 6 additions & 0 deletions config/jest/setupTest.js
@@ -1 +1,7 @@
import 'jest-styled-components';

beforeEach(() => {
// Every test we write should have at least one assertion. If this fails, we need to look at the invoking test to ensure there's not a promise being swallowed or something.
// Review this for more context: https://github.com/IBM/carbon-addons-iot-react/issues/1143#issuecomment-623577505
expect.hasAssertions();
});
1 change: 1 addition & 0 deletions jest.config.js
Expand Up @@ -86,6 +86,7 @@ module.exports = {
__DEV__: false,
},
setupFiles: ['<rootDir>/config/jest/setup.js'],
testEnvironment: 'jest-environment-jsdom-sixteen',
setupFilesAfterEnv: ['<rootDir>/config/jest/setupTest.js'],
testMatch: [
'<rootDir>/**/__tests__/**/*.js?(x)',
Expand Down
15 changes: 8 additions & 7 deletions package.json
Expand Up @@ -150,13 +150,12 @@
"@storybook/addon-storyshots": "^5.3.17",
"@storybook/addons": "^5.3.17",
"@storybook/react": "^5.3.17",
"@testing-library/dom": "^6.10.1",
"@testing-library/jest-dom": "^5.0.2",
"@testing-library/react": "^9.3.2",
"@testing-library/dom": "^7.2.2",
"@testing-library/jest-dom": "^5.5.0",
"@testing-library/react": "^10.0.3",
"autoprefixer": "^9.4.4",
"babel-core": "^7.0.0-bridge.0",
"babel-eslint": "^10.1.0",
"babel-jest": "^25.3.0",
"babel-loader": "^8.1.0",
"babel-plugin-dev-expression": "^0.2.2",
"babel-plugin-lodash": "^3.3.4",
Expand All @@ -178,7 +177,7 @@
"eslint-config-prettier": "^4.1.0",
"eslint-plugin-babel": "^5.3.0",
"eslint-plugin-import": "^2.14.0",
"eslint-plugin-jest": "^22.1.2",
"eslint-plugin-jest": "^23.8.2",
"eslint-plugin-jsx-a11y": "^6.1.2",
"eslint-plugin-lodash": "^5.1.0",
"eslint-plugin-prettier": "^3.0.1",
Expand All @@ -189,9 +188,9 @@
"file-loader": "^4.0.0",
"full-icu": "^1.3.1",
"husky": "^1.3.1",
"jest": "^24.9.0",
"jest": "^25.5.1",
"jest-environment-jsdom-sixteen": "^1.0.3",
"jest-styled-components": "^6.3.1",
"jsdom": "^15.2.1",
"lint-staged": "^8.1.0",
"moment-timezone": "^0.5.26",
"node-sass": "^4.12.0",
Expand Down Expand Up @@ -225,6 +224,8 @@
"stylelint": "^10.1.0",
"stylelint-config-recommended-scss": "^3.3.0",
"stylelint-scss": "^3.10.0",
"weak": "^1.0.1",
"weak-napi": "^2.0.1",
"webpack": "^4.28.4",
"whatwg-fetch": "^3.0.0"
},
Expand Down
31 changes: 16 additions & 15 deletions src/components/Breadcrumb/Breadcrumb.test.jsx
Expand Up @@ -8,9 +8,6 @@ const commonProps = {
onClick: () => console.log('clicked'),
};

const originalOffsetHeight = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'clientWidth');
const originalOffsetWidth = Object.getOwnPropertyDescriptor(HTMLElement.prototype, 'scrollWidth');

describe('Breadcrumb', () => {
test('overflows when container is smaller than breadcrumbs', () => {
const { container } = render(
Expand Down Expand Up @@ -39,8 +36,8 @@ describe('Breadcrumb with overflow', () => {
});

afterAll(() => {
Object.defineProperty(HTMLElement.prototype, 'clientWidth', originalOffsetHeight);
Object.defineProperty(HTMLElement.prototype, 'scrollWidth', originalOffsetWidth);
delete HTMLElement.prototype.clientWidth;
delete HTMLElement.prototype.scrollWidth;
});

test('overflows when container is smaller than breadcrumbs', () => {
Expand All @@ -55,15 +52,26 @@ describe('Breadcrumb with overflow', () => {
});

describe('has dev console warning(s)', () => {
const originalConsole = console.error;
const originalDev = window.__DEV__;
const originalResizeObserver = window.ResizeObserver;

// Applies only to tests in this describe block
beforeAll(() => {
jest.spyOn(console, 'error').mockImplementation(() => {});
});

beforeEach(() => {
window.__DEV__ = true;
window.ResizeObserver = undefined;
console.error = jest.fn();
});

afterEach(() => {
console.error.mockClear();
window.__DEV__ = originalDev;
window.ResizeObserver = originalResizeObserver;
});

afterAll(() => {
console.error.mockRestore();
});

test('when ResizeObserver is not supported in the current environment', () => {
Expand All @@ -76,12 +84,5 @@ describe('Breadcrumb with overflow', () => {
);
expect(console.error).toHaveBeenCalledTimes(1);
});

afterEach(() => {
// restore to original values
window.__DEV__ = originalDev;
window.ResizeObserver = originalResizeObserver;
console.error = originalConsole;
});
});
});
48 changes: 26 additions & 22 deletions src/components/Card/Card.test.jsx
@@ -1,13 +1,14 @@
import { mount } from 'enzyme';
import React from 'react';
/* eslint-disable*/
import { Tooltip } from 'carbon-components-react';
import { render, fireEvent, waitForElement } from '@testing-library/react';
import { render, fireEvent, waitFor } from '@testing-library/react';
import { Popup20 } from '@carbon/icons-react';

import { CARD_SIZES, CARD_TITLE_HEIGHT, CARD_ACTIONS } from '../../constants/LayoutConstants';
import { settings } from '../../constants/Settings';

import CardRangePicker from './CardRangePicker';
import Card from './Card';
import { settings } from '../../constants/Settings';

const { iotPrefix } = settings;

Expand All @@ -19,17 +20,21 @@ const cardProps = {
};

describe('Card testcases', () => {
test('small', () => {
it('small', () => {
const wrapper = mount(<Card {...cardProps} size={CARD_SIZES.SMALL} />);

// small should have full header
expect(wrapper.find(`.${iotPrefix}--card--header`)).toHaveLength(1);
});

test('child size prop', () => {
it('child size prop', () => {
const childRenderInTitleCard = jest.fn();

mount(<Card title="My Title" size={CARD_SIZES.MEDIUM} children={childRenderInTitleCard} />);
mount(
<Card title="My Title" size={CARD_SIZES.MEDIUM}>
{childRenderInTitleCard}
</Card>
);
expect(childRenderInTitleCard).toHaveBeenCalledWith(
{
width: 0,
Expand All @@ -41,7 +46,7 @@ describe('Card testcases', () => {

const childRenderInNoTitleCard = jest.fn();

mount(<Card size={CARD_SIZES.MEDIUM} children={childRenderInNoTitleCard} />);
mount(<Card size={CARD_SIZES.MEDIUM}>{childRenderInNoTitleCard}</Card>);
expect(childRenderInNoTitleCard).toHaveBeenCalledWith(
{
width: 0,
Expand All @@ -52,7 +57,7 @@ describe('Card testcases', () => {
);
});

test('render icons', () => {
it('render icons', () => {
let wrapper = mount(
<Card {...cardProps} size={CARD_SIZES.SMALL} availableActions={{ range: true }} />
);
Expand All @@ -78,7 +83,7 @@ describe('Card testcases', () => {
expect(wrapper.find(CardRangePicker)).toHaveLength(0);
});

test('additional prop based elements', () => {
it('additional prop based elements', () => {
let wrapper = mount(<Card {...cardProps} size={CARD_SIZES.LARGE} tooltip={tooltipElement} />);
// tooltip prop will render a tooltip in the header
expect(wrapper.find(`.${iotPrefix}--card--header`).find(Tooltip)).toHaveLength(1);
Expand All @@ -90,16 +95,16 @@ describe('Card testcases', () => {
);
expect(wrapper.find(`.${iotPrefix}--card--skeleton-wrapper`)).toHaveLength(1);
});
test('isExpanded', () => {
let wrapper = mount(
it('isExpanded', () => {
const wrapper = mount(
<Card {...cardProps} isExpanded size={CARD_SIZES.LARGE} tooltip={tooltipElement} />
);
//isExpanded renders the modal wrapper around it
// isExpanded renders the modal wrapper around it
expect(wrapper.find('.bx--modal')).toHaveLength(1);
});
test('card actions', () => {
it('card actions', () => {
const mockOnCardAction = jest.fn();
let wrapper = mount(
const wrapper = mount(
<Card
{...cardProps}
isExpanded
Expand All @@ -116,7 +121,7 @@ describe('Card testcases', () => {
expect(mockOnCardAction).toHaveBeenCalledWith(cardProps.id, CARD_ACTIONS.CLOSE_EXPANDED_CARD);

mockOnCardAction.mockClear();
let wrapper2 = mount(
const wrapper2 = mount(
<Card
{...cardProps}
size={CARD_SIZES.LARGE}
Expand All @@ -131,9 +136,9 @@ describe('Card testcases', () => {
.props.onClick();
expect(mockOnCardAction).toHaveBeenCalledWith(cardProps.id, CARD_ACTIONS.OPEN_EXPANDED_CARD);
});
test('card editable actions', async done => {
it('card editable actions', async () => {
const mockOnCardAction = jest.fn();
const { getByRole, getByTitle, getByText } = render(
const { getByTitle, getByText } = render(
<Card
{...cardProps}
isEditable
Expand All @@ -145,25 +150,24 @@ describe('Card testcases', () => {
);
fireEvent.click(getByTitle('Open and close list of options'));
// Click on the first overflow menu item
const firstMenuItem = await waitForElement(() => getByText('Edit card'));
const firstMenuItem = await waitFor(() => getByText('Edit card'));
fireEvent.click(firstMenuItem);
expect(mockOnCardAction).toHaveBeenCalledWith(cardProps.id, CARD_ACTIONS.EDIT_CARD);
mockOnCardAction.mockClear();
// Reopen menu
fireEvent.click(getByTitle('Open and close list of options'));
const secondElement = await waitForElement(() => getByText('Clone card'));
const secondElement = await waitFor(() => getByText('Clone card'));
fireEvent.click(secondElement);
expect(mockOnCardAction).toHaveBeenCalledWith(cardProps.id, CARD_ACTIONS.CLONE_CARD);

// Reopen menu
fireEvent.click(getByTitle('Open and close list of options'));
mockOnCardAction.mockClear();
const thirdElement = await waitForElement(() => getByText('Delete card'));
const thirdElement = await waitFor(() => getByText('Delete card'));
fireEvent.click(thirdElement);
expect(mockOnCardAction).toHaveBeenCalledWith(cardProps.id, CARD_ACTIONS.DELETE_CARD);
done();
});
test('card toolbar renders in header only when there are actions', () => {
it('card toolbar renders in header only when there are actions', () => {
const wrapperWithActions = mount(
<Card {...cardProps} isExpanded size={CARD_SIZES.SMALL} availableActions={{ expand: true }} />
);
Expand Down

0 comments on commit 3bd2f51

Please sign in to comment.