Skip to content

Commit

Permalink
[Security Solutions] Hopefully reduces Cypress flake with timeline no…
Browse files Browse the repository at this point in the history
…tes (#99299)

## Summary

Hopefully reduces flake within cypress timeline notes.
* Removed additional notes about issues with the timelineid. I don't think that's a problem anymore now that I removed the assertion issue.
* Removed the pipe since the timeline click can cause multiple async URL actions to occur which will cause indeterminism.
* I added a visibility check to Cypress for the timeline link to be visible on the screen before we try to click on it which should hopefully give us enough breathing room for the click handler to be added.
* Added a query that returns no results as that can cause "UI blocking" since the UI loading for timeline takes a while. The "UI blocking" could be causing issues with clicks, but at the very least this speeds up the test and removes one more thing to worry about if this fails again.

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
  • Loading branch information
FrankHassanabad authored May 5, 2021
1 parent 366c16c commit 669be33
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { timeline } from '../../objects/timeline';
import { timelineNonValidQuery } from '../../objects/timeline';

import { NOTES_TEXT, NOTES_TEXT_AREA } from '../../screens/timeline';
import { createTimeline } from '../../tasks/api_calls/timelines';
Expand All @@ -19,14 +19,14 @@ import { waitForTimelinesPanelToBeLoaded } from '../../tasks/timelines';
import { TIMELINES_URL } from '../../urls/navigation';

describe('Timeline notes tab', () => {
let timelineId: string | undefined;
let timelineId: string;

before(() => {
cleanKibana();
loginAndWaitForPageWithoutDateRange(TIMELINES_URL);
waitForTimelinesPanelToBeLoaded();

createTimeline(timeline)
createTimeline(timelineNonValidQuery)
.then((response) => {
if (response.body.data.persistTimeline.timeline.savedObjectId == null) {
cy.log('"createTimeline" did not return expected response');
Expand All @@ -35,11 +35,8 @@ describe('Timeline notes tab', () => {
waitForTimelinesPanelToBeLoaded();
})
.then(() => {
// TODO: It would be great to add response validation to avoid such things like
// the bang below and to more easily understand where failures are coming from -
// client vs server side
openTimelineById(timelineId!).then(() => {
addNotesToTimeline(timeline.notes);
openTimelineById(timelineId).then(() => {
addNotesToTimeline(timelineNonValidQuery.notes);
});
});
});
Expand All @@ -49,7 +46,7 @@ describe('Timeline notes tab', () => {
});

it('should contain notes', () => {
cy.get(NOTES_TEXT).should('have.text', timeline.notes);
cy.get(NOTES_TEXT).should('have.text', timelineNonValidQuery.notes);
});

it('should render mockdown', () => {
Expand Down
10 changes: 10 additions & 0 deletions x-pack/plugins/security_solution/cypress/objects/timeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ export const timeline: CompleteTimeline = {
filter,
};

/**
* Timeline query that finds no valid data to cut down on test failures
* or other issues for when we want to test one specific thing and not also
* test the queries happening
*/
export const timelineNonValidQuery: CompleteTimeline = {
...timeline,
query: 'query_to_intentionally_find_nothing: *',
};

export const caseTimeline: Timeline = {
title: 'SIEM test',
description: 'description',
Expand Down
17 changes: 5 additions & 12 deletions x-pack/plugins/security_solution/cypress/tasks/timeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,21 +234,14 @@ export const openTimelineTemplateFromSettings = (id: string) => {
};

export const openTimelineById = (timelineId: string): Cypress.Chainable<JQuery<HTMLElement>> => {
// Why are we checking for null if it is typed to 'string'? We don't currently validate the timeline response
// so technically we cannot guarantee that we will have the id. Changing the type to 'string | null' results in
// a lot of other changes being needed that would be best as part of a cleanup. Added a log, to give a dev a clue
// as to whether it's failing client or server side.
if (timelineId == null) {
// Log out if for some reason this happens to be null just in case for our tests we experience
// value of null. Some tests return an "any" which is why this could happen.
cy.log('"timelineId" is null or undefined');
}

cy.root()
.pipe(($el) => {
$el.find(TIMELINE_TITLE_BY_ID(timelineId)).trigger('click');
return $el.find(QUERY_TAB_BUTTON).find('.euiBadge');
})
.should('be.visible');
return cy.root().find(TIMELINE_TITLE_BY_ID(timelineId));
// We avoid use cypress.pipe() here and multiple clicks because each of these clicks
// can result in a new URL async operation occurring and then we get indeterminism as the URL loads multiple times.
return cy.get(TIMELINE_TITLE_BY_ID(timelineId)).should('be.visible').click({ force: true });
};

export const pinFirstEvent = () => {
Expand Down

0 comments on commit 669be33

Please sign in to comment.