Skip to content

Commit

Permalink
[APM] Fix some warnings logged in APM tests (elastic#52487)
Browse files Browse the repository at this point in the history
* [APM] Fix some warnings logged in APM tests

(Seemingly) since the React upgrade in 439708a, our tests have started logging various warnings/errors to the console. The test suite is still passing but it creates a lot of noise.

Changes:

- use `act` or `wait` when appropriate
- mock useFetcher calls
- cleanup in useDelayedVisbility

* Replace tick() with wait()
  • Loading branch information
dgieselaar committed Dec 18, 2019
1 parent f0ae9f4 commit 876c2d9
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { history } from '../../../../utils/history';
import { TransactionOverview } from '..';
import { IUrlParams } from '../../../../context/UrlParamsContext/types';
import * as useServiceTransactionTypesHook from '../../../../hooks/useServiceTransactionTypes';
import * as useFetcherHook from '../../../../hooks/useFetcher';
import { fromQuery } from '../../../shared/Links/url_helpers';
import { Router } from 'react-router-dom';
import { UrlParamsProvider } from '../../../../context/UrlParamsContext';
Expand Down Expand Up @@ -52,6 +53,8 @@ function setup({
.spyOn(useServiceTransactionTypesHook, 'useServiceTransactionTypes')
.mockReturnValue(serviceTransactionTypes);

jest.spyOn(useFetcherHook, 'useFetcher').mockReturnValue({} as any);

return render(
<KibanaCoreContext.Provider value={coreMock}>
<Router history={history}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import {
UrlParamsContext,
useUiFilters
} from '../../../../context/UrlParamsContext';
import { tick } from '../../../../utils/testHelpers';
import { DatePicker } from '../index';
import { IUrlParams } from '../../../../context/UrlParamsContext/types';
import { history } from '../../../../utils/history';
import { mount } from 'enzyme';
import { EuiSuperDatePicker } from '@elastic/eui';
import { MemoryRouter } from 'react-router-dom';
import { wait } from '@testing-library/react';

const mockHistoryPush = jest.spyOn(history, 'push');
const mockRefreshTimeRange = jest.fn();
Expand Down Expand Up @@ -84,7 +84,7 @@ describe('DatePicker', () => {
});
expect(mockRefreshTimeRange).not.toHaveBeenCalled();
jest.advanceTimersByTime(1000);
await tick();
await wait();
expect(mockRefreshTimeRange).toHaveBeenCalled();
wrapper.unmount();
});
Expand All @@ -94,7 +94,7 @@ describe('DatePicker', () => {
mountDatePicker({ refreshPaused: true, refreshInterval: 1000 });
expect(mockRefreshTimeRange).not.toHaveBeenCalled();
jest.advanceTimersByTime(1000);
await tick();
await wait();
expect(mockRefreshTimeRange).not.toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,10 @@ export class Delayed {
public onChange(onChangeCallback: Callback) {
this.onChangeCallback = onChangeCallback;
}

public destroy() {
if (this.timeoutId) {
window.clearTimeout(this.timeoutId);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { renderHook } from '@testing-library/react-hooks';
import {
renderHook,
act,
RenderHookResult
} from '@testing-library/react-hooks';
import { useDelayedVisibility } from '.';

describe('useFetcher', () => {
let hook;
let hook: RenderHookResult<any, any>;

beforeEach(() => {
jest.useFakeTimers();
});
Expand All @@ -26,9 +31,15 @@ describe('useFetcher', () => {
});

hook.rerender(true);
jest.advanceTimersByTime(10);
act(() => {
jest.advanceTimersByTime(10);
});

expect(hook.result.current).toEqual(false);
jest.advanceTimersByTime(50);
act(() => {
jest.advanceTimersByTime(50);
});

expect(hook.result.current).toEqual(true);
});

Expand All @@ -38,8 +49,11 @@ describe('useFetcher', () => {
});

hook.rerender(true);
jest.advanceTimersByTime(100);
act(() => {
jest.advanceTimersByTime(100);
});
hook.rerender(false);

expect(hook.result.current).toEqual(true);
});

Expand All @@ -49,11 +63,22 @@ describe('useFetcher', () => {
});

hook.rerender(true);
jest.advanceTimersByTime(100);

act(() => {
jest.advanceTimersByTime(100);
});

hook.rerender(false);
jest.advanceTimersByTime(900);
act(() => {
jest.advanceTimersByTime(900);
});

expect(hook.result.current).toEqual(true);
jest.advanceTimersByTime(100);

act(() => {
jest.advanceTimersByTime(100);
});

expect(hook.result.current).toEqual(false);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ export function useDelayedVisibility(
setIsVisible(visibility);
});
delayedRef.current = delayed;

return () => {
delayed.destroy();
};
}, [hideDelayMs, showDelayMs, minimumVisibleDuration]);

useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import { Location, History } from 'history';
import { MemoryRouter, Router } from 'react-router-dom';
import moment from 'moment-timezone';
import { IUrlParams } from '../types';
import { tick } from '../../../utils/testHelpers';
import { getParsedDate } from '../helpers';
import { wait } from '@testing-library/react';

function mountParams(location: Location) {
return mount(
Expand Down Expand Up @@ -143,13 +143,13 @@ describe('UrlParamsContext', () => {
</Router>
);

await tick();
await wait();

expect(calls.length).toBe(1);

wrapper.find('button').simulate('click');

await tick();
await wait();

expect(calls.length).toBe(2);

Expand Down Expand Up @@ -194,11 +194,11 @@ describe('UrlParamsContext', () => {
</Router>
);

await tick();
await wait();

wrapper.find('button').simulate('click');

await tick();
await wait();

const params = getDataFromOutput(wrapper);
expect(params.start).toEqual('2000-06-14T00:00:00.000Z');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
*/

import React from 'react';
import { render } from '@testing-library/react';
import { delay, tick } from '../utils/testHelpers';
import { render, wait } from '@testing-library/react';
import { delay } from '../utils/testHelpers';
import { useFetcher } from './useFetcher';
import { KibanaCoreContext } from '../../../observability/public/context/kibana_core';
import { LegacyCoreStart } from 'kibana/public';
Expand Down Expand Up @@ -76,7 +76,8 @@ describe('when simulating race condition', () => {

it('should render "Hello from Peter" after 200ms', async () => {
jest.advanceTimersByTime(200);
await tick();

await wait();

expect(renderSpy).lastCalledWith({
data: 'Hello from Peter',
Expand All @@ -87,7 +88,7 @@ describe('when simulating race condition', () => {

it('should render "Hello from Peter" after 600ms', async () => {
jest.advanceTimersByTime(600);
await tick();
await wait();

expect(renderSpy).lastCalledWith({
data: 'Hello from Peter',
Expand All @@ -98,7 +99,7 @@ describe('when simulating race condition', () => {

it('should should NOT have rendered "Hello from John" at any point', async () => {
jest.advanceTimersByTime(600);
await tick();
await wait();

expect(renderSpy).not.toHaveBeenCalledWith({
data: 'Hello from John',
Expand All @@ -109,7 +110,7 @@ describe('when simulating race condition', () => {

it('should send and receive calls in the right order', async () => {
jest.advanceTimersByTime(600);
await tick();
await wait();

expect(requestCallOrder).toEqual([
['request', 'John', 500],
Expand Down
4 changes: 0 additions & 4 deletions x-pack/legacy/plugins/apm/public/utils/testHelpers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ export async function getRenderedHref(Component: React.FC, location: Location) {
</MemoryRouter>
);

await tick();
await waitForElement(() => el.container.querySelector('a'));

const a = el.container.querySelector('a');
Expand All @@ -75,9 +74,6 @@ export function delay(ms: number) {
return new Promise(resolve => setTimeout(resolve, ms));
}

// Await this when you need to "flush" promises to immediately resolve or throw in tests
export const tick = () => new Promise(resolve => setImmediate(resolve, 0));

export function expectTextsNotInDocument(output: any, texts: string[]) {
texts.forEach(text => {
try {
Expand Down

0 comments on commit 876c2d9

Please sign in to comment.