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

[NP] add HTTP resources testing strategies #54908

Merged
merged 5 commits into from
Jan 28, 2020
Merged
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
324 changes: 316 additions & 8 deletions src/core/TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ If the unit under test expects a particular response from a Core API, the test w

#### Example

```ts
```typescript
import { elasticsearchServiceMock } from 'src/core/server/mocks';

test('my test', async () => {
Expand All @@ -59,11 +59,319 @@ test('my test', async () => {
});
```

### Strategies for specific Core APIs
## Strategies for specific Core APIs

#### HTTP Routes
### HTTP Routes
The HTTP API interface is another public contract of Kibana, although not every Kibana endpoint is for external use. When evaluating the required level of test coverage for an HTTP resource, make your judgment based on whether an endpoint is considered to be public or private. Public API is expected to have a higher level of test coverage.
Public API tests should cover the **observable behavior** of the system, therefore they should be close to the real user interactions as much as possible, ideally by using HTTP requests to communicate with the Kibana server as a real user would do.

_How to test route handlers_
##### Preconditions
We are going to add tests for `myPlugin` plugin that allows to format user-provided text, store and retrieve it later.
The plugin has *thin* route controllers isolating all the network layer dependencies and delegating all the logic to the plugin model.

```typescript
class TextFormatter {
public static async format(text: string, sanitizer: Deps['sanitizer']) {
// sanitizer.sanitize throws MisformedTextError when passed text contains HTML markup
const sanitizedText = await sanitizer.sanitize(text);
return sanitizedText;
}

public static async save(text: string, savedObjectsClient: SavedObjectsClient) {
const { id } = await savedObjectsClient.update('myPlugin-type', 'myPlugin', {
userText: text
});
return { id };
}

public static async getById(id: string, savedObjectsClient: SavedObjectsClient) {
const { attributes } = await savedObjectsClient.get('myPlugin-type', id);
return { text: attributes.userText };
}
}
router.get(
{
path: '/myPlugin/formatter',
validate: {
query: schema.object({
text: schema.string({ maxLength: 100 }),
}),
},
},
async (context, request, response) => {
try {
const formattedText = await TextFormatter.format(request.query.text, deps.sanitizer);
return response.ok({ body: formattedText });
} catch(error) {
if (error instanceof MisformedTextError) {
return response.badRequest({ body: error.message })
}

throw e;
}
}
);
router.post(
{
path: '/myPlugin/formatter/text',
validate: {
body: schema.object({
text: schema.string({ maxLength: 100 }),
}),
},
},
async (context, request, response) => {
try {
const { id } = await TextFormatter.save(request.query.text, context.core.savedObjects.client);
return response.ok({ body: { id } });
} catch(error) {
if (SavedObjectsErrorHelpers.isConflictError(error)) {
return response.conflict({ body: error.message })
}
throw e;
}
}
);

router.get(
{
path: '/myPlugin/formatter/text/{id}',
validate: {
params: schema.object({
id: schema.string(),
}),
},
},
async (context, request, response) => {
try {
const { text } = await TextFormatter.getById(request.params.id, context.core.savedObjects.client);
return response.ok({
body: text
});
} catch(error) {
if (SavedObjectsErrorHelpers.isNotFoundError(error)) {
return response.notFound()
}
throw e;
}
}
);
```

#### Unit testing
Unit tests provide the simplest and fastest way to test the logic in your route controllers and plugin models.
Use them whenever adding an integration test is hard and slow due to complex setup or the number of logic permutations.
Since all external core and plugin dependencies are mocked, you don't have the guarantee that the whole system works as
expected.
Pros:
- fast
- easier to debug

Cons:
- doesn't test against real dependencies
- doesn't cover integration with other plugins

###### Example
You can leverage existing unit-test infrastructure for this. You should add `*.test.ts` file and use dependencies mocks to cover the functionality with a broader test suit that covers:
- input permutations
- input edge cases
- expected exception
- interaction with dependencies
```typescript
// src/plugins/my_plugin/server/formatter.test.ts
describe('TextFormatter', () => {
describe('format()', () => {
const sanitizer = sanitizerMock.createSetup();
sanitizer.sanitize.mockImplementation((input: string) => `sanitizer result:${input}`);

it('formats text to a ... format', async () => {
expect(await TextFormatter.format('aaa', sanitizer)).toBe('...');
});

it('calls Sanitizer.sanitize with correct arguments', async () => {
await TextFormatter.format('aaa', sanitizer);
expect(sanitizer.sanitize).toHaveBeenCalledTimes(1);
expect(sanitizer.sanitize).toHaveBeenCalledWith('aaa');
});

it('throws MisformedTextError if passed string contains banned symbols', async () => {
sanitizer.sanitize.mockRejectedValueOnce(new MisformedTextError());
await expect(TextFormatter.format('any', sanitizer)).rejects.toThrow(MisformedTextError);
});
// ... other tests
});
});
```

#### Integration tests
Depending on the number of external dependencies, you can consider implementing several high-level integration tests.
They would work as a set of [smoke tests](https://en.wikipedia.org/wiki/Smoke_testing_(software)) for the most important functionality.
Main subjects for tests should be:
- authenticated / unauthenticated access to an endpoint.
- endpoint validation (params, query, body).
- main business logic.
- dependencies on other plugins.

##### Functional Test Runner
If your plugin relies on the elasticsearch server to store data and supports additional configuration, you can leverage the Functional Test Runner(FTR) to implement integration tests.
FTR bootstraps an elasticsearch and a Kibana instance and runs the test suite against it.
Pros:
- runs the whole Elastic stack
- tests cross-plugin integration
- emulates a real user interaction with the stack
- allows adjusting config values

Cons:
- slow start
- hard to debug
- brittle tests

###### Example
You can reuse existing [api_integration](/test/api_integration/config.js) setup by registering a test file within a [test loader](/test/api_integration/apis/index.js). More about the existing FTR setup in the [contribution guide](/CONTRIBUTING.md#running-specific-kibana-tests)

The tests cover:
- authenticated / non-authenticated user access (when applicable)
```typescript
// TODO after https://github.com/elastic/kibana/pull/53208/
```
- request validation
```typescript
// test/api_integration/apis/my_plugin/something.ts
export default function({ getService }: FtrProviderContext) {
const supertest = getService('supertest');
describe('myPlugin', () => {
it('validate params before to store text', async () => {
const response = await supertest
.post('/myPlugin/formatter/text')
.set('content-type', 'application/json')
.send({ text: 'aaa'.repeat(100) })
.expect(400);

expect(response.body).to.have.property('message');
mshustov marked this conversation as resolved.
Show resolved Hide resolved
expect(response.body.message).to.contain('must have a maximum length of [100]');
});
});
```
- the main logic of the plugin
```typescript
export default function({ getService }: FtrProviderContext) {
const supertest = getService('supertest');
describe('myPlugin', () => {
it('stores text', async () => {
const response = await supertest
.post('/myPlugin/formatter/text')
.set('content-type', 'application/json')
.send({ text: 'aaa' })
.expect(200);

expect(response.body).to.have.property('id');
expect(response.body.id).to.be.a('string');
});

it('retrieves text', async () => {
const { body } = await supertest
.post('/myPlugin/formatter/text')
.set('content-type', 'application/json')
.send({ text: 'bbb' })
.expect(200);

const response = await supertest.get(`/myPlugin/formatter/text/${body.id}`).expect(200);
expect(response.text).be('bbb');
});

it('returns NotFound error when cannot find a text', async () => {
await supertest
.get('/myPlugin/something/missing')
.expect(404, 'Saved object [myPlugin-type/missing] not found');
});
});
```

##### TestUtils
Copy link
Contributor

Choose a reason for hiding this comment

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

Although it's a form of integration testing, I wonder if we shouldn't move this out of the "Integration tests" section into it's own section called "Controller testing" or "controller integration testing". ITO the testing pyramid we would have from bottom to top: unit testing, controller testing, integration testing. Because it's a very lightweight type of integration test we should encourage more "controller testing" than fullstack integration tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TestUtils is not limited by controller testing. For example, you can use it to test any data kept in memory by your plugin (registry pattern).

It can be utilized if your plugin doesn't interact with the elasticsearch server or mocks the own methods doing so.
Runs tests against real Kibana server instance.
Pros:
- runs the real Kibana instance
- tests cross-plugin integration
- emulates a real user interaction with the HTTP resources

Cons:
mshustov marked this conversation as resolved.
Show resolved Hide resolved
- faster than FTR because it doesn't run elasticsearch instance, but still slow
- hard to debug
- doesn't cover Kibana CLI logic

###### Example
To have access to Kibana TestUtils, you should create `integration_tests` folder and import `test_utils` within a test file:
```typescript
// src/plugins/my_plugin/server/integration_tests/formatter.test.ts
import * as kbnTestServer from 'src/test_utils/kbn_server';
mshustov marked this conversation as resolved.
Show resolved Hide resolved

describe('myPlugin', () => {
describe('GET /myPlugin/formatter', () => {
let root: ReturnType<typeof kbnTestServer.createRoot>;
beforeAll(async () => {
root = kbnTestServer.createRoot();
await root.setup();
await root.start();
mshustov marked this conversation as resolved.
Show resolved Hide resolved
}, 30000);

afterAll(async () => await root.shutdown());
it('validates given text', async () => {
const response = await kbnTestServer.request
.get(root, '/myPlugin/formatter')
.query({ text: 'input string'.repeat(100) })
.expect(400);

expect(response.body).toHaveProperty('message');
});

it('formats given text', async () => {
const response = await kbnTestServer.request
.get(root, '/myPlugin/formatter')
.query({ text: 'input string' })
.expect(200);

expect(response.text).toBe('...');
});

it('returns BadRequest if passed string contains banned symbols', async () => {
await kbnTestServer.request
.get(root, '/myPlugin/formatter')
.query({ text: '<script>' })
.expect(400, 'Text cannot contain unescaped HTML markup.');
});
});
});
```
Sometimes we want to test a route controller logic and don't rely on the internal logic of the platform or a third-party plugin.
Then we can apply a hybrid approach and mock the necessary method of `TextFormatter` model to test how `MisformedTextError`
handled in the route handler without calling `sanitizer` dependency directly.
```typescript
jest.mock('../path/to/model');
import { TextFormatter } from '../path/to/model';
import { MisformedTextError } from '../path/to/sanitizer'

describe('myPlugin', () => {
describe('GET /myPlugin/formatter', () => {
let root: ReturnType<typeof kbnTestServer.createRoot>;
beforeAll(async () => {
root = kbnTestServer.createRoot();
await root.setup();
await root.start();
}, 30000);

afterAll(async () => await root.shutdown());
it('returns BadRequest if Sanitizer throws MisformedTextError', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

TextFormatter.format.mockRejectedValueOnce(new MisformedTextError());

await kbnTestServer.request
.get(root, '/myPlugin/formatter')
.query({ text: 'any text' })
.expect(400, 'bad bad request');
});
});
});
```

### Applications

Expand All @@ -80,7 +388,7 @@ While applications do get an opportunity to unmount and run cleanup logic, it is

By following the [renderApp](./CONVENTIONS.md#applications) convention, you can greatly reduce the amount of logic in your application's mount function. This makes testing your application's actual rendering logic easier.

```tsx
```typescript jsx
/** public/plugin.ts */
class Plugin {
setup(core) {
Expand All @@ -104,7 +412,7 @@ We _could_ still write tests for this logic, but you may find that you're just a
<details>
<summary>See example</summary>

```ts
```typescript
/** public/plugin.test.ts */
jest.mock('./application', () => ({ renderApp: jest.fn() }));
import { coreMock } from 'src/core/public/mocks';
Expand Down Expand Up @@ -144,7 +452,7 @@ describe('Plugin', () => {

The more interesting logic is in `renderApp`:

```ts
```typescript
/** public/application.ts */
import React from 'react';
import ReactDOM from 'react-dom';
Expand Down Expand Up @@ -184,7 +492,7 @@ In testing `renderApp` you should be verifying that:
1) Your application mounts and unmounts correctly
2) Cleanup logic is completed as expected

```ts
```typescript
/** public/application.test.ts */
import { coreMock } from 'src/core/public/mocks';
import { renderApp } from './application';
Expand Down