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

refactor(tests): fix memory leak, intermittent failures #1168

Merged
merged 17 commits into from
May 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
469d84d
fix(tests): upgrade jest, isolate and restore mocks, log heap usage i…
tay1orjones May 4, 2020
b50d33b
fix(hierarchylist): properly mock lodash debounce, ensure all tests h…
tay1orjones May 4, 2020
9e8acfe
fix(simplelist): all tests should have at least one assertion
tay1orjones May 5, 2020
0a484b6
fix(jest): use jest-environment-jsdom-sixteen to avoid shared prototypes
tay1orjones May 6, 2020
c726a40
fix(tests): properly mock console.error, listcard assertions, proper …
tay1orjones May 6, 2020
1dfc05c
fix(timepickerspinner): refactor to remove unstable selectionStart tests
tay1orjones May 7, 2020
e149589
fix(ListCard): update snapshot to include data array default
tay1orjones May 7, 2020
d4e4ca4
fix(datetimepicker): tests should use ISO 8601 compliant date string …
tay1orjones May 7, 2020
f554e56
chore(tests): asynchronus tests using async/await syntax do not requi…
tay1orjones May 7, 2020
2bc7520
fix(tests): ensure async tests resolve, refactor unscoped eslint disa…
tay1orjones May 8, 2020
8c07320
fix(test): meet coverage thresholds after refactoring
tay1orjones May 8, 2020
4173fe7
Merge branch 'master' of github.com:IBM/carbon-addons-iot-react into …
tay1orjones May 8, 2020
94c6e7f
test(header): add assertion to the empty onClick test
tay1orjones May 8, 2020
9d91501
refactor(tests): fix eslint errors
tay1orjones May 8, 2020
cde5d35
test(setup): remove empty afterEach
tay1orjones May 11, 2020
8ea871f
Merge branch 'master' of github.com:IBM/carbon-addons-iot-react into …
tay1orjones May 11, 2020
d87c2e8
Merge branch 'master' into investigate-jest-failures
tay1orjones May 12, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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;
tay1orjones marked this conversation as resolved.
Show resolved Hide resolved
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
Original file line number Diff line number Diff line change
@@ -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
Loading