-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[CTI] bring back skipped cypress tests #108978
Conversation
Pinging @elastic/security-solution (Team: SecuritySolution) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🪄 Thanks for bringing these back to life! 🪄
I just had one question about the addition of a span
into our markup, but it's probably fine.
CI will determine whether this is 🟢 or not; approving to defer.
@@ -37,8 +37,7 @@ import { | |||
import { ALERTS_URL } from '../../urls/navigation'; | |||
import { addsFieldsToTimeline } from '../../tasks/rule_details'; | |||
|
|||
// TODO: Doesn't look like the roll over is happening for these tests. 'indicator' is still referenced in the fields browser | |||
describe.skip('CTI Enrichment', () => { | |||
describe('CTI Enrichment', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😉
@@ -202,11 +202,11 @@ const FormattedFieldValueComponent: React.FC<{ | |||
</EuiFlexGroup> | |||
} | |||
> | |||
<>{value}</> | |||
<span data-test-subj={`formatted-field-${fieldName}`}>{value}</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you verify that this has no impact to layout?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looked good in the cypress view
@elasticmachine merge upstream |
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ecezalp :)
@elasticmachine merge upstream |
Looks like the previous failing test it is related wit this: #109038 so everything should be working fine now :) |
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/ml/embeddables/anomaly_charts_dashboard_embeddables·ts.machine learning embeddables anomaly charts with multi metric job can open job selection flyoutStandard Out
Stack Trace
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @ecezalp |
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
Summary
unskips cti_enrichments.spec.ts
unskips indicator_match_rule.spec.ts
Checklist
Delete any items that are not applicable to this PR.