Add SO migration testing guidance to testing guide (#105959)
This commit is contained in:
Josh Dover
GitHub
parent
9edcf9e71e
commit
283349ac2b
|
|
@ -122,7 +122,7 @@ Since Elasticsearch has a default limit of 1000 fields per index, plugins should
|
|||
fields they add to the mappings. Similarly, Saved Object types should never use `dynamic: true` as this can cause an arbitrary
|
||||
amount of fields to be added to the .kibana index.
|
||||
|
||||
## References
|
||||
## References
|
||||
|
||||
Declare <DocLink id="kibDevDocsSavedObjectsIntro" section="References" text="Saved Object references"/> by adding an id, type and name to the
|
||||
`references` array.
|
||||
|
|
@ -155,12 +155,14 @@ identify this reference. This guarantees that the id the reference points to alw
|
|||
visualization id was directly stored in `dashboard.panels[0].visualization` there is a risk that this id gets updated without
|
||||
updating the reference in the references array.
|
||||
|
||||
## Writing migrations
|
||||
## Migrations
|
||||
|
||||
Saved Objects support schema changes between Kibana versions, which we call migrations. Migrations are
|
||||
applied when a Kibana installation is upgraded from one version to the next, when exports are imported via
|
||||
applied when a Kibana installation is upgraded from one version to a newer version, when exports are imported via
|
||||
the Saved Objects Management UI, or when a new object is created via the HTTP API.
|
||||
|
||||
### Writing migrations
|
||||
|
||||
Each Saved Object type may define migrations for its schema. Migrations are specified by the Kibana version number, receive an input document,
|
||||
and must return the fully migrated document to be persisted to Elasticsearch.
|
||||
|
||||
|
|
@ -241,10 +243,11 @@ export const dashboardVisualization: SavedObjectsType = {
|
|||
in which this migration was released. So if you are creating a migration which will
|
||||
be part of the v7.10.0 release, but will also be backported and released as v7.9.3, the migration version should be: 7.9.3.
|
||||
|
||||
Migrations should be written defensively, an exception in a migration function will prevent a Kibana upgrade from succeeding and will cause downtime for our users.
|
||||
Having said that, if a
|
||||
document is encountered that is not in the expected shape, migrations are encouraged to throw an exception to abort the upgrade. In most scenarios, it is better to
|
||||
fail an upgrade than to silently ignore a corrupt document which can cause unexpected behaviour at some future point in time.
|
||||
Migrations should be written defensively, an exception in a migration function will prevent a Kibana upgrade from succeeding and will cause downtime for our users.
|
||||
Having said that, if a document is encountered that is not in the expected shape, migrations are encouraged to throw an exception to abort the upgrade. In most scenarios, it is better to
|
||||
fail an upgrade than to silently ignore a corrupt document which can cause unexpected behaviour at some future point in time. When such a scenario is encountered,
|
||||
the error should be verbose and informative so that the corrupt document can be corrected, if possible.
|
||||
|
||||
It is critical that you have extensive tests to ensure that migrations behave as expected with all possible input documents. Given how simple it is to test all the branch
|
||||
conditions in a migration function and the high impact of a bug in this code, there’s really no reason not to aim for 100% test code coverage.
|
||||
### Testing Migrations
|
||||
|
||||
Bugs in a migration function cause downtime for our users and therefore have a very high impact. Follow the <DocLink id="kibDevTutorialTestingPlugins" section="saved-object-migrations" text="Saved Object migrations section in the plugin testing guide"/>.
|
||||
|
|
|
|||
|
|
@ -569,7 +569,7 @@ describe('renderApp', () => {
|
|||
});
|
||||
```
|
||||
|
||||
### SavedObjects
|
||||
### SavedObjectsClient
|
||||
|
||||
#### Unit Tests
|
||||
|
||||
|
|
@ -794,6 +794,192 @@ Kibana and esArchiver to load fixture data into Elasticsearch.
|
|||
|
||||
_todo: fully worked out example_
|
||||
|
||||
### Saved Objects migrations
|
||||
|
||||
_Also see <DocLink id="kibDevTutorialSavedObject" section="migrations" text="How to write a migration"/>._
|
||||
|
||||
It is critical that you have extensive tests to ensure that migrations behave as expected with all possible input
|
||||
documents. Given how simple it is to test all the branch conditions in a migration function and the high impact of a
|
||||
bug in this code, there’s really no reason not to aim for 100% test code coverage.
|
||||
|
||||
It's recommend that you primarily leverage unit testing with Jest for testing your migration function. Unit tests will
|
||||
be a much more effective approach to testing all the different shapes of input data and edge cases that your migration
|
||||
may need to handle. With more complex migrations that interact with several components or may behave different depending
|
||||
on registry contents (such as Embeddable migrations), we recommend that you use the Jest Integration suite which allows
|
||||
you to create a full instance Kibana and all plugins in memory and leverage the import API to test migrating documents.
|
||||
|
||||
#### Throwing exceptions
|
||||
Keep in mind that any exception thrown by your migration function will cause Kibana to fail to upgrade. This should almost
|
||||
never happen for our end users and we should be exhaustive in our testing to be sure to catch as many edge cases that we
|
||||
could possibly handle. This entails ensuring that the migration is written defensively; we should try to avoid every bug
|
||||
possible in our implementation.
|
||||
|
||||
In general, exceptions should only be thrown when the input data is corrupted and doesn't match the expected schema. In
|
||||
such cases, it's important that an informative error message is included in the exception and we do not rely on implicit
|
||||
runtime exceptions such as "null pointer exceptions" like `TypeError: Cannot read property 'foo' of undefined`.
|
||||
|
||||
#### Unit testing
|
||||
|
||||
Unit testing migration functions is typically pretty straight forward and comparable to other types of Jest testing. In
|
||||
general, you should focus this tier of testing on validating output and testing input edge cases. One focus of this tier
|
||||
should be trying to find edge cases that throw exceptions the migration shouldn't. As you can see in this simple
|
||||
example, the coverage here is very exhaustive and verbose, which is intentional.
|
||||
|
||||
```ts
|
||||
import { migrateCaseFromV7_9_0ToV7_10_0 } from './case_migrations';
|
||||
|
||||
const validInput_7_9_0 = {
|
||||
id: '1',
|
||||
type: 'case',
|
||||
attributes: {
|
||||
connector_id: '1234';
|
||||
}
|
||||
}
|
||||
|
||||
describe('Case migrations v7.7.0 -> v7.8.0', () => {
|
||||
it('transforms the connector field', () => {
|
||||
expect(migrateCaseFromV7_9_0ToV7_10_0(validInput_7_9_0)).toEqual({
|
||||
id: '1',
|
||||
type: 'case',
|
||||
attributes: {
|
||||
connector: {
|
||||
id: '1234', // verify id was moved into subobject
|
||||
name: 'none', // verify new default field was added
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it('handles empty string', () => {
|
||||
expect(migrateCaseFromV7_9_0ToV7_10_0({
|
||||
id: '1',
|
||||
type: 'case',
|
||||
attributes: {
|
||||
connector_id: ''
|
||||
}
|
||||
})).toEqual({
|
||||
id: '1',
|
||||
type: 'case',
|
||||
attributes: {
|
||||
connector: {
|
||||
id: 'none',
|
||||
name: 'none',
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it('handles null', () => {
|
||||
expect(migrateCaseFromV7_9_0ToV7_10_0({
|
||||
id: '1',
|
||||
type: 'case',
|
||||
attributes: {
|
||||
connector_id: null
|
||||
}
|
||||
})).toEqual({
|
||||
id: '1',
|
||||
type: 'case',
|
||||
attributes: {
|
||||
connector: {
|
||||
id: 'none',
|
||||
name: 'none',
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it('handles undefined', () => {
|
||||
expect(migrateCaseFromV7_9_0ToV7_10_0({
|
||||
id: '1',
|
||||
type: 'case',
|
||||
attributes: {
|
||||
// Even though undefined isn't a valid JSON or Elasticsearch value, we should test it anyways since there
|
||||
// could be some JavaScript layer that casts the field to `undefined` for some reason.
|
||||
connector_id: undefined
|
||||
}
|
||||
})).toEqual({
|
||||
id: '1',
|
||||
type: 'case',
|
||||
attributes: {
|
||||
connector: {
|
||||
id: 'none',
|
||||
name: 'none',
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
expect(migrateCaseFromV7_9_0ToV7_10_0({
|
||||
id: '1',
|
||||
type: 'case',
|
||||
attributes: {
|
||||
// also test without the field present at all
|
||||
}
|
||||
})).toEqual({
|
||||
id: '1',
|
||||
type: 'case',
|
||||
attributes: {
|
||||
connector: {
|
||||
id: 'none',
|
||||
name: 'none',
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
#### Integration testing
|
||||
With more complicated migrations, the behavior of the migration may be dependent on values from other plugins which may
|
||||
be difficult or even impossible to test with unit tests. You need to actually bootstrap Kibana, load the plugins, and
|
||||
then test the full end-to-end migration. This type of set up will also test ingesting your documents into Elasticsearch
|
||||
against the mappings defined by your Saved Object type.
|
||||
|
||||
This can be achieved using the `jest_integration` suite and the `kbnTestServer` utility for starting an in-memory
|
||||
instance of Kibana. You can then leverage the import API to test migrations. This API applies the same migrations to
|
||||
imported documents as are applied at Kibana startup and is much easier to work with for testing.
|
||||
|
||||
```ts
|
||||
// You may need to adjust these paths depending on where your test file is located.
|
||||
// The absolute path is src/core/test_helpers/so_migrations
|
||||
import { createTestHarness, SavedObjectTestHarness } from '../../../../src/core/test_helpers/so_migrations';
|
||||
|
||||
describe('my plugin migrations', () => {
|
||||
let testHarness: SavedObjectTestHarness;
|
||||
|
||||
beforeAll(async () => {
|
||||
testHarness = createTestHarness();
|
||||
await testHarness.start();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await testHarness.stop();
|
||||
});
|
||||
|
||||
it('successfully migrates valid case documents', async () => {
|
||||
expect(
|
||||
await testHarness.migrate([
|
||||
{ type: 'case', id: '1', attributes: { connector_id: '1234' }, references: [] },
|
||||
{ type: 'case', id: '2', attributes: { connector_id: '' }, references: [] },
|
||||
{ type: 'case', id: '3', attributes: { connector_id: null }, references: [] },
|
||||
])
|
||||
).toEqual([
|
||||
expect.objectContaining(
|
||||
{ type: 'case', id: '1', attributes: { connector: { id: '1234', name: 'none' } } }),
|
||||
expect.objectContaining(
|
||||
{ type: 'case', id: '2', attributes: { connector: { id: 'none', name: 'none' } } }),
|
||||
expect.objectContaining(
|
||||
{ type: 'case', id: '3', attributes: { connector: { id: 'none', name: 'none' } } }),
|
||||
])
|
||||
})
|
||||
})
|
||||
```
|
||||
|
||||
There are some caveats about using the import/export API for testing migrations:
|
||||
- You cannot test the startup behavior of Kibana this way. This should not have any effect on type migrations but does
|
||||
mean that this method cannot be used for testing the migration algorithm itself.
|
||||
- While not yet supported, if support is added for migrations that affect multiple types, it's possible that the
|
||||
behavior during import may vary slightly from the upgrade behavior.
|
||||
|
||||
### Elasticsearch
|
||||
|
||||
_How to test ES clients_
|
||||
|
|
|
|||
Loading…
Reference in a new issue