From 4cdb4c43f8742253dee021d6475e53a9d44ce2fa Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Wed, 16 Dec 2020 16:28:10 +0100 Subject: [PATCH 1/5] Attempt to stabilize cloneIndex integration tests --- .../migrationsv2/integration_tests/actions.test.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts b/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts index 8947a5ec2171c..6acad399a0cff 100644 --- a/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts +++ b/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts @@ -209,13 +209,17 @@ describe('migration actions', () => { describe('cloneIndex', () => { afterEach(async () => { try { - await client.indices.delete({ index: 'yellow_then_green_index' }); + await client.indices.delete({ index: 'clone_*' }); } catch (e) { /** ignore */ } }); it('resolves right if cloning into a new target index', () => { - const task = cloneIndex(client, 'existing_index_with_write_block', 'yellow_then_green_index'); + const task = cloneIndex( + client, + 'existing_index_with_write_block', + 'clone_yellow_then_green_index_1' + ); expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Right", @@ -229,7 +233,7 @@ describe('migration actions', () => { it('resolves right after waiting for index status to be green if clone target already existed', async () => { // Create a yellow index await client.indices.create({ - index: 'yellow_then_green_index', + index: 'clone_yellow_then_green_index_2', body: { mappings: { properties: {} }, settings: { @@ -243,7 +247,7 @@ describe('migration actions', () => { const cloneIndexPromise = cloneIndex( client, 'existing_index_with_write_block', - 'yellow_then_green_index' + 'clone_yellow_then_green_index_2' )(); let indexGreen = false; @@ -273,7 +277,7 @@ describe('migration actions', () => { }); }); it('resolves left index_not_found_exception if the source index does not exist', () => { - const task = cloneIndex(client, 'no_such_index', 'yellow_then_green_index'); + const task = cloneIndex(client, 'no_such_index', 'clone_yellow_then_green_index_3'); expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", From 50e998b9e0e6401bd5fa1baf4fc38c8c9a38be0c Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Thu, 17 Dec 2020 17:27:32 +0100 Subject: [PATCH 2/5] Unskip test --- .../migrationsv2/integration_tests/actions.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts b/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts index 45ee6bc508cd0..dbce1a7fe0718 100644 --- a/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts +++ b/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts @@ -207,7 +207,7 @@ describe('migration actions', () => { }); describe('cloneIndex', () => { - afterEach(async () => { + afterAll(async () => { try { await client.indices.delete({ index: 'clone_*' }); } catch (e) { @@ -230,7 +230,7 @@ describe('migration actions', () => { } `); }); - it.skip('resolves right after waiting for index status to be green if clone target already existed', async () => { + it('resolves right after waiting for index status to be green if clone target already existed', async () => { // Create a yellow index await client.indices.create({ index: 'clone_yellow_then_green_index_2', From d9d464cfa257c086be6a83156dfe5c4cc56d9540 Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Fri, 18 Dec 2020 10:58:51 +0100 Subject: [PATCH 3/5] return resolves/rejects and add assertions counts to each test --- .../integration_tests/actions.test.ts | 47 +++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts b/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts index dbce1a7fe0718..50855ba6c0658 100644 --- a/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts +++ b/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts @@ -100,6 +100,7 @@ describe('migration actions', () => { describe('fetchIndices', () => { it('resolves right empty record if no indices were found', async () => { + expect.assertions(1); const task = fetchIndices(client, ['no_such_index']); return expect(task()).resolves.toMatchInlineSnapshot(` Object { @@ -109,6 +110,7 @@ describe('migration actions', () => { `); }); it('resolves right record with found indices', async () => { + expect.assertions(1); const res = (await fetchIndices(client, [ 'no_such_index', 'existing_index_with_docs', @@ -131,6 +133,7 @@ describe('migration actions', () => { await createIndex(client, 'new_index_without_write_block', { properties: {} })(); }); it('resolves right when setting the write block succeeds', async () => { + expect.assertions(1); const task = setWriteBlock(client, 'new_index_without_write_block'); return expect(task()).resolves.toMatchInlineSnapshot(` Object { @@ -140,6 +143,7 @@ describe('migration actions', () => { `); }); it('resolves right when setting a write block on an index that already has one', () => { + expect.assertions(1); const task = setWriteBlock(client, 'existing_index_with_write_block'); return expect(task()).resolves.toMatchInlineSnapshot(` Object { @@ -149,6 +153,7 @@ describe('migration actions', () => { `); }); it('once resolved, prevents further writes to the index', async () => { + expect.assertions(1); const task = setWriteBlock(client, 'new_index_without_write_block'); await task(); const sourceDocs = ([ @@ -162,6 +167,7 @@ describe('migration actions', () => { ).rejects.toMatchObject(expect.anything()); }); it('resolves left index_not_found_exception when the index does not exist', () => { + expect.assertions(1); const task = setWriteBlock(client, 'no_such_index'); return expect(task()).resolves.toMatchInlineSnapshot(` Object { @@ -181,6 +187,7 @@ describe('migration actions', () => { await setWriteBlock(client, 'existing_index_with_write_block_2')(); }); it('resolves right if successful when an index already has a write block', () => { + expect.assertions(1); const task = removeWriteBlock(client, 'existing_index_with_write_block_2'); return expect(task()).resolves.toMatchInlineSnapshot(` Object { @@ -190,6 +197,7 @@ describe('migration actions', () => { `); }); it('resolves right if successful when an index does not have a write block', () => { + expect.assertions(1); const task = removeWriteBlock(client, 'existing_index_without_write_block_2'); return expect(task()).resolves.toMatchInlineSnapshot(` Object { @@ -199,6 +207,7 @@ describe('migration actions', () => { `); }); it('rejects if there is a non-retryable error', () => { + expect.assertions(1); const task = removeWriteBlock(client, 'no_such_index'); return expect(task()).rejects.toMatchInlineSnapshot( `[ResponseError: index_not_found_exception]` @@ -215,12 +224,13 @@ describe('migration actions', () => { } }); it('resolves right if cloning into a new target index', () => { + expect.assertions(1); const task = cloneIndex( client, 'existing_index_with_write_block', 'clone_yellow_then_green_index_1' ); - expect(task()).resolves.toMatchInlineSnapshot(` + return expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Right", "right": Object { @@ -231,6 +241,7 @@ describe('migration actions', () => { `); }); it('resolves right after waiting for index status to be green if clone target already existed', async () => { + expect.assertions(2); // Create a yellow index await client.indices.create({ index: 'clone_yellow_then_green_index_2', @@ -277,8 +288,9 @@ describe('migration actions', () => { }); }); it('resolves left index_not_found_exception if the source index does not exist', () => { + expect.assertions(1); const task = cloneIndex(client, 'no_such_index', 'clone_yellow_then_green_index_3'); - expect(task()).resolves.toMatchInlineSnapshot(` + return expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", "left": Object { @@ -293,6 +305,7 @@ describe('migration actions', () => { // Reindex doesn't return any errors on it's own, so we have to test // together with waitForReindexTask describe('reindex & waitForReindexTask', () => { + expect.assertions(2); it('resolves right when reindex succeeds without reindex script', async () => { const res = (await reindex( client, @@ -324,6 +337,7 @@ describe('migration actions', () => { `); }); it('resolves right when reindex succeeds with reindex script', async () => { + expect.assertions(2); const res = (await reindex( client, 'existing_index_with_docs', @@ -353,6 +367,7 @@ describe('migration actions', () => { `); }); it('resolves right, ignores version conflicts and does not update existing docs when reindex multiple times', async () => { + expect.assertions(3); // Reindex with a script let res = (await reindex( client, @@ -401,6 +416,7 @@ describe('migration actions', () => { `); }); it('resolves right and proceeds to add missing documents if there are some existing docs conflicts', async () => { + expect.assertions(2); // Simulate a reindex that only adds some of the documents from the // source index into the target index await createIndex(client, 'reindex_target_4', { properties: {} })(); @@ -448,6 +464,7 @@ describe('migration actions', () => { `); }); it('resolves left incompatible_mapping_exception if all reindex failures are due to a strict_dynamic_mapping_exception', async () => { + expect.assertions(1); // Simulates one instance having completed the UPDATE_TARGET_MAPPINGS // step which makes the mappings incompatible with outdated documents. // If another instance then tries a reindex it will get a @@ -483,6 +500,7 @@ describe('migration actions', () => { `); }); it('resolves left incompatible_mapping_exception if all reindex failures are due to a mapper_parsing_exception', async () => { + expect.assertions(1); // Simulates one instance having completed the UPDATE_TARGET_MAPPINGS // step which makes the mappings incompatible with outdated documents. // If another instance then tries a reindex it will get a @@ -516,6 +534,7 @@ describe('migration actions', () => { `); }); it('resolves left index_not_found_exception if source index does not exist', async () => { + expect.assertions(1); const res = (await reindex( client, 'no_such_index', @@ -535,6 +554,7 @@ describe('migration actions', () => { `); }); it('resolves left target_index_had_write_block if all failures are due to a write block', async () => { + expect.assertions(1); const res = (await reindex( client, 'existing_index_with_docs', @@ -555,6 +575,7 @@ describe('migration actions', () => { `); }); it('resolves left if requireAlias=true and the target is not an alias', async () => { + expect.assertions(1); const res = (await reindex( client, 'existing_index_with_docs', @@ -578,6 +599,7 @@ describe('migration actions', () => { }); describe('verifyReindex', () => { + expect.assertions(1); it('resolves right if source and target indices have the same amount of documents', async () => { const res = (await reindex( client, @@ -597,6 +619,7 @@ describe('migration actions', () => { `); }); it('resolves left if source and target indices have different amount of documents', () => { + expect.assertions(1); const task = verifyReindex(client, 'existing_index_with_docs', 'existing_index_2'); return expect(task()).resolves.toMatchInlineSnapshot(` Object { @@ -608,6 +631,7 @@ describe('migration actions', () => { `); }); it('rejects if source or target index does not exist', async () => { + expect.assertions(2); let task = verifyReindex(client, 'no_such_index', 'existing_index_2'); await expect(task()).rejects.toMatchInlineSnapshot( `[ResponseError: index_not_found_exception]` @@ -622,6 +646,7 @@ describe('migration actions', () => { describe('searchForOutdatedDocuments', () => { it('only returns documents that match the outdatedDocumentsQuery', async () => { + expect.assertions(2); const resultsWithQuery = ((await searchForOutdatedDocuments( client, 'existing_index_with_docs', @@ -639,6 +664,7 @@ describe('migration actions', () => { expect(resultsWithoutQuery.length).toBe(4); }); it('resolves with _id, _source, _seq_no and _primary_term', async () => { + expect.assertions(1); const results = ((await searchForOutdatedDocuments(client, 'existing_index_with_docs', { match: { title: { query: 'doc' } }, })()) as Either.Right).right.outdatedDocuments; @@ -659,6 +685,7 @@ describe('migration actions', () => { describe('waitForPickupUpdatedMappingsTask', () => { it('rejects if there are failures', async () => { + expect.assertions(1); const res = (await pickupUpdatedMappings( client, 'existing_index_with_write_block' @@ -673,6 +700,7 @@ describe('migration actions', () => { }); }); it('rejects if there is an error', async () => { + expect.assertions(1); const res = (await pickupUpdatedMappings( client, 'no_such_index' @@ -686,6 +714,7 @@ describe('migration actions', () => { `); }); it('resolves right when successful', async () => { + expect.assertions(1); const res = (await pickupUpdatedMappings( client, 'existing_index_with_docs' @@ -704,6 +733,7 @@ describe('migration actions', () => { describe('updateAndPickupMappings', () => { it('resolves right when mappings were updated and picked up', async () => { + expect.assertions(3); // Create an index without any mappings and insert documents into it await createIndex(client, 'existing_index_without_mappings', { dynamic: false as any, @@ -752,6 +782,7 @@ describe('migration actions', () => { describe('updateAliases', () => { describe('remove', () => { it('resolves left index_not_found_exception when the index does not exist', () => { + expect.assertions(1); const task = updateAliases(client, [ { remove: { @@ -773,6 +804,7 @@ describe('migration actions', () => { }); describe('with must_exist=false', () => { it('resolves left alias_not_found_exception when alias does not exist', async () => { + expect.assertions(1); const task = updateAliases(client, [ { remove: { @@ -794,6 +826,7 @@ describe('migration actions', () => { }); describe('with must_exist=true', () => { it('resolves left alias_not_found_exception when alias does not exist on specified index', async () => { + expect.assertions(1); const task = updateAliases(client, [ { remove: { @@ -813,6 +846,7 @@ describe('migration actions', () => { `); }); it('resolves left alias_not_found_exception when alias does not exist', async () => { + expect.assertions(1); const task = updateAliases(client, [ { remove: { @@ -835,6 +869,7 @@ describe('migration actions', () => { }); describe('remove_index', () => { it('left index_not_found_exception if index does not exist', () => { + expect.assertions(1); const task = updateAliases(client, [ { remove_index: { @@ -853,6 +888,7 @@ describe('migration actions', () => { `); }); it('left remove_index_not_a_concrete_index when remove_index targets an alias', () => { + expect.assertions(1); const task = updateAliases(client, [ { remove_index: { @@ -877,6 +913,7 @@ describe('migration actions', () => { await client.indices.delete({ index: 'yellow_then_green_index' }); }); it('resolves right after waiting for an index status to be green if the index already existed', async () => { + expect.assertions(2); // Create a yellow index await client.indices.create( { @@ -919,9 +956,10 @@ describe('migration actions', () => { }); }); it('rejects when there is an unexpected error creating the index', () => { + expect.assertions(1); // Creating an index with the same name as an existing alias to induce // failure - expect( + return expect( createIndex(client, 'existing_index_2_alias', undefined as any)() ).rejects.toMatchInlineSnapshot(`[ResponseError: invalid_index_name_exception]`); }); @@ -929,6 +967,7 @@ describe('migration actions', () => { describe('bulkOverwriteTransformedDocuments', () => { it('resolves right when documents do not yet exist in the index', () => { + expect.assertions(1); const newDocs = ([ { _source: { title: 'doc 5' } }, { _source: { title: 'doc 6' } }, @@ -943,6 +982,7 @@ describe('migration actions', () => { `); }); it('resolves right even if there were some version_conflict_engine_exception', async () => { + expect.assertions(1); const existingDocs = ((await searchForOutdatedDocuments( client, 'existing_index_with_docs', @@ -961,6 +1001,7 @@ describe('migration actions', () => { `); }); it('rejects if there are errors', () => { + expect.assertions(1); const newDocs = ([ { _source: { title: 'doc 5' } }, { _source: { title: 'doc 6' } }, From 426c2f81d469258a37a1284067bdc2544b088e9b Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Fri, 18 Dec 2020 12:08:03 +0100 Subject: [PATCH 4/5] Await don't return expect promises --- .../integration_tests/actions.test.ts | 92 +++++++++---------- 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts b/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts index 50855ba6c0658..05432d65c0558 100644 --- a/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts +++ b/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts @@ -102,7 +102,7 @@ describe('migration actions', () => { it('resolves right empty record if no indices were found', async () => { expect.assertions(1); const task = fetchIndices(client, ['no_such_index']); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Right", "right": Object {}, @@ -116,7 +116,7 @@ describe('migration actions', () => { 'existing_index_with_docs', ])()) as Either.Right; - return expect(res.right).toEqual( + expect(res.right).toEqual( expect.objectContaining({ existing_index_with_docs: { aliases: {}, @@ -135,17 +135,17 @@ describe('migration actions', () => { it('resolves right when setting the write block succeeds', async () => { expect.assertions(1); const task = setWriteBlock(client, 'new_index_without_write_block'); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Right", "right": "set_write_block_succeeded", } `); }); - it('resolves right when setting a write block on an index that already has one', () => { + it('resolves right when setting a write block on an index that already has one', async () => { expect.assertions(1); const task = setWriteBlock(client, 'existing_index_with_write_block'); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Right", "right": "set_write_block_succeeded", @@ -162,14 +162,14 @@ describe('migration actions', () => { { _source: { title: 'doc 3' } }, { _source: { title: 'doc 4' } }, ] as unknown) as SavedObjectsRawDoc[]; - return expect( + await expect( bulkOverwriteTransformedDocuments(client, 'new_index_without_write_block', sourceDocs)() ).rejects.toMatchObject(expect.anything()); }); - it('resolves left index_not_found_exception when the index does not exist', () => { + it('resolves left index_not_found_exception when the index does not exist', async () => { expect.assertions(1); const task = setWriteBlock(client, 'no_such_index'); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", "left": Object { @@ -186,30 +186,30 @@ describe('migration actions', () => { await createIndex(client, 'existing_index_with_write_block_2', { properties: {} })(); await setWriteBlock(client, 'existing_index_with_write_block_2')(); }); - it('resolves right if successful when an index already has a write block', () => { + it('resolves right if successful when an index already has a write block', async () => { expect.assertions(1); const task = removeWriteBlock(client, 'existing_index_with_write_block_2'); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Right", "right": "remove_write_block_succeeded", } `); }); - it('resolves right if successful when an index does not have a write block', () => { + it('resolves right if successful when an index does not have a write block', async () => { expect.assertions(1); const task = removeWriteBlock(client, 'existing_index_without_write_block_2'); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Right", "right": "remove_write_block_succeeded", } `); }); - it('rejects if there is a non-retryable error', () => { + it('rejects if there is a non-retryable error', async () => { expect.assertions(1); const task = removeWriteBlock(client, 'no_such_index'); - return expect(task()).rejects.toMatchInlineSnapshot( + await expect(task()).rejects.toMatchInlineSnapshot( `[ResponseError: index_not_found_exception]` ); }); @@ -223,14 +223,14 @@ describe('migration actions', () => { /** ignore */ } }); - it('resolves right if cloning into a new target index', () => { + it('resolves right if cloning into a new target index', async () => { expect.assertions(1); const task = cloneIndex( client, 'existing_index_with_write_block', 'clone_yellow_then_green_index_1' ); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Right", "right": Object { @@ -273,7 +273,7 @@ describe('migration actions', () => { indexGreen = true; }, 10); - return cloneIndexPromise.then((res) => { + await cloneIndexPromise.then((res) => { // Assert that the promise didn't resolve before the index became green expect(indexGreen).toBe(true); expect(res).toMatchInlineSnapshot(` @@ -287,10 +287,10 @@ describe('migration actions', () => { `); }); }); - it('resolves left index_not_found_exception if the source index does not exist', () => { + it('resolves left index_not_found_exception if the source index does not exist', async () => { expect.assertions(1); const task = cloneIndex(client, 'no_such_index', 'clone_yellow_then_green_index_3'); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", "left": Object { @@ -543,7 +543,7 @@ describe('migration actions', () => { false )()) as Either.Right; const task = waitForReindexTask(client, res.right.taskId, '10s'); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", "left": Object { @@ -565,7 +565,7 @@ describe('migration actions', () => { const task = waitForReindexTask(client, res.right.taskId, '10s'); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", "left": Object { @@ -586,7 +586,7 @@ describe('migration actions', () => { const task = waitForReindexTask(client, res.right.taskId, '10s'); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", "left": Object { @@ -599,8 +599,8 @@ describe('migration actions', () => { }); describe('verifyReindex', () => { - expect.assertions(1); it('resolves right if source and target indices have the same amount of documents', async () => { + expect.assertions(1); const res = (await reindex( client, 'existing_index_with_docs', @@ -618,10 +618,10 @@ describe('migration actions', () => { } `); }); - it('resolves left if source and target indices have different amount of documents', () => { + it('resolves left if source and target indices have different amount of documents', async () => { expect.assertions(1); const task = verifyReindex(client, 'existing_index_with_docs', 'existing_index_2'); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", "left": Object { @@ -695,7 +695,7 @@ describe('migration actions', () => { // We can't do a snapshot match because the response includes an index // id which ES assigns dynamically - return expect(task()).rejects.toMatchObject({ + await expect(task()).rejects.toMatchObject({ message: /pickupUpdatedMappings task failed with the following failures:\n\[\{\"index\":\"existing_index_with_write_block\"/, }); }); @@ -708,7 +708,7 @@ describe('migration actions', () => { const task = waitForPickupUpdatedMappingsTask(client, res.right.taskId, '10s'); - return expect(task()).rejects.toMatchInlineSnapshot(` + await expect(task()).rejects.toMatchInlineSnapshot(` [Error: pickupUpdatedMappings task failed with the following error: {"type":"index_not_found_exception","reason":"no such index [no_such_index]","resource.type":"index_or_alias","resource.id":"no_such_index","index_uuid":"_na_","index":"no_such_index"}] `); @@ -722,7 +722,7 @@ describe('migration actions', () => { const task = waitForPickupUpdatedMappingsTask(client, res.right.taskId, '10s'); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Right", "right": "pickup_updated_mappings_succeeded", @@ -775,13 +775,13 @@ describe('migration actions', () => { 'existing_index_without_mappings', { match: { title: { query: 'doc' } } } )()) as Either.Right).right.outdatedDocuments; - return expect(pickedUpSearchResults.length).toBe(4); + expect(pickedUpSearchResults.length).toBe(4); }); }); describe('updateAliases', () => { describe('remove', () => { - it('resolves left index_not_found_exception when the index does not exist', () => { + it('resolves left index_not_found_exception when the index does not exist', async () => { expect.assertions(1); const task = updateAliases(client, [ { @@ -792,7 +792,7 @@ describe('migration actions', () => { }, }, ]); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", "left": Object { @@ -814,7 +814,7 @@ describe('migration actions', () => { }, }, ]); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", "left": Object { @@ -836,7 +836,7 @@ describe('migration actions', () => { }, }, ]); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", "left": Object { @@ -856,7 +856,7 @@ describe('migration actions', () => { }, }, ]); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", "left": Object { @@ -868,7 +868,7 @@ describe('migration actions', () => { }); }); describe('remove_index', () => { - it('left index_not_found_exception if index does not exist', () => { + it('left index_not_found_exception if index does not exist', async () => { expect.assertions(1); const task = updateAliases(client, [ { @@ -877,7 +877,7 @@ describe('migration actions', () => { }, }, ]); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", "left": Object { @@ -887,7 +887,7 @@ describe('migration actions', () => { } `); }); - it('left remove_index_not_a_concrete_index when remove_index targets an alias', () => { + it('left remove_index_not_a_concrete_index when remove_index targets an alias', async () => { expect.assertions(1); const task = updateAliases(client, [ { @@ -896,7 +896,7 @@ describe('migration actions', () => { }, }, ]); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Left", "left": Object { @@ -944,7 +944,7 @@ describe('migration actions', () => { indexGreen = true; }, 10); - return createIndexPromise.then((res) => { + await createIndexPromise.then((res) => { // Assert that the promise didn't resolve before the index became green expect(indexGreen).toBe(true); expect(res).toMatchInlineSnapshot(` @@ -955,18 +955,18 @@ describe('migration actions', () => { `); }); }); - it('rejects when there is an unexpected error creating the index', () => { + it('rejects when there is an unexpected error creating the index', async () => { expect.assertions(1); // Creating an index with the same name as an existing alias to induce // failure - return expect( + await expect( createIndex(client, 'existing_index_2_alias', undefined as any)() ).rejects.toMatchInlineSnapshot(`[ResponseError: invalid_index_name_exception]`); }); }); describe('bulkOverwriteTransformedDocuments', () => { - it('resolves right when documents do not yet exist in the index', () => { + it('resolves right when documents do not yet exist in the index', async () => { expect.assertions(1); const newDocs = ([ { _source: { title: 'doc 5' } }, @@ -974,7 +974,7 @@ describe('migration actions', () => { { _source: { title: 'doc 7' } }, ] as unknown) as SavedObjectsRawDoc[]; const task = bulkOverwriteTransformedDocuments(client, 'existing_index_with_docs', newDocs); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Right", "right": "bulk_index_succeeded", @@ -993,21 +993,21 @@ describe('migration actions', () => { ...existingDocs, ({ _source: { title: 'doc 8' } } as unknown) as SavedObjectsRawDoc, ]); - return expect(task()).resolves.toMatchInlineSnapshot(` + await expect(task()).resolves.toMatchInlineSnapshot(` Object { "_tag": "Right", "right": "bulk_index_succeeded", } `); }); - it('rejects if there are errors', () => { + it('rejects if there are errors', async () => { expect.assertions(1); const newDocs = ([ { _source: { title: 'doc 5' } }, { _source: { title: 'doc 6' } }, { _source: { title: 'doc 7' } }, ] as unknown) as SavedObjectsRawDoc[]; - return expect( + await expect( bulkOverwriteTransformedDocuments(client, 'existing_index_with_write_block', newDocs)() ).rejects.toMatchObject(expect.anything()); }); From ee7e5f584316f483f41b18439fe2296b9209c1dd Mon Sep 17 00:00:00 2001 From: Rudolf Meijering Date: Fri, 18 Dec 2020 12:11:23 +0100 Subject: [PATCH 5/5] Await don't return expect promises for other tests too --- .../catch_retryable_es_client_errors.test.ts | 4 ++-- .../migrations_state_action_machine.test.ts | 16 ++++++++-------- .../migrationsv2/state_action_machine.test.ts | 12 ++++++------ 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/core/server/saved_objects/migrationsv2/actions/catch_retryable_es_client_errors.test.ts b/src/core/server/saved_objects/migrationsv2/actions/catch_retryable_es_client_errors.test.ts index 3186d7456383a..33787e3fce53a 100644 --- a/src/core/server/saved_objects/migrationsv2/actions/catch_retryable_es_client_errors.test.ts +++ b/src/core/server/saved_objects/migrationsv2/actions/catch_retryable_es_client_errors.test.ts @@ -22,14 +22,14 @@ import { elasticsearchClientMock } from '../../../elasticsearch/client/mocks'; import { catchRetryableEsClientErrors } from './catch_retryable_es_client_errors'; describe('catchRetryableEsClientErrors', () => { - it('rejects non-retryable response errors', () => { + it('rejects non-retryable response errors', async () => { const error = new esErrors.ResponseError( elasticsearchClientMock.createApiResponse({ body: { error: { type: 'cluster_block_exception' } }, statusCode: 400, }) ); - return expect(Promise.reject(error).catch(catchRetryableEsClientErrors)).rejects.toBe(error); + await expect(Promise.reject(error).catch(catchRetryableEsClientErrors)).rejects.toBe(error); }); describe('returns left retryable_es_client_error for', () => { it('NoLivingConnectionsError', async () => { diff --git a/src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.test.ts b/src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.test.ts index 6dbb986e868ee..2baf27e94edb3 100644 --- a/src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.test.ts +++ b/src/core/server/saved_objects/migrationsv2/migrations_state_action_machine.test.ts @@ -142,8 +142,8 @@ describe('migrationsStateActionMachine', () => { } `); }); - it('resolves when reaching the DONE state', () => { - return expect( + it('resolves when reaching the DONE state', async () => { + await expect( migrationStateActionMachine({ initialState, logger: mockLogger.get(), @@ -152,8 +152,8 @@ describe('migrationsStateActionMachine', () => { }) ).resolves.toEqual(expect.anything()); }); - it('resolves with migrated status if some sourceIndex in the DONE state', () => { - return expect( + it('resolves with migrated status if some sourceIndex in the DONE state', async () => { + await expect( migrationStateActionMachine({ initialState: { ...initialState, ...{ sourceIndex: Option.some('source-index') } }, logger: mockLogger.get(), @@ -162,8 +162,8 @@ describe('migrationsStateActionMachine', () => { }) ).resolves.toEqual(expect.objectContaining({ status: 'migrated' })); }); - it('resolves with patched status if none sourceIndex in the DONE state', () => { - return expect( + it('resolves with patched status if none sourceIndex in the DONE state', async () => { + await expect( migrationStateActionMachine({ initialState: { ...initialState, ...{ sourceIndex: Option.none } }, logger: mockLogger.get(), @@ -172,8 +172,8 @@ describe('migrationsStateActionMachine', () => { }) ).resolves.toEqual(expect.objectContaining({ status: 'patched' })); }); - it('rejects with error message when reaching the FATAL state', () => { - return expect( + it('rejects with error message when reaching the FATAL state', async () => { + await expect( migrationStateActionMachine({ initialState: { ...initialState, reason: 'the fatal reason' } as State, logger: mockLogger.get(), diff --git a/src/core/server/saved_objects/migrationsv2/state_action_machine.test.ts b/src/core/server/saved_objects/migrationsv2/state_action_machine.test.ts index 15dde10eb21ec..3760b92591346 100644 --- a/src/core/server/saved_objects/migrationsv2/state_action_machine.test.ts +++ b/src/core/server/saved_objects/migrationsv2/state_action_machine.test.ts @@ -86,14 +86,14 @@ describe('state action machine', () => { }); }); - test('rejects if an exception is throw from inside an action', () => { - return expect( + test('rejects if an exception is throw from inside an action', async () => { + await expect( stateActionMachine({ ...state, controlState: 'THROW' }, next, countUntilThree) ).rejects.toThrowErrorMatchingInlineSnapshot(`"Invalid control state"`); }); - test('resolve with the final state once all steps are completed', () => { - return expect(finalStateP).resolves.toMatchInlineSnapshot(` + test('resolve with the final state once all steps are completed', async () => { + await expect(finalStateP).resolves.toMatchInlineSnapshot(` Object { "controlState": "DONE", "count": 3, @@ -101,8 +101,8 @@ describe('state action machine', () => { `); }); - test("rejects if control state doesn't change after 50 steps", () => { - return expect( + test("rejects if control state doesn't change after 50 steps", async () => { + await expect( stateActionMachine(state, next, countUntilModel(51)) ).rejects.toThrowErrorMatchingInlineSnapshot( `"Control state didn't change after 50 steps aborting."`