[SecuritySolution] Fix flaky timeline template creation tests #173206
Conversation
janmonschke
added
release_note:skip
| describe('Timeline Templates', { tags: ['@ess', '@serverless'] }, () => { | ||
| beforeEach(() => { | ||
| login(); | ||
| deleteTimelines(); | ||
| cy.intercept('PATCH', '/api/timeline').as('timeline'); | ||
| }); | ||
|
|
||
| it.skip('Creates a timeline template', () => { | ||
| it('Creates a timeline template', () => { | ||
| createTimeline(getTimeline()); |
There was a problem hiding this comment.
Why do we need to create a regular timeline before creating a template?
There was a problem hiding this comment.
Nice catch. I must've copy pasted it from the other test. Fixed in 2f23f20
| describe('Timeline Templates', { tags: ['@ess', '@serverless'] }, () => { | ||
| beforeEach(() => { | ||
| login(); | ||
| deleteTimelines(); | ||
| cy.intercept('PATCH', '/api/timeline').as('timeline'); | ||
| }); | ||
|
|
||
| it.skip('Creates a timeline template', () => { | ||
| it('Creates a timeline template', () => { |
There was a problem hiding this comment.
I get the purpose of this test to kind of test the majority of the timeline template functionality, but do you think it's worth breaking this apart to
describe('Timeline templates', { tags: ['@ess', '@serverless'] }, () => {
test('should be able to create a template', () => {});
test('should be able to be save a template', () => { });
test('should be able to update a template', () => {});
});
or something similarly
There was a problem hiding this comment.
I would rather test this in one test so that we don't create dependencies between tests or duplicate a lot of the creation code.
To make more sense of the logic and in order to make it easier to debug on the CI, I added a couple of cy.logs. Let me know if that's better. da5e697
There was a problem hiding this comment.
That makes sense and I understand why. In this scenario I would have suggested putting the creation in a beforeAll and then still breaking the tests apart, but with cypress not running the beforeAll on retries could make those flaky and not make true independent tests. All that being said, this may be the best compromise.
|
Do you mind doing flaky test run for serverless as well since these tests run on serverless, thanks! |
@michaelolo24 The flaky test runner was run on both ESS and severless: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/4520 |
Missed that, thanks! |
| cy.get(TIMELINE_TITLE).should('have.text', getTimeline().title); | ||
| cy.get(TIMELINE_DESCRIPTION).should('have.text', getTimeline().description); | ||
| cy.get(TIMELINE_QUERY).should('have.text', getTimeline().query); | ||
| cy.get(TIMELINE_QUERY).should('contain.text', getTimeline().query); | ||
| // Comments this assertion until we agreed what to do with the filters. | ||
| // cy.get(TIMELINE_FILTER(timeline.filter)).should('exist'); |
There was a problem hiding this comment.
Should we remove this comment?
There was a problem hiding this comment.
Yeah, let's do that :) c7c270d
|
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @janmonschke |
kibanamachine
added
v8.13.0
backport:skip
Summary
Fixes #165738
Fixes the flaky timeline template tests. Some of the test code wasn't valid anymore (e.g. notes in timeline templates), so I removed those and cleaned up the now unnecessary code.
Flaky test runner results (flaky runner results before requested changes)