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

Failing test: Jest Tests.x-pack/plugins/security_solution/public/detection_engine/rule_gaps/components/manual_rule_run - ManualRuleRunModal should render confirmation button disabled if selected end date is in future (#186189) #186296

Merged
merged 5 commits into from
Jun 21, 2024
Merged

Failing test: Jest Tests.x-pack/plugins/security_solution/public/detection_engine/rule_gaps/components/manual_rule_run - ManualRuleRunModal should render confirmation button disabled if selected end date is in future (#186189) #186296

merged 5 commits into from
Jun 21, 2024

Conversation

e40pud
Copy link
Contributor

@e40pud e40pud commented Jun 17, 2024 •
edited
Loading

Summary

Related tickets #186189 and #186187, and #186188

Attempt to fix failing manual rule run tests added in this PR #184500

@e40pud
Failing test: Jest Tests.x-pack/plugins/security_solution/public/dete…
a251e9e 
…ction_engine/rule_gaps/components/manual_rule_run - ManualRuleRunModal should render confirmation button disabled if selected end date is in future (elastic#186189)
@e40pud e40pud added release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Engine Security Solution Detection Engine Area labels Jun 17, 2024
@e40pud e40pud self-assigned this Jun 17, 2024
@e40pud e40pud requested a review from a team as a code owner June 17, 2024 14:03
@e40pud e40pud requested a review from rylnd June 17, 2024 14:03
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-engine (Team:Detection Engine)

@e40pud
Copy link
Contributor Author

e40pud commented Jun 18, 2024

@elasticmachine merge upstream

rylnd
rylnd approved these changes Jun 20, 2024
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.

@e40pud these changes look reasonable, but I'm not seeing any explanation as to what the problem was (that caused the NoMethodError for scrollIntoView), or how these changes address that problem.

If you could provide some of that context, that would help both review and posterity. Thanks!

...ecurity_solution/public/detection_engine/rule_gaps/components/manual_rule_run/index.test.tsx Outdated Show resolved Hide resolved
@e40pud
Copy link
Contributor Author

e40pud commented Jun 21, 2024

@elasticmachine merge upstream

@e40pud
Copy link
Contributor Author

e40pud commented Jun 21, 2024 •
edited
Loading

@e40pud these changes look reasonable, but I'm not seeing any explanation as to what the problem was (that caused the NoMethodError for scrollIntoView), or how these changes address that problem.

If you could provide some of that context, that would help both review and posterity. Thanks!

@rylnd I was never able to reproduce the issue locally and thus decided to restructure tests and used methods. My initial thoughts were that the cause is the getByText method which calls scrollIntoView method for whatever reasons.

Now, I found this discussion https://stackoverflow.com/a/53294906 and the link to the issue jsdom/jsdom#1695. Updated tests and added proposed solution. I still would like to keep my changes in this PR besides the actual fix - I feel that it looks more clear and easier to grasp.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

cc @e40pud

@rylnd
Copy link
Contributor

rylnd commented Jun 21, 2024

That's great info, thanks @e40pud!

@e40pud e40pud merged commit 6d94d89 into elastic:main Jun 21, 2024
36 checks passed
@kibanamachine kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels Jun 21, 2024
This was referenced Jun 21, 2024
Closed
Closed
Closed
bhapas pushed a commit to bhapas/kibana that referenced this pull request Jun 24, 2024
@e40pud @kibanamachine @bhapas
Failing test: Jest Tests.x-pack/plugins/security_solution/public/dete…
849fcb4 
…ction_engine/rule_gaps/components/manual_rule_run - ManualRuleRunModal should render confirmation button disabled if selected end date is in future (elastic#186189) (elastic#186296)

## Summary

Related tickets elastic#186189 and
elastic#186187, and
elastic#186188

Attempt to fix failing manual rule run tests added in this PR
elastic#184500

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
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

@e40pud e40pud

Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Detection Engine Security Solution Detection Engine Area Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.15.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@e40pud @elasticmachine @kibana-ci @rylnd @kibanamachine