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.

Sign up for GitHub

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

Jump to bottom

Risk score test unskip and clear transform #168469

Merged
merged 11 commits into from
Oct 13, 2023
Merged

Risk score test unskip and clear transform #168469

merged 11 commits into from
Oct 13, 2023

Conversation

nkhristinin
Copy link
Contributor

@nkhristinin nkhristinin commented Oct 10, 2023 •
edited
Loading

Risk score test unskip and clear transform

Run 50 times and 100 times on flaky test runner and it was green

@nkhristinin
Copy link
Contributor Author

@elasticmachine merge upstream

@nkhristinin nkhristinin changed the title enable test and add logs Risk score test unskipp and clear transform Oct 12, 2023
@nkhristinin nkhristinin marked this pull request as ready for review October 12, 2023 15:20
@nkhristinin nkhristinin requested a review from a team as a code owner October 12, 2023 15:20
@nkhristinin
Copy link
Contributor Author

@elasticmachine merge upstream

@nkhristinin nkhristinin added the release_note:skip Skip the PR/issue when compiling release notes label Oct 12, 2023
@nkhristinin nkhristinin changed the title Risk score test unskipp and clear transform Risk score test unskip and clear transform Oct 12, 2023
rylnd
rylnd reviewed Oct 12, 2023
View reviewed changes
Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

As I mentioned in the comments, I think we should abstract those teardown functions into a named helper like cleanRiskEngine and just call that before/after our tests.

I'm not sure if you intend to merge this PR, or open a new one; let me know and I can approve, though.

...ction_engine_api_integration/security_and_spaces/group10/risk_engine/init_and_status_apis.ts
Comment on lines +35 to +42
await cleanRiskEngineConfig({ kibanaServer });
await deleteRiskEngineTask({ es, log });
await deleteAllRiskScores(log, es);
await clearTransforms({
es,
log,
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

We should abstract this to a function named cleanRiskEngine({ kibanaServer, es, log })

...ction_engine_api_integration/security_and_spaces/group10/risk_engine/init_and_status_apis.ts
@@ -45,10 +57,11 @@ export default ({ getService }: FtrProviderContext) => {
supertest,
log,
});
await deleteRiskEngineTask({ es, log });
});

// FLAKY: https://github.com/elastic/kibana/issues/168376
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget to delete these comments

x-pack/test/detection_engine_api_integration/security_and_spaces/group10/risk_engine/utils.ts Outdated
es.indices.deleteDataStream({ name: [`risk-score.risk-score-${namespace}`] }),
es.indices.delete({
index: [`risk-score.risk-score-latest-${namespace}`],
}),
]);
log.info(result);
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this just for debugging, or did you intend to leave this here?

rylnd
rylnd approved these changes Oct 12, 2023
View reviewed changes
Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

I see that you triggered a build shortly before I reviewed; since this is potentially blocking folks, let's just merge this with the outdated comments and without the refactor; we can do those in a followup 👍

@nkhristinin
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Investigations - Security Solution Cypress Tests #2 / Discover Timeline State Integration save/restore should save/restore discover dataview/timerange/filter/query/columns when timeline is opened via url should save/restore discover dataview/timerange/filter/query/columns when timeline is opened via url
  • [job] [logs] Investigations - Security Solution Cypress Tests #2 / Discover Timeline State Integration saved search should rename the saved search on timeline rename should rename the saved search on timeline rename

Metrics [docs]

✅ unchanged

History

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

@nkhristinin nkhristinin merged commit caeae04 into elastic:main Oct 13, 2023
@kibanamachine kibanamachine added v8.12.0 backport:skip This commit does not require backporting labels Oct 13, 2023
@rylnd
Copy link
Contributor

rylnd commented Oct 13, 2023

@nkhristinin should this be backported to 8.11 to help with stability there?

This was referenced Oct 13, 2023
Closed
Closed
Merged
@rylnd
Copy link
Contributor

rylnd commented Oct 13, 2023

💚 All backports created successfully

Status Branch Result
✅ 8.11

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

rylnd pushed a commit to rylnd/kibana that referenced this pull request Oct 13, 2023
@nkhristinin
Risk score test unskip and clear transform (elastic#168469)
4aed17e 
# Risk score test unskip and clear transform

Run 50 times and 100 times on flaky test runner and it was green

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Ryland Herrick <[email protected]>
(cherry picked from commit caeae04)
This was referenced Oct 13, 2023
Closed
Merged
rylnd added a commit that referenced this pull request Oct 13, 2023
@rylnd @nkhristinin
[8.11] Risk score test unskip and clear transform (#168469) (#168867)
aebe730 
# Backport

This will backport the following commits from `main` to `8.11`:
- [Risk score test unskip and clear transform
(#168469)](#168469)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Khristinin
Nikita","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-10-13T06:47:36Z","message":"Risk
score test unskip and clear transform (#168469)\n\n# Risk score test
unskip and clear transform \r\n\r\n\r\nRun 50 times and 100 times on
flaky test runner and it was
green\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>\r\nCo-authored-by:
Ryland Herrick
<[email protected]>","sha":"caeae04fac6602d54a543ececa10fbd7c91ddd7c","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:skip","v8.12.0"],"number":168469,"url":"https://github.com/elastic/kibana/pull/168469","mergeCommit":{"message":"Risk
score test unskip and clear transform (#168469)\n\n# Risk score test
unskip and clear transform \r\n\r\n\r\nRun 50 times and 100 times on
flaky test runner and it was
green\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>\r\nCo-authored-by:
Ryland Herrick
<[email protected]>","sha":"caeae04fac6602d54a543ececa10fbd7c91ddd7c"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/168469","number":168469,"mergeCommit":{"message":"Risk
score test unskip and clear transform (#168469)\n\n# Risk score test
unskip and clear transform \r\n\r\n\r\nRun 50 times and 100 times on
flaky test runner and it was
green\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<[email protected]>\r\nCo-authored-by:
Ryland Herrick
<[email protected]>","sha":"caeae04fac6602d54a543ececa10fbd7c91ddd7c"}}]}]
BACKPORT-->

Co-authored-by: Khristinin Nikita <[email protected]>
dej611 pushed a commit to dej611/kibana that referenced this pull request Oct 17, 2023
@nkhristinin @kibanamachine
@rylnd @dej611
Risk score test unskip and clear transform (elastic#168469)
68e374f 
# Risk score test unskip and clear transform 


Run 50 times and 100 times on flaky test runner and it was green

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Ryland Herrick <[email protected]>
@rylnd rylnd mentioned this pull request Oct 17, 2023
Closed
rylnd added a commit that referenced this pull request Nov 8, 2023
@rylnd
[Entity Analytics]Integration test cleanup (#168869)
ed9956d 
## Summary

Followup to some recent work to improve test reliability (namely
#168469):

* Consolidates teardown of risk engine artifacts into a single helper
function
* Removes references to flaky test issues in comments
* Log utils errors at the warning level
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rylnd rylnd rylnd approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes v8.11.0 v8.12.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nkhristinin @kibana-ci @rylnd @kibanamachine