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

[AO] Unskip the custom threshold rule executor unit test #167120

Merged
merged 7 commits into from
Sep 28, 2023

Conversation

maryam-saeidi
Copy link
Member

@maryam-saeidi maryam-saeidi commented Sep 25, 2023

Fixes #162299
Fixes #162540

Summary

This PR unskips the custom threshold rule executor unit test and removes the warning implementation from the BE.

🧪 How to test

  • Create a custom threshold rule, it should work as before. (Warning implementation logic was already removed from FE; this PR only removes the BE implementation.)

@maryam-saeidi maryam-saeidi self-assigned this Sep 25, 2023
@maryam-saeidi maryam-saeidi added the release_note:skip Skip the PR/issue when compiling release notes label Sep 25, 2023
@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #56 / Profiling API tests has_setup.spec.ts cloud Loading profiling data Profiling status check APM integration is not installed Admin user has been set up
  • [job] [logs] FTR Configs #14 / serverless common UI Index Management Index Templates "before all" hook for "renders the index templates tab"

Metrics [docs]

✅ unchanged

History

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

cc @maryam-saeidi

@maryam-saeidi maryam-saeidi marked this pull request as ready for review September 26, 2023 11:48
@maryam-saeidi maryam-saeidi requested a review from a team as a code owner September 26, 2023 11:48
@mgiota mgiota self-requested a review September 27, 2023 12:16
@@ -123,8 +133,7 @@ const setEvaluationResults = (response: Array<Record<string, Evaluation>>) => {
jest.requireMock('./lib/evaluate_rule').evaluateRule.mockImplementation(() => response);
};

// FAILING: https://github.com/elastic/kibana/issues/155534
describe.skip('The metric threshold alert type', () => {
describe('The metric threshold alert type', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to this PR, but is it easy to to change threshold alert type to threshold rule type?

@mgiota
Copy link
Contributor

mgiota commented Sep 27, 2023

@maryam-saeidi Code review LGTM! I need to test it locally as well.

What do BE and FE mean?

@mgiota
Copy link
Contributor

mgiota commented Sep 28, 2023

I created a custom threshold rule and everything works as expected!

@mgiota mgiota self-requested a review September 28, 2023 05:56
@maryam-saeidi
Copy link
Member Author

What do BE and FE mean?

@mgiota I mean, in the rule flyout (front-end), we already removed the warning threshold, so it is only possible to set the alert threshold. By back-end, I mean the rule execution logic in the server folder.

@maryam-saeidi maryam-saeidi merged commit 6ab0c68 into elastic:main Sep 28, 2023
@kibanamachine kibanamachine added v8.11.0 backport:skip This commit does not require backporting labels Sep 28, 2023
@maryam-saeidi maryam-saeidi deleted the 162299-fix-test branch September 28, 2023 12:09
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 release_note:skip Skip the PR/issue when compiling release notes v8.11.0
Projects
None yet
5 participants