Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 2 additions & 5 deletions packages/kbn-test/src/jest/utils/testbed/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ In order to prevent flakiness in component integration tests, please consider th
});
```

- **Do not use** using `nextTick()`, `waitFor`, or `waitForFunc` helpers in tests. These helpers use `setTimeout` underneath and add latency in the tests, especially on CI where a timeout (even of a few ms) can trigger a timeout error. These helpers will eventually be deprecated once existing tests has been updated.
- **Do not use** using `nextTick()` helper in tests. This helper use `setTimeout` underneath and add latency in the tests, especially on CI where a timeout (even of a few ms) can trigger a timeout error.

- **Do not declare** `component.update()` inside `act()`. Each `act()` call should contain a chunk of actions that updates the internal state(s). The `component.update()` that re-renders the internal DOM needs to be called outside, before asserting against the updated DOM.

Expand Down Expand Up @@ -274,14 +274,11 @@ expect(tableCellsValues).toEqual([

An object with the following methods:

##### `setInputValue(input, value, isAsync)`
##### `setInputValue(input, value)`

Set the value of a form input. The input can either be a test subject (a string) or an Enzyme react wrapper. If you specify a test subject,
you can provide a nested path to access it by separating the parent and child with a dot (e.g. `myForm.followerIndexName`).

`isAsync`: flag that will return a Promise that resolves on the next "tick". This is useful if updating the input triggers
an async operation (like a HTTP request) and we need it to resolve so the DOM gets updated (default: `false`).

```js
await form.setInputValue('myInput', 'some value', true);
```
Expand Down
52 changes: 5 additions & 47 deletions packages/kbn-test/src/jest/utils/testbed/testbed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,64 +157,24 @@ export function registerTestBed<T extends string = string>(
});
};

const waitForFn: TestBed<T>['waitForFn'] = async (predicate, errMessage) => {
const triggeredAt = Date.now();

const MAX_WAIT_TIME = 30000;
const WAIT_INTERVAL = 50;

const process = async (): Promise<void> => {
const isOK = await predicate();

if (isOK) {
// Great! nothing else to do here.
return;
}

const timeElapsed = Date.now() - triggeredAt;
if (timeElapsed > MAX_WAIT_TIME) {
throw new Error(errMessage);
}

return new Promise((resolve) => setTimeout(resolve, WAIT_INTERVAL)).then(() => {
component.update();
return process();
});
};

return process();
};

const waitFor: TestBed<T>['waitFor'] = (testSubject: T, count = 1) => {
return waitForFn(
() => Promise.resolve(exists(testSubject, count)),
`I waited patiently for the "${testSubject}" test subject to appear with no luck. It is nowhere to be found!`
);
};

/**
* ----------------------------------------------------------------
* Forms
* ----------------------------------------------------------------
*/

const setInputValue: TestBed<T>['form']['setInputValue'] = (
input,
value,
isAsync = false
) => {
const setInputValue: TestBed<T>['form']['setInputValue'] = function (input, value) {
if (arguments.length === 3) {
throw new Error(`Passing the "isAsync" arg is not supported anymore.`);
}

const formInput = typeof input === 'string' ? find(input) : input;

if (!formInput.length) {
throw new Error(`Input "${input}" was not found.`);
}
formInput.simulate('change', { target: { value } });
component.update();

if (!isAsync) {
return;
}
return new Promise((resolve) => setTimeout(resolve));
};

const setSelectValue: TestBed<T>['form']['setSelectValue'] = (
Expand Down Expand Up @@ -334,8 +294,6 @@ export function registerTestBed<T extends string = string>(
exists,
find,
setProps,
waitFor,
waitForFn,
table: {
getMetaData,
},
Expand Down
19 changes: 1 addition & 18 deletions packages/kbn-test/src/jest/utils/testbed/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,31 +61,14 @@ export interface TestBed<T = string> {
* @param updatedProps The updated prop object
*/
setProps: (updatedProps: any) => void;
/**
* Helper to wait until an element appears in the DOM as hooks updates cycles are tricky.
* Useful when loading a component that fetches a resource from the server
* and we need to wait for the data to be fetched (and bypass any "loading" state).
*/
waitFor: (testSubject: T, count?: number) => Promise<void>;
waitForFn: (predicate: () => Promise<boolean>, errMessage: string) => Promise<void>;
form: {
/**
* Set the value of a form text input.
*
* In some cases, changing an input value triggers an HTTP request to validate
* the field. Even if we return immediately the response on the mock server we
* still need to wait until the next tick before the DOM updates.
* Setting isAsync to "true" takes care of that.
*
* @param input The form input. Can either be a data-test-subj or a reactWrapper (can be a nested path. e.g. "myForm.myInput").
* @param value The value to set
* @param isAsync If set to true will return a Promise that resolves on the next "tick"
*/
setInputValue: (
input: T | ReactWrapper,
value: string,
isAsync?: boolean
) => Promise<void> | void;
setInputValue: (input: T | ReactWrapper, value: string) => void;
/**
* Set the value of a <EuiSelect /> or a mocked <EuiSuperSelect />
* For the <EuiSuperSelect /> you need to mock it like this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { act } from 'react-dom/test-utils';

import { API_BASE_PATH } from '../../../common/constants';
import { FollowerIndexForm } from '../../app/components/follower_index_form/follower_index_form';
Expand Down Expand Up @@ -98,18 +99,20 @@ describe('Edit follower index', () => {
httpRequestsMockHelpers.setLoadRemoteClustersResponse(remoteClusters);
httpRequestsMockHelpers.setGetFollowerIndexResponse(FOLLOWER_INDEX_EDIT);

testBed = await setup();
await testBed.waitFor('followerIndexForm');
await act(async () => {
testBed = await setup();
});

testBed.component.update();
});

test('is consumed correctly', async () => {
const { actions, form, component, find, waitFor } = testBed;
const { actions, form, component, find } = testBed;

form.setInputValue('maxRetryDelayInput', '10s');

actions.clickSaveForm();
component.update(); // The modal to confirm the update opens
await waitFor('confirmModalTitleText');
find('confirmModalConfirmButton').simulate('click');

await nextTick(); // Make sure the Request went through
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export class JobCreateUi extends Component {
startJobAfterCreation: false,
};

this.lastIndexPatternValidationTime = 0;
this.lastIndexPatternValidationIdx = 0;
}

componentDidMount() {
Expand Down Expand Up @@ -159,7 +159,7 @@ export class JobCreateUi extends Component {
requestIndexPatternValidation = debounce((resetDefaults = true) => {
const indexPattern = this.getIndexPattern();

const lastIndexPatternValidationTime = (this.lastIndexPatternValidationTime = Date.now());
const lastIndexPatternValidationIdx = ++this.lastIndexPatternValidationIdx;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relying on the Date for this as we can simply use an incremental counter. Better for the tests too.

validateIndexPattern(indexPattern)
.then((response) => {
// We don't need to do anything if this component has been unmounted.
Expand All @@ -168,7 +168,7 @@ export class JobCreateUi extends Component {
}

// Only re-request if the index pattern changed.
if (lastIndexPatternValidationTime !== this.lastIndexPatternValidationTime) {
if (lastIndexPatternValidationIdx !== this.lastIndexPatternValidationIdx) {
return;
}

Expand Down Expand Up @@ -291,7 +291,7 @@ export class JobCreateUi extends Component {
}

// Ignore all responses except that to the most recent request.
if (lastIndexPatternValidationTime !== this.lastIndexPatternValidationTime) {
if (lastIndexPatternValidationIdx !== this.lastIndexPatternValidationIdx) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
* 2.0.
*/

export { mockHttpRequest } from './setup_environment';

import { setup as jobCreateSetup } from './job_create.helpers';
import { setup as jobListSetup } from './job_list.helpers';
import { setup as jobCloneSetup } from './job_clone.helpers';

export { nextTick, getRandomString, findTestSubject } from '@kbn/test/jest';

export { mockHttpRequest } from './setup_environment';
export { getRandomString, findTestSubject } from '@kbn/test/jest';

export { wrapComponent } from './setup_context';

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

jest.mock('lodash', () => ({
...jest.requireActual('lodash'),
debounce: (fn: () => unknown) => fn,
}));

jest.mock('../../../crud_app/services/documentation_links', () => {
const coreMocks = jest.requireActual('../../../../../../../src/core/public/mocks');

return {
init: jest.fn(),
documentationLinks: coreMocks.docLinksServiceMock.createStartContract().links,
};
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* 2.0.
*/

import { act } from 'react-dom/test-utils';

import { registerTestBed } from '@kbn/test/jest';
import { rollupJobsStore } from '../../../crud_app/store';
import { JobCreate } from '../../../crud_app/sections';
Expand All @@ -22,7 +24,9 @@ export const setup = (props) => {
// User actions
const clickNextStep = () => {
const button = testBed.find('rollupJobNextButton');
button.simulate('click');
act(() => {
button.simulate('click');
});
component.update();
};

Expand All @@ -34,31 +38,43 @@ export const setup = (props) => {

const clickSave = () => {
const button = testBed.find('rollupJobSaveButton');
button.simulate('click');
act(() => {
button.simulate('click');
});
component.update();
};

// Forms
const fillFormFields = async (step) => {
switch (step) {
case 'logistics':
form.setInputValue('rollupJobName', JOB_TO_CREATE.id);
await form.setInputValue('rollupIndexPattern', JOB_TO_CREATE.indexPattern, true);
form.setInputValue('rollupIndexName', JOB_TO_CREATE.rollupIndex);
act(() => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure what's the difference between await act(async () => { and this version without await and async?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we want to be as synchronous as possible and use act(). In some cases using the synchronous version is not enough because, for example, clicking a button triggers a promise that resolves on the next tick and then updates the state. The way to wait for that next tick is to use the async version await act(async() => {...}).

I usually start with the sync version and if the state does not update I change to the async one. With component integration tests it is not always easy to remember all the moving parts occuring after changing a field value or clicking a button.

form.setInputValue('rollupJobName', JOB_TO_CREATE.id);
});
act(() => {
form.setInputValue('rollupIndexPattern', JOB_TO_CREATE.indexPattern);
});
act(() => {
form.setInputValue('rollupIndexName', JOB_TO_CREATE.rollupIndex);
});
break;
case 'date-histogram':
form.setInputValue('rollupJobInterval', JOB_TO_CREATE.interval);
act(() => {
form.setInputValue('rollupJobInterval', JOB_TO_CREATE.interval);
});
break;
default:
return;
}

component.update();
};

// Navigation
const goToStep = async (targetStep) => {
const stepHandlers = {
1: () => fillFormFields('logistics'),
2: () => fillFormFields('date-histogram'),
1: async () => await fillFormFields('logistics'),
2: async () => await fillFormFields('date-histogram'),
};

let currentStep = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import './jest.mocks';

interface RequestMocks {
jobs?: object;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,13 @@
* 2.0.
*/

import { mockHttpRequest, pageHelpers } from './helpers';

import { act } from 'react-dom/test-utils';
import { setHttp, init as initDocumentation } from '../../crud_app/services';
import { mockHttpRequest, pageHelpers, nextTick } from './helpers';
import { JOB_TO_CLONE, JOB_CLONE_INDEX_PATTERN_CHECK } from './helpers/constants';
import { coreMock, docLinksServiceMock } from '../../../../../../src/core/public/mocks';

jest.mock('lodash', () => ({
...jest.requireActual('lodash'),
debounce: (fn) => fn,
}));

const { setup } = pageHelpers.jobClone;
const {
jobs: [{ config: jobConfig }],
Expand All @@ -29,11 +26,16 @@ describe('Cloning a rollup job through create job wizard', () => {
let startMock;

beforeAll(() => {
jest.useFakeTimers();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering why we would need fakeTimers if we don't use any of advanceTimersByTime or runAllTimers, would you mind explaining?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to have the guarantee that there aren't any timeout running in the tests. We are not using jest for isolated unit tests but for component integration tests which load all the npm dependencies which could potentially use timeouts. And we don't want them in our test.

By adding this on top of our tests we are removing any risk of timing out so I would recommend to declare jest.useFakeTimers(); to every component integration test file.

startMock = coreMock.createStart();
setHttp(startMock.http);
initDocumentation(docLinksServiceMock.createStartContract());
});

afterAll(() => {
jest.useRealTimers();
});

beforeEach(() => {
mockHttpRequest(startMock.http, { indxPatternVldtResp: JOB_CLONE_INDEX_PATTERN_CHECK });

Expand Down Expand Up @@ -149,9 +151,10 @@ describe('Cloning a rollup job through create job wizard', () => {

// Changing the index pattern value after cloning a rollup job should update a number of values.
// On each view of the set up wizard we check for the expected state after this change.
form.setInputValue('rollupIndexPattern', 'test');
// Fires off a network request.
await nextTick();

await act(async () => {
form.setInputValue('rollupIndexPattern', 'test');
});

const {
groups: { date_histogram: dateHistogram },
Expand Down
Loading