Skip to content

Commit

Permalink
Make sure test kit migrators are run only once (#154446)
Browse files Browse the repository at this point in the history
Fixes #152472 (hopefully)

In the failing test, we were reusing the _kibana migrator test kit_
migrator more than once.
This can lead to unexpected results, as the `runMigrations()` method
closes and flushes logging appenders when it is run.
This PR ensures that each migrator in the test kit is run only once.
  • Loading branch information
gsoldevila authored Apr 6, 2023
1 parent d32cac4 commit 1f7aaff
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import { type TestElasticsearchUtils } from '@kbn/core-test-helpers-kbn-server';
import { ElasticsearchClient } from '@kbn/core-elasticsearch-server';
import type { MigrationResult } from '@kbn/core-saved-objects-base-server-internal';
import {
readLog,
clearLog,
Expand All @@ -19,13 +18,14 @@ import {
getIdenticalMappingsMigrator,
getIncompatibleMappingsMigrator,
startElasticsearch,
KibanaMigratorTestKit,
} from '../kibana_migrator_test_kit';
import { delay } from '../test_utils';

describe('when migrating to a new version', () => {
let esServer: TestElasticsearchUtils['es'];
let esClient: ElasticsearchClient;
let runMigrations: (rerun?: boolean | undefined) => Promise<MigrationResult[]>;
let migratorTestKitFactory: () => Promise<KibanaMigratorTestKit>;

beforeAll(async () => {
esServer = await startElasticsearch();
Expand All @@ -39,8 +39,9 @@ describe('when migrating to a new version', () => {
describe('and the mappings remain the same', () => {
it('the migrator skips reindexing', async () => {
// we run the migrator with the same identic baseline types
runMigrations = (await getIdenticalMappingsMigrator()).runMigrations;
await runMigrations();
migratorTestKitFactory = () => getIdenticalMappingsMigrator();
const testKit = await migratorTestKitFactory();
await testKit.runMigrations();

const logs = await readLog();
expect(logs).toMatch('INIT -> WAIT_FOR_YELLOW_SOURCE.');
Expand All @@ -67,8 +68,9 @@ describe('when migrating to a new version', () => {
describe("and the mappings' changes are still compatible", () => {
it('the migrator skips reindexing', async () => {
// we run the migrator with altered, compatible mappings
runMigrations = (await getCompatibleMappingsMigrator()).runMigrations;
await runMigrations();
migratorTestKitFactory = () => getCompatibleMappingsMigrator();
const testKit = await migratorTestKitFactory();
await testKit.runMigrations();

const logs = await readLog();
expect(logs).toMatch('INIT -> WAIT_FOR_YELLOW_SOURCE.');
Expand All @@ -94,9 +96,10 @@ describe('when migrating to a new version', () => {

describe("and the mappings' changes are NOT compatible", () => {
it('the migrator reindexes documents to a new index', async () => {
// we run the migrator with altered, compatible mappings
runMigrations = (await getIncompatibleMappingsMigrator()).runMigrations;
await runMigrations();
// we run the migrator with incompatible mappings
migratorTestKitFactory = () => getIncompatibleMappingsMigrator();
const testKit = await migratorTestKitFactory();
await testKit.runMigrations();

const logs = await readLog();
expect(logs).toMatch('INIT -> WAIT_FOR_YELLOW_SOURCE.');
Expand All @@ -115,8 +118,9 @@ describe('when migrating to a new version', () => {

afterEach(async () => {
// we run the migrator again to ensure that the next time state is loaded everything still works as expected
const migratorTestKit = await migratorTestKitFactory();
await clearLog();
await runMigrations(true);
await migratorTestKit.runMigrations();

const logs = await readLog();
expect(logs).toMatch('INIT -> WAIT_FOR_YELLOW_SOURCE.');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import type { ISavedObjectsRepository } from '@kbn/core-saved-objects-api-server
import { getDocLinks, getDocLinksMeta } from '@kbn/doc-links';
import type { DocLinksServiceStart } from '@kbn/core-doc-links-server';
import { baselineDocuments, baselineTypes } from './kibana_migrator_test_kit.fixtures';
import { delay } from './test_utils';

export const defaultLogFilePath = Path.join(__dirname, 'kibana_migrator_test_kit.log');

Expand Down Expand Up @@ -121,6 +122,7 @@ export const getKibanaMigratorTestKit = async ({
types = [],
logFilePath = defaultLogFilePath,
}: KibanaMigratorTestKitParams = {}): Promise<KibanaMigratorTestKit> => {
let hasRun = false;
const loggingSystem = new LoggingSystem();
const loggerFactory = loggingSystem.asLoggerFactory();

Expand All @@ -147,9 +149,13 @@ export const getKibanaMigratorTestKit = async ({
kibanaBranch
);

const runMigrations = async (rerun?: boolean) => {
const runMigrations = async () => {
if (hasRun) {
throw new Error('The test kit migrator can only be run once. Please instantiate it again.');
}
hasRun = true;
migrator.prepareMigrations();
const migrationResults = await migrator.runMigrations({ rerun });
const migrationResults = await migrator.runMigrations();
await loggingSystem.stop();
return migrationResults;
};
Expand Down Expand Up @@ -385,6 +391,7 @@ export const getIncompatibleMappingsMigrator = async ({
};

export const readLog = async (logFilePath: string = defaultLogFilePath): Promise<string> => {
await delay(0.1);
return await fs.readFile(logFilePath, 'utf-8');
};

Expand Down

0 comments on commit 1f7aaff

Please sign in to comment.