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

Splits migrationsv2 actions and unit tests into separate files #101200

Conversation

TinaHeiligers
Copy link
Contributor

@TinaHeiligers TinaHeiligers commented Jun 2, 2021

Summary

Resolves #99479

The migrations v2 implementation's actions are split into one file per action to improve navigating the code.
This PR specifically splits the src/core/server/saved_objects/migrationsv2/actions/index.ts and src/core/server/saved_objects/migrationsv2/actions/index.test.ts files into one-per-action type.

An additional file was created for the CONSTANTS previously declared in index.ts.

Checklist

For maintainers

@TinaHeiligers TinaHeiligers added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.14.0 project:ResilientSavedObjectMigrations Reduce Kibana upgrade failures by making saved object migrations more resilient labels Jun 2, 2021
@TinaHeiligers TinaHeiligers marked this pull request as ready for review June 2, 2021 20:21
@TinaHeiligers TinaHeiligers requested a review from a team as a code owner June 2, 2021 20:21
@@ -19,7 +19,7 @@ async function removeLogFile() {
// ignore errors if it doesn't exist
await asyncUnlink(logFilePath).catch(() => void 0);
}

// TINA TODO: figure out why this test is failing
Copy link
Contributor

@mshustov mshustov Jun 3, 2021

Choose a reason for hiding this comment

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

😄
is it still falling?

* Side Public License, v 1.
*/

import { catchRetryableEsClientErrors } from './catch_retryable_es_client_errors';
Copy link
Contributor

Choose a reason for hiding this comment

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

the current PR shows that we don't have good test coverage in the unit tests. we also have some integration tests for actions https://github.com/elastic/kibana/blob/5ff9382330d38763703fd05e308742495190c480/src/core/server/saved_objects/migrationsv2/integration_tests/actions.test.ts

Since we are not splitting the tests by action type (as this would significantly slow down their execution) should we at least move them closer to this /actions folder?

@TinaHeiligers
Copy link
Contributor Author

should we at least move them closer to this /actions folder?

Makes sense. Not within the scope of the original issue but we could do it anyway.

@TinaHeiligers TinaHeiligers added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 3, 2021
@TinaHeiligers TinaHeiligers enabled auto-merge (squash) June 3, 2021 13:35
@TinaHeiligers
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@TinaHeiligers TinaHeiligers merged commit 843a81e into elastic:master Jun 3, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 3, 2021
…ic#101200)

* Splits migrationsv2 actions and unit tests into separate files

* Moves actions integration tests
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 3, 2021
…) (#101304)

* Splits migrationsv2 actions and unit tests into separate files

* Moves actions integration tests

Co-authored-by: Christiane (Tina) Heiligers <christiane.heiligers@elastic.co>
@TinaHeiligers TinaHeiligers deleted the so-migrations/split-actions-into-files branch June 4, 2021 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed project:ResilientSavedObjectMigrations Reduce Kibana upgrade failures by making saved object migrations more resilient release_note:skip Skip the PR/issue when compiling release notes v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Break up Migrations v2 Implementation into multiple files
3 participants