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

[Security Solution] Unskip timeline creation Cypress test #85871

Merged
merged 2 commits into from Dec 15, 2020
Merged

[Security Solution] Unskip timeline creation Cypress test #85871

merged 2 commits into from Dec 15, 2020

Conversation

angorayc
Copy link
Contributor

Summary

#79389

@angorayc angorayc added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.11.0 Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. labels Dec 14, 2020
@angorayc angorayc requested review from a team as code owners December 14, 2020 21:30
michaelolo24
michaelolo24 reviewed Dec 14, 2020
View reviewed changes
x-pack/plugins/security_solution/cypress/screens/timeline.ts
@@ -103,7 +105,7 @@ export const TIMELINE_FIELDS_BUTTON =
'[data-test-subj="timeline"] [data-test-subj="show-field-browser"]';

export const TIMELINE_FILTER = (filter: TimelineFilter) => {
return `[data-test-subj="filter filter-enabled filter-key-${filter.field} filter-value-${filter.value} filter-unpinned"]`;
return `[data-test-subj="filter filter-enabled filter-key-${filter.field} filter-value-${filter.value} filter-unpinned "]`;
Copy link
Contributor

Choose a reason for hiding this comment

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

was this extra space on the end here meaningful in some way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As it is how it looks atm, and the component is not made by us. Update it here to fit current case and get the right element.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good. Could you add a comment to that effect? Someone's editor may accidentally fix that on save or something and recreate the error. Might be worth contacting the code owners of the component too, to just remove the space in their code as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @michaelolo24, we should fix the code and see if break anything else

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Distributable file count

id before after diff
default 47130 47890 +760

History

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

@angorayc angorayc merged commit e261aa2 into elastic:master Dec 15, 2020
@angorayc angorayc mentioned this pull request Dec 15, 2020
Merged
angorayc added a commit to angorayc/kibana that referenced this pull request Dec 15, 2020
@angorayc
[Security Solution] Unskip timeline creation Cypress test (elastic#85871
d05a7e5 
)

* fix cypress

* add a comment
gmmorris added a commit to ymao1/kibana that referenced this pull request Dec 15, 2020
@gmmorris
Merge branch 'master' into pr/85462
fa70d31 
* master: (66 commits)
  [Alerting] fixes broken Alerting Example plugin (elastic#85774)
  [APM] Service overview instances table (elastic#85770)
  [Security Solution] Unskip timeline creation Cypress test (elastic#85871)
  properly recognize enterprise licenses (elastic#85849)
  [SecuritySolution][Detections] Adds SavedObject persistence to Signals Migrations (elastic#85690)
  [TSVB] Fix functional tests flakiness and unskip them (elastic#85388)
  [Fleet] Change permissions for Fleet enroll role (elastic#85802)
  Gauge visualization can no longer be clicked to filter on values since Kibana 7.10.0 (elastic#84768)
  [Security Solution][Detections] Add alert source to detection rule action context (elastic#85488)
  [Discover] Don't display hide/show button for histogram when there's no time filter (elastic#85424)
  skip flaky suite (elastic#78553)
  License checks for alerts plugin (elastic#85649)
  skip flaky suite (elastic#84992)
  skip 'query return results valid for scripted field' elastic#78553
  Allow action types to perform their own mustache variable escaping in parameter templates (elastic#83919)
  [ML] More machine learning links in doc_links_service.ts (elastic#85365)
  Removed Alerting & Event Log deprecated fields that should not be using (elastic#85652)
  Closes elastic#79995 by adding new tab in transaction details to show related trace logs. (elastic#85859)
  Fix outdated jest snapshot
  [Maps] Surface on prem EMS (elastic#85729)
  ...
MadameSheema
MadameSheema reviewed Dec 15, 2020
View reviewed changes
x-pack/plugins/security_solution/cypress/integration/timeline_creation.spec.ts
@@ -46,11 +47,11 @@ import { openTimeline } from '../tasks/timelines';
import { OVERVIEW_URL } from '../urls/navigation';

// FLAKY: https://github.com/elastic/kibana/issues/79389
Copy link
Member

Choose a reason for hiding this comment

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

Can you please remove this line and close the related ticket if is still opened?

MadameSheema
MadameSheema reviewed Dec 15, 2020
View reviewed changes
x-pack/plugins/security_solution/cypress/integration/timeline_creation.spec.ts
getTimelineById(timelineId).then((singleTimeline) => {
const noteId = singleTimeline!.body.data.getOneTimeline.notes[0].noteId;

cy.get(`${NOTE_BY_NOTE_ID(noteId)} p`).should('have.text', timeline.notes);
Copy link
Member

Choose a reason for hiding this comment

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

Can we please extract this to our screens folder?

@angorayc
Copy link
Contributor Author

Thank you @MadameSheema , I've fixed them in #85905

angorayc added a commit that referenced this pull request Dec 15, 2020
@angorayc @kibanamachine
[Security Solution] Unskip timeline creation Cypress test (#85871) (#…
4bcf39f 
…85923)

* fix cypress

* add a comment

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@angorayc angorayc mentioned this pull request Dec 22, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XavierM XavierM XavierM left review comments

@michaelolo24 michaelolo24 michaelolo24 approved these changes

@MadameSheema MadameSheema MadameSheema left review comments

Assignees
No one assigned
Labels
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@angorayc @kibanamachine @XavierM @michaelolo24 @MadameSheema