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

[Endpoint] Functional Tests cleanup #68756

Merged
merged 8 commits into from
Jun 10, 2020
Merged

[Endpoint] Functional Tests cleanup #68756

merged 8 commits into from
Jun 10, 2020

Conversation

paul-tavares
Copy link
Contributor

Summary

  • Deleted functinoal_ingest_failure tests (no longer applicable)
  • Removed Alerts and Resolver tests (no longer applicable)
  • Renamed functional_endpoint directory to security_solution_endpoint
  • Moved common page object that were in the endpoint_page to the page_utils
  • Removed several unnecessary disabled eslint rules under the management code

Checklist

@paul-tavares paul-tavares added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Endpoint Management Feature:Endpoint Elastic Endpoint feature v7.9.0 labels Jun 10, 2020
@paul-tavares paul-tavares requested review from a team as code owners June 10, 2020 12:21
@paul-tavares paul-tavares self-assigned this Jun 10, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-management (Team:Endpoint Management)

@elasticmachine
Copy link
Contributor

Pinging @elastic/endpoint-app-team (Feature:Endpoint)

@paul-tavares paul-tavares changed the title [Endpoint] Functional Tests clean [Endpoint] Functional Tests cleanup Jun 10, 2020
nnamdifrankie
nnamdifrankie reviewed Jun 10, 2020
View reviewed changes
x-pack/test/security_solution_endpoint/apps/endpoint/index.ts Outdated
Comment on lines 15 to 17

// loadTestFile(require.resolve('./alerts'));
// loadTestFile(require.resolve('./resolver'));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is good, because it is now part of siem dashboard?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes It is part of the Siem

Copy link
Contributor

Choose a reason for hiding this comment

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

It is in the SIEM but we can remove for now

Copy link
Contributor

Choose a reason for hiding this comment

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

But Why you deleting all the test

@EricDavisX
Copy link
Contributor

@igor was working to bring the Resolver tests back to life... not sure if this needs him and you to sync up or not. I think it might be ok to have the tests maintained for now, even if they hit a hidden route to render the alerts. We could certainly break something and want to know right?

@paul-tavares
Copy link
Contributor Author

@EricDavisX thanks for the comment. I did follow up with the Endpoint response team yesterday and got the ok to remove them, but will resurface that discussion again prior merging this 😃

@IgorGuz2000
Copy link
Contributor

Why Are we deleting all the test for the alerts and resolver

@paul-tavares
Copy link
Contributor Author

@IgorGuz2000 I was deleting them because I did not think they applied anymore. Based on your comment I can bring them back and will create an follow up issue for someone to re-enable them. Sound good?

@oatkiller
Copy link
Contributor

I'm working on adding Resolver back into the app. I can take a look at adding tests

@dplumlee
Copy link
Contributor

dplumlee commented Jun 10, 2020 •
edited
Loading

@paul-tavares @IgorGuz2000 I'm unaware of any reasons to keep the alerts functional tests, the page isn't being used and the pieces we do take off to use elsewhere would require us to write new tests anyways. In my opinion, that file is dead code and should be deleted

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@paul-tavares paul-tavares merged commit 577381d into elastic:master Jun 10, 2020
@paul-tavares paul-tavares deleted the task/EMT-446-cleanup branch June 10, 2020 21:10
@paul-tavares paul-tavares mentioned this pull request Jun 10, 2020
Merged
paul-tavares added a commit to paul-tavares/kibana that referenced this pull request Jun 10, 2020
@paul-tavares
[Endpoint] Functional Tests cleanup (elastic#68756)
31c71be 
* Removed several unnecessary disabled eslint rules
* moved common pageobject from endpoint_list to page_utils
* Rename functional_endpoint to security_solution_endpoint
* Delete `functional_endpoint_ingest_failure` no longer applicable

# Conflicts:
#	.github/CODEOWNERS
#	x-pack/test/security_solution_endpoint/apps/endpoint/resolver.ts
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 11, 2020
@gmmorris
Merge branch 'master' into alerting/consumer-based-rbac
34b7cf9 
* master: (38 commits)
  Support migrating from reserved feature privileges (elastic#68504)
  add `preference` field to SavedObjectsFindOptions (elastic#68620)
  [ILM] Add "wait for snapshot" policy field to Delete phase (elastic#68505)
  Cleanup old license overwrites (elastic#68744)
  Bump TypeScript to v3.9 (elastic#67666)
  [APM] Service maps - adds new storybook stories to test out various data sets (elastic#68727)
  Fix vega specification parsing (elastic#67963)
  docs: add more api information (elastic#68717)
  [APM] Don't show annotations on charts with no data (elastic#68829)
  [Metrics UI] Fix Inventory View sorting by handling null values (elastic#67889)
  skip flaky suite (elastic#68836)
  [SIEM][Detections Engine] - Fix reference rule url overflow (elastic#68640)
  Index pattern public api => common (elastic#68289)
  [APM] Lazy-load alert triggers (elastic#68806)
  [DOCS] Fix table formatting in ingest manager settings (elastic#68824)
  [Endpoint] Functional Tests cleanup (elastic#68756)
  revert previous commit which was unintentional
  Use Github token instead for project assignments
  [SIEM][Exceptions] - ExceptionsViewer cleanup (elastic#68739)
  move @kbn/storybook to devDeps (elastic#68791)
  ...
paul-tavares added a commit that referenced this pull request Jun 12, 2020
@paul-tavares @elasticmachine
[7.x] [Endpoint] Functional Tests cleanup (#68756) (#68822)
521493f 
* [Endpoint] Functional Tests cleanup (#68756)

* Removed several unnecessary disabled eslint rules
* moved common pageobject from endpoint_list to page_utils
* Rename functional_endpoint to security_solution_endpoint
* Delete `functional_endpoint_ingest_failure` no longer applicable

# Conflicts:
#	.github/CODEOWNERS
#	x-pack/test/security_solution_endpoint/apps/endpoint/resolver.ts

* Sync Endpoint Alerts page objects with master

- PR #64704 was never
  backported, thus this change is needed to ensure build is
  successful.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IgorGuz2000 IgorGuz2000 IgorGuz2000 left review comments

@nnamdifrankie nnamdifrankie nnamdifrankie left review comments

@kevinlog kevinlog kevinlog approved these changes

Assignees

@paul-tavares paul-tavares

Labels
Feature:Endpoint Elastic Endpoint feature release_note:skip Skip the PR/issue when compiling release notes v7.9.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@paul-tavares @elasticmachine @EricDavisX @IgorGuz2000 @oatkiller @dplumlee @kibanamachine @kevinlog @nnamdifrankie