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 repository api unit tests into individual tests #173218

Merged
merged 4 commits into from
Dec 13, 2023

Conversation

TinaHeiligers
Copy link
Contributor

Summary

Structural cleanup of the SavedObjectsRepository, continued from where #157154 left off.
This PR holds changes to repository.test.ts, and splits the api unit tests into dedicated test files next to the api implementation.

Tests

This PR does not alter test coverage, its primary aim is to improve DevEx when it comes to SOR api unit testing. Hopefully, it'll make life easier for everyone touching the code.

@TinaHeiligers TinaHeiligers added chore Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Dec 12, 2023
@TinaHeiligers TinaHeiligers marked this pull request as ready for review December 12, 2023 21:57
@TinaHeiligers TinaHeiligers requested a review from a team as a code owner December 12, 2023 21:57
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@TinaHeiligers
Copy link
Contributor Author

@elasticmachine merge upstream

@TinaHeiligers
Copy link
Contributor Author

FYI @elastic/kibana-security

Copy link
Contributor

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

Great work, this will help significantly for maintainability!

Note, I didn't check the full content of each test, as I assume it was mostly moving things around, but the files I checked made sense, so does the split.

Comment on lines 54 to 55
// BEWARE: The SavedObjectClient depends on the implementation details of the SavedObjectsRepository
// so any breaking changes to this repository are considered breaking changes to the SavedObjectsClient.
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: no really sure why that comment was on the SOR test file in the first place, seems like something that was very old. I think we should remove it from all those files now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done: 8276f13

@TinaHeiligers TinaHeiligers enabled auto-merge (squash) December 13, 2023 15:48
@TinaHeiligers
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Serverless Rule Management - Security Solution Cypress Tests #2 / Related integrations related Integrations Advanced Setting is disabled rules management table should not display a badge with the installed integrations should not display a badge with the installed integrations

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
@kbn/core-saved-objects-api-server-internal 4 11 +7

Total ESLint disabled count

id before after diff
@kbn/core-saved-objects-api-server-internal 10 17 +7

History

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

@TinaHeiligers TinaHeiligers merged commit 49267f3 into elastic:main Dec 13, 2023
38 checks passed
@TinaHeiligers TinaHeiligers self-assigned this Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting chore release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.12.0 v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants