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

[Security Solution] Unskip cypress tests #86653

Merged
merged 19 commits into from
Jan 8, 2021

Conversation

angorayc
Copy link
Contributor

@angorayc angorayc commented Dec 21, 2020

Summary

This PR is to unskip data provider's Cypress test.

  1. The flakiness in timeline_data_provider.spec may cause by Cypress tries to retrieve the close button before timeline is opened, therefor removing the 1 sec waiting time while opening timeline.
    Failing test: "after each" hook for "renders the data provider of a host dragged from the All Hosts widget on the hosts page" - timeline data providers "after each" hook for "renders the data provider of a host dragged from the All Hosts widget on the hosts page" #85098
    Failing test: renders the data provider of a host dragged from the All Hosts widget on the hosts page - timeline data providers renders the data provider of a host dragged from the All Hosts widget on the hosts page #62060

  2. Update the data-test-subj for filters of search bar:
    x-pack/plugins/security_solution/cypress/screens/timeline.ts Line 109
    It added an extra whitespace in the data-test-subj and failed the cypress test, which looks like

data-test-subj="filter filter-enabled filter-key-${filter.field} filter-value-${filter.value} filter-unpinned "

I had a quick fix by adding an extra whitespace in x-pack/plugins/security_solution/cypress/screens/timeline.ts Line 111 in my previous PR to adapt to it, but I don't think that's a good solution.

#79389

@angorayc angorayc requested review from a team as code owners December 21, 2020 15:30
@angorayc angorayc added release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0 Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting Security Solution Threat Hunting Team labels Dec 21, 2020
@@ -113,6 +113,7 @@ export const checkIdToggleField = () => {
};

export const closeTimeline = () => {
cy.wait(1000);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we could find another way instead of .wait()?

@angorayc angorayc requested a review from a team as a code owner December 21, 2020 16:27
@angorayc angorayc changed the title unskip data provider cypress test [Security Solution] Unskip cypress tests Dec 22, 2020
@@ -166,7 +166,8 @@ export const pinFirstEvent = () => {

export const populateTimeline = () => {
executeTimelineKQL(hostExistsQuery);
cy.get(SERVER_SIDE_EVENT_COUNT)
cy.get(QUERY_TAB_EVENTS_FOOTER)
.find(SERVER_SIDE_EVENT_COUNT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want to find SERVER_SIDE_EVENT_COUNT inside QUERY_TAB_EVENTS_FOOTER you should use https://docs.cypress.io/api/commands/within.html#Syntax

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.cypress.io/api/commands/find.html#Syntax looks good to me too.
I am using find as the looks flat, however within seems always follow by a callback.

@@ -145,7 +145,14 @@ export function FilterItem(props: Props) {
const dataTestSubjNegated = filter.meta.negate ? 'filter-negated' : '';
const dataTestSubjDisabled = `filter-${isDisabled(labelConfig) ? 'disabled' : 'enabled'}`;
const dataTestSubjPinned = `filter-${isFilterPinned(filter) ? 'pinned' : 'unpinned'}`;
return `filter ${dataTestSubjDisabled} ${dataTestSubjKey} ${dataTestSubjValue} ${dataTestSubjPinned} ${dataTestSubjNegated}`;
return classNames(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we limit the scope of the changes to the Security solution plugin only?

Copy link
Contributor Author

@angorayc angorayc Dec 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I understand, but it added an extra whitespace in the data-test-subj and failed the cypress test.
I had a quick fix by adding an extra whitespace in x-pack/plugins/security_solution/cypress/screens/timeline.ts Line 111 in my previous PR to adapt to it, but I don't think that's a good solution.
#85871

@angorayc
Copy link
Contributor Author

@elasticmachine merge upstream

@angorayc
Copy link
Contributor Author

@elasticmachine merge upstream

@angorayc
Copy link
Contributor Author

@elasticmachine merge upstream

@angorayc
Copy link
Contributor Author

@elasticmachine merge upstream

@angorayc
Copy link
Contributor Author

@elasticmachine merge upstream

@angorayc
Copy link
Contributor Author

angorayc commented Jan 4, 2021

@elasticmachine merge upstream

Copy link
Contributor

@lizozom lizozom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct!

Copy link
Member

@MadameSheema MadameSheema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of thanks for taking the time to fix this :)

@angorayc
Copy link
Contributor Author

angorayc commented Jan 6, 2021

@elasticmachine merge upstream

@angorayc
Copy link
Contributor Author

angorayc commented Jan 7, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
data 270.9KB 270.9KB -34.0B
securitySolution 8.5MB 8.5MB +369.0B
total +335.0B

History

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

@angorayc angorayc merged commit 4986bea into elastic:master Jan 8, 2021
angorayc added a commit to angorayc/kibana that referenced this pull request Jan 8, 2021
* unskip data provider cypress test

* remove extra whitespace for filter classes

* remove cy.wait

* update functional test

* fix cypress and add tabType to dataTestSubj

* fix cypress test

* revert createNewTimeline task

* fix dependency

* fix line error

Co-authored-by: Kibana Machine <[email protected]>
angorayc added a commit to angorayc/kibana that referenced this pull request Jan 8, 2021
* unskip data provider cypress test

* remove extra whitespace for filter classes

* remove cy.wait

* update functional test

* fix cypress and add tabType to dataTestSubj

* fix cypress test

* revert createNewTimeline task

* fix dependency

* fix line error

Co-authored-by: Kibana Machine <[email protected]>
angorayc added a commit that referenced this pull request Jan 8, 2021
* unskip data provider cypress test

* remove extra whitespace for filter classes

* remove cy.wait

* update functional test

* fix cypress and add tabType to dataTestSubj

* fix cypress test

* revert createNewTimeline task

* fix dependency

* fix line error

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
angorayc added a commit that referenced this pull request Jan 8, 2021
* unskip data provider cypress test

* remove extra whitespace for filter classes

* remove cy.wait

* update functional test

* fix cypress and add tabType to dataTestSubj

* fix cypress test

* revert createNewTimeline task

* fix dependency

* fix line error

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 11, 2021
* master:
  [APM] Define placement “Right” to offset tooltip (elastic#87729)
  Fix UI glitch on SOM delete confirmation modal (elastic#87623)
  Remove src/plugins/vis_default_editor -> src/plugins/visualizations cyclic dependencies (elastic#86988)
  [Timelion] Fix tests flakiness on suggestion click (elastic#87273)
  [Uptime] Fix/details page tabs (elastic#86296)
  [ML] Fix earliest and latest texts for date fields (elastic#87482)
  chore(NA): move grokdebugger plugin test fixtures out of __tests__ folder (elastic#87765)
  [Security Solution] Refactor Cypress scenarios to use internal contex… (elastic#86609)
  [Security Solution] Unskip cypress tests (elastic#86653)
@oatkiller oatkiller added the Feature:Timeline Security Solution Timeline feature label Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Timeline Security Solution Timeline feature release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting Security Solution Threat Hunting Team v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants