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

[Files] Tests for createEsFileClient() usage when indexAsAlias option and minor refactor #153815

Conversation

paul-tavares
Copy link
Contributor

Summary

This PR builds on top of #153342 and:

  • adds test for the indexAsAlias option that was added to createEsFileClient()
  • removes fetchDoc() utility (not needed)

@paul-tavares paul-tavares added release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.8.0 labels Mar 27, 2023
@paul-tavares paul-tavares self-assigned this Mar 27, 2023
it('should be an empty stream on empty response', async () => {
client.get.mockResponseOnce(toReadable());
const onData = jest.fn();
describe('with `indexIsAlias` set to `false`', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From here down, everything is the same as before. All I did was wrap the test cases in a describe() whose title is more descriptive of that block of tests.

Here is a screen capture from my IDE:

image

const { esClient, index, indexIsAlias } = this;
let doc: FileDocument<M> | undefined;

if (indexIsAlias) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed fetchDoc() utility since it was only being used here and moved the implementation here.


if (!doc) {
this.logger.error(`File with id "${id}" not found`);
this.logger.error(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jloleysens - I think this addresses your comment in the prior PR. If .search() does not find the doc, then it will throw an error here.

@paul-tavares paul-tavares marked this pull request as ready for review March 28, 2023 13:49
@paul-tavares paul-tavares requested a review from a team as a code owner March 28, 2023 13:49
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-defend-workflows (Team:Defend Workflows)

Copy link
Contributor

@vadimkibana vadimkibana left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 433 436 +3

Total ESLint disabled count

id before after diff
securitySolution 513 516 +3

History

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

cc @paul-tavares

@paul-tavares paul-tavares merged commit 8c6e58c into elastic:main Mar 28, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Mar 28, 2023
@paul-tavares paul-tavares deleted the task/olm-files-plugin-tests-for-index-as-alias-change branch March 28, 2023 18:28
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 OLM Sprint release_note:skip Skip the PR/issue when compiling release notes Team:Defend Workflows “EDR Workflows” sub-team of Security Solution v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants