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

[ResponseOps] can we remove some tests after refactoring task runner? #133831

Closed
Tracked by #124206
pmuellr opened this issue Jun 7, 2022 · 2 comments · Fixed by #140127
Closed
Tracked by #124206

[ResponseOps] can we remove some tests after refactoring task runner? #133831

pmuellr opened this issue Jun 7, 2022 · 2 comments · Fixed by #140127
Assignees
Labels
blocked Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@pmuellr
Copy link
Member

pmuellr commented Jun 7, 2022

meta issue: #124206

As part of the review for one of the items from the meta issue - Refactor alerting task runner - combine loadRuleAttributesAndRun() and validateAndExecuteRule(), a question was raised if we could remove some of the task runner tests, since we now had some tests that are now testing some of the "inner" functions that we couldn't test before. Comment thread here: #132079 (comment)

At first glance, I don't think we can just remove the suggested tests, as the task runner API implementation functions have other side effects that the tests check, that the new tests can't check.

But I'm thinking after more refactoring, we may be in a position to remove some of these - or similar tests. I think we should wait till we're done with most of the refactoring before looking into this.

blocked on #131548

meta issue: #124206

@pmuellr pmuellr added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework labels Jun 7, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@mikecote
Copy link
Contributor

mikecote commented Jun 9, 2022

adding blocked label, we will unblock once all the refactoring issues are complete so we can review the remaining tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Feature:Alerting/RulesFramework Issues related to the Alerting Rules Framework Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants