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

[tests] leaking memory, tests silently failing, intermittent failures in CI #1143

Closed
2 tasks
tay1orjones opened this issue May 4, 2020 · 2 comments · Fixed by #1168
Closed
2 tasks

[tests] leaking memory, tests silently failing, intermittent failures in CI #1143

tay1orjones opened this issue May 4, 2020 · 2 comments · Fixed by #1168
Assignees
Labels
has internal tracking issue issues that have a link to an issue on the internal IBM tracking/reporting repository released type: bug 🐛
Milestone

Comments

@tay1orjones
Copy link
Member

tay1orjones commented May 4, 2020

Internal Watson-IoT/pal-tracking#400

Describe the bug

We're continuing to see intermittent test failures both locally and in CI. Tests unrelated to a current change set will fail, and then pass on re-run.

Following this write up, Running yarn test:base --logHeapUsage --silent --runInBand --no-cache provides a very clear output showing our tests are leaking memory. Heap size grows with each test instead of being relatively flat. Something is preventing tests from being garbage collected after they pass.

Click to expand the resulting console output
$ cross-env NODE_ICU_DATA=node_modules/full-icu TZ=America/Chicago jest --testPathPattern='.*\.test\.js(x)?' --coverage --logHeapUsage --silent --runInBand --no-cache
 PASS  src/components/Table/tableReducer.test.jsx (9.737s, 205 MB heap size)
 PASS  src/utils/__tests__/cardUtilityFunctions.test.js (218 MB heap size)
 PASS  src/components/Table/Table.test.jsx (268 MB heap size)
 PASS  src/components/BarChartCard/BarChartCard.test.jsx (387 MB heap size)
 PASS  src/components/TableCard/TableCard.test.jsx (315 MB heap size)
 PASS  src/components/List/HierarchyList/HierarchyList.test.jsx (10.984s, 276 MB heap size)
 PASS  src/components/Table/TableHead/TableHead.test.jsx (306 MB heap size)
 PASS  src/components/Header/Header.test.jsx (322 MB heap size)
 PASS  src/components/TimeSeriesCard/TimeSeriesCard.test.jsx (350 MB heap size)
 PASS  src/components/Card/DataStateRenderer.test.jsx (359 MB heap size)
 PASS  src/components/TimeSeriesCard/timeSeriesUtils.test.js (369 MB heap size)
 PASS  src/components/Card/Card.test.jsx (395 MB heap size)
 PASS  src/components/DateTimePicker/DateTimePicker.test.jsx (426 MB heap size)
(node:80848) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'src' of undefined
(node:80848) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:80848) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
 PASS  src/components/Dashboard/Dashboard.test.jsx (454 MB heap size)
 PASS  src/components/TileGallery/TileGallery.test.jsx (408 MB heap size)
 PASS  src/components/ImageCard/ImageHotspots.test.jsx (418 MB heap size)
 PASS  src/components/FileDrop/FileDrop.test.jsx (434 MB heap size)
 PASS  src/components/WizardInline/StatefulWizard.test.jsx (464 MB heap size)
 PASS  src/components/List/SimpleList/SimpleList.test.jsx (484 MB heap size)
 PASS  src/components/TimePickerSpinner/TimePickerSpinner.test.jsx (496 MB heap size)
 PASS  src/components/TileCatalogNew/TileCatalogNew.test.jsx (521 MB heap size)
 PASS  src/components/StorybookSnapshots.test.js (31.69s, 837 MB heap size)
 PASS  src/components/SideNav/SideNav.test.jsx (855 MB heap size)
 PASS  src/components/Table/TableCellRenderer/TableCellRenderer.test.jsx (869 MB heap size)
 PASS  src/components/Table/TableDetailWizard/StatefulTableDetailWizard.test.jsx (888 MB heap size)
 PASS  src/utils/__tests__/publicAPI.test.js (917 MB heap size)
 PASS  src/components/Table/TableHead/ColumnHeaderRow/ColumnHeaderRow.test.jsx (940 MB heap size)
 PASS  src/components/WizardModal/WizardModal.test.jsx (953 MB heap size)
 PASS  src/components/TileCatalog/tileCatalogReducer.test.js (963 MB heap size)
 PASS  src/components/Table/TableBody/TableBodyRow/TableBodyRow.test.jsx (519 MB heap size)
 PASS  src/components/TileCatalog/StatefulTileCatalog.test.jsx (537 MB heap size)
 PASS  src/components/Dashboard/CardRenderer.test.jsx (556 MB heap size)
 PASS  src/components/PageWizard/PageWizard.test.jsx (582 MB heap size)
 PASS  src/components/PageTitleBar/PageTitleBar.test.jsx (593 MB heap size)
 PASS  src/components/Table/TableBody/RowActionsCell/RowActionsCell.test.jsx (605 MB heap size)
 PASS  src/components/PageWizard/StatefulPageWizard.test.jsx (625 MB heap size)
 PASS  src/utils/__tests__/componentUtilityFunctions.test.js (641 MB heap size)
 PASS  src/components/IconSwitch/IconSwitch.test.jsx (647 MB heap size)
 PASS  src/components/Breadcrumb/Breadcrumb.test.jsx (680 MB heap size)
 PASS  src/components/Table/StatefulTableMockReducers.test.jsx (727 MB heap size)
 PASS  src/components/List/ListItem/ListItem.test.jsx (753 MB heap size)
 PASS  src/components/GaugeCard/GaugeCard.test.jsx (763 MB heap size)
 PASS  src/components/Card/CardRangePicker.test.jsx (778 MB heap size)
 PASS  src/components/ResourceList/ResourceList.test.jsx (792 MB heap size)
 PASS  src/components/ValueCard/ValueCard.test.jsx (811 MB heap size)
 PASS  src/components/List/List.test.jsx (832 MB heap size)
 PASS  src/components/SimplePagination/SimplePagination.test.jsx (848 MB heap size)
 PASS  src/components/Table/TableHead/FilterHeaderRow/FilterHeaderRow.test.jsx (857 MB heap size)
 PASS  src/components/Dashboard/DashboardGrid.test.jsx (872 MB heap size)
 PASS  src/components/ImageCard/ImageHotspotsRender.test.jsx (880 MB heap size)
 PASS  src/components/ListCard/ListCard.test.jsx (698 MB heap size)
 PASS  src/components/Table/TableDetailWizard/TableDetailWizard.test.jsx (720 MB heap size)
 PASS  src/components/Header/HeaderAction/HeaderActionMenu.test.jsx (732 MB heap size)
 PASS  src/components/WizardInline/WizardInline.test.jsx (761 MB heap size)
 PASS  src/components/Table/Pagination.test.jsx (771 MB heap size)
 PASS  src/components/Accordion/AccordionItemDefer.test.jsx (787 MB heap size)
 PASS  src/components/ComposedModal/ComposedModal.test.jsx (806 MB heap size)
 PASS  src/components/Table/StatefulTable.test.jsx (868 MB heap size)
 PASS  src/components/WizardInline/WizardFooter/WizardFooter.test.jsx (877 MB heap size)
 PASS  src/components/Page/PageHero.test.jsx (889 MB heap size)
 PASS  src/components/ComposedStructuredList/ComposedStructuredList.test.jsx (900 MB heap size)
 PASS  src/components/NavigationBar/NavigationBar.test.jsx (912 MB heap size)
 PASS  src/components/ProgressIndicator/ProgressIndicator.test.jsx (933 MB heap size)
 PASS  src/components/TileCatalog/TileCatalog.test.jsx (946 MB heap size)
 PASS  src/components/List/ListHeader/ListHeader.test.jsx (960 MB heap size)
 PASS  src/components/ImageCard/CardIcon.test.jsx (973 MB heap size)
 PASS  src/components/Button/Button.test.jsx (984 MB heap size)
 PASS  src/components/Table/TableDetailWizard/TableDetailWizardHeader/TableDetailWizardHeader.test.jsx (993 MB heap size)
 PASS  src/components/Page/PageWorkArea.test.jsx (1003 MB heap size)
 PASS  src/components/AddCard/AddCard.test.jsx (1012 MB heap size)

=============================== Coverage summary ===============================
Statements   : 93.77% ( 3085/3290 )
Branches     : 84.95% ( 2269/2671 )
Functions    : 90.09% ( 873/969 )
Lines        : 93.94% ( 2961/3152 )
================================================================================

Test Suites: 70 passed, 70 total
Tests:       727 passed, 727 total
Snapshots:   283 passed, 283 total
Time:        101.671s
✨  Done in 106.77s.

Additionally there is an error present in the console:

(node:38634) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'src' of undefined
(node:38634) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 4)

Its from HierarchyList.test.jsx. Specifically, the two tests using waitForElementToBeRemoved are not resolved, the element is never removed. The promise is rejected, but the error is swallowed because we do not .catch() it, so the tests pass.

The HierarchyList uses lodash's debounce which doesn't play nicely with jest's useFakeTimers. In addition, we can't just mock it to return the function given because we need to actually test the functionality. You search - 150ms later, the dom is updated with the reduced search list.


I think these two things are related. There's a ton of issues in the jest repo on memory leaks, but the first place to start is to ensure the environment is properly configured.

From there, we then ensure the following:

  1. Async operations are finished or rejections caught
  2. Timers are properly mocked (e.g. setInterval, setTimeout).
  3. All references to the global scope are not kept (mocks)

Items to fix

  • Upgrade to Jest 25
  • Anywhere we override or mock a native function/module etc should be encapsulated within a beforeEach/beforeAll block that is within a describe block. All should have a restore of the original function/module within an afterEach/afterAll.
    • This includes usages of jest.useFakeTimers()
    • Also includes places where we override prototype methods on HTMLElement, like in the Breadcrumb.test.jsx
    • Includes console.error mocking, like the one in Table.test.jsx
@tay1orjones tay1orjones self-assigned this May 4, 2020
@tay1orjones tay1orjones added the has internal tracking issue issues that have a link to an issue on the internal IBM tracking/reporting repository label May 4, 2020
@tay1orjones
Copy link
Member Author

tay1orjones commented May 4, 2020

After reviewing some of this with @scottdickerson, he pointed out that one primary way to surface async operations not finished or caught would be to expect.hasAssertions() for every test in the entire suite via a global beforeEach. This is echoed in the jest docs on promises:

Be sure to return the promise - if you omit this return statement, your test will complete before the promise returned from fetchData resolves and then() has a chance to execute the callback.

If you expect a promise to be rejected, use the .catch method. Make sure to add expect.assertions to verify that a certain number of assertions are called. Otherwise a fulfilled promise would not fail the test.

It looks like we currently have 6 tests that fail this.

Also, if we mock lodash to immediately invoke the debounce method we won't have to waitForElementToBeRemoved at all and can immediately assert if the search results are properly rendered. No need to test implementation details of the debounce - we're aiming to test if the search works reducing the result set.

@tay1orjones
Copy link
Member Author

🎉 This issue has been resolved in version 2.77.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@davidicus davidicus added this to the Sprint 48 milestone Jun 3, 2020
@tay1orjones tay1orjones mentioned this issue Jul 8, 2020
32 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has internal tracking issue issues that have a link to an issue on the internal IBM tracking/reporting repository released type: bug 🐛
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants