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

Removes archives #86537

Merged
merged 30 commits into from
Dec 28, 2020
Merged

Conversation

MadameSheema
Copy link
Member

@MadameSheema MadameSheema commented Dec 18, 2020

Summary

In this PR we are deleting all the archives that are not related with data shippers indexes in order to help to stabilise more the test suite.

Also some refactor has been done following up some of the comments of this PR: #86093

@MadameSheema MadameSheema force-pushed the removes-archives branch 3 times, most recently from 9e6e6c5 to b6a10d2 Compare December 19, 2020 11:13
@MadameSheema MadameSheema self-assigned this Dec 21, 2020
@MadameSheema MadameSheema added v7.11.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. labels Dec 21, 2020
@MadameSheema MadameSheema marked this pull request as ready for review December 21, 2020 11:44
Copy link
Contributor

@patrykkopycinski patrykkopycinski left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

This looks amazing! So nice to get rid of all those archives.

I had a few comments about consistency/redundancy, but this was already approved and I'm fine moving some of those tasks to a later PR. If you do want to fix them here, I'm happy to do another quick pass and approve 👍

x-pack/plugins/security_solution/cypress/tasks/common.ts Outdated Show resolved Hide resolved

afterEach(() => {
esArchiverUnload('alerts');
waitForAlertsPanelToBeLoaded();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little bit wary of moving these steps into a beforeEach because of the fail-fast behavior and potential for false negatives, but I think that the speed and DRYness of this approach make it an acceptable tradeoff.

loginAndWaitForPageWithoutDateRange(DETECTIONS_URL);
waitForAlertsPanelToBeLoaded();
waitForAlertsIndexToBeCreated();
createCustomRule(newRule, 'rule1');
Copy link
Contributor

Choose a reason for hiding this comment

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

💯 this is so much better, it's immediately clear that we're working with four rules in this test.

});

after(() => {
esArchiverUnload('prebuilt_rules_loaded');
cy.clock().invoke('restore');
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this teardown is specific to a single time-traveling test. It would be nice if such tests were isolated in their own context with clock-specific setup/teardown hooks.


Remember to clean up the state of the test after its execution.
Remember to use the `cleanKibana` method before starting the execution of the test
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than leaving this as a task for the test writer, we could enforce it as a global beforeEach hook.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here I'm not 100% sure. There are some cases where using before instead of beforeEach, save us execution time. So we need to take into consideration that if we move that to a global hook, we are going to make our tests more slow. Another reason is because I don't like how Cypress handles right now global hooks.

Anyway, this is a think we can investigate/work in a different PR :)

@MadameSheema
Copy link
Member Author

@elasticmachine merge upstream

@MadameSheema
Copy link
Member Author

@elasticmachine merge upstream

@patrykkopycinski
Copy link
Contributor

@elasticmachine merge upstream

@patrykkopycinski
Copy link
Contributor

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Distributable file count

id before after diff
default 48026 48027 +1

History

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

@patrykkopycinski patrykkopycinski merged commit c0d6e12 into elastic:master Dec 28, 2020
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request Dec 28, 2020
# Conflicts:
#	x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request Dec 28, 2020
# Conflicts:
#	x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts
patrykkopycinski added a commit that referenced this pull request Dec 28, 2020
# Conflicts:
#	x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts

Co-authored-by: MadameSheema <[email protected]>
patrykkopycinski added a commit that referenced this pull request Dec 28, 2020
# Conflicts:
#	x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts

Co-authored-by: MadameSheema <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 30, 2020
* master: (108 commits)
  [DOCS] Refreshes Data Visualizer screenshot (elastic#87017)
  [Security Solution] Change 'anti-virus' text to 'antivirus' (elastic#87001)
  [securitySolution/cypress] temporarily limit to PRs
  [AppServices/Examples] Add the example for Reporting integration (elastic#82091)
  [Build Chromium] Improve git checkout (elastic#83225)
  Deprecate `services.callCluster` in alerts and actions executors (elastic#86474)
  [Security Solution] Use system node version for Cypress and increase exec command timeout (elastic#86985)
  [Lens] Add more chart editor tests based on the debug state (elastic#86750)
  [Lens] Integrate searchSessionId into Lens app (elastic#86297)
  skip "pagination updates results and page number" elastic#86975
  skip "Custom detection rules" elastic#83772
  [logging/json] use merge from kbn/std (elastic#86330)
  skip network and timeline inspection. elastic#85677, elastic#85678
  skip "adds correctly a filter to the global search bar" elastic#86552
  [ftr/flags] improve help text (elastic#86971)
  skip "Fields Browser rendering. elastic#60209
  skip "Closes and opens alerts" elastic#83773
  [Security Solution] Skip failing Cypress tests (elastic#86967)
  Removes archives (elastic#86537)
  [ML] Fix charts grid on the Anomaly Explorer page (elastic#86904)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 30, 2020
* master: (72 commits)
  [DOCS] Refreshes Data Visualizer screenshot (elastic#87017)
  [Security Solution] Change 'anti-virus' text to 'antivirus' (elastic#87001)
  [securitySolution/cypress] temporarily limit to PRs
  [AppServices/Examples] Add the example for Reporting integration (elastic#82091)
  [Build Chromium] Improve git checkout (elastic#83225)
  Deprecate `services.callCluster` in alerts and actions executors (elastic#86474)
  [Security Solution] Use system node version for Cypress and increase exec command timeout (elastic#86985)
  [Lens] Add more chart editor tests based on the debug state (elastic#86750)
  [Lens] Integrate searchSessionId into Lens app (elastic#86297)
  skip "pagination updates results and page number" elastic#86975
  skip "Custom detection rules" elastic#83772
  [logging/json] use merge from kbn/std (elastic#86330)
  skip network and timeline inspection. elastic#85677, elastic#85678
  skip "adds correctly a filter to the global search bar" elastic#86552
  [ftr/flags] improve help text (elastic#86971)
  skip "Fields Browser rendering. elastic#60209
  skip "Closes and opens alerts" elastic#83773
  [Security Solution] Skip failing Cypress tests (elastic#86967)
  Removes archives (elastic#86537)
  [ML] Fix charts grid on the Anomaly Explorer page (elastic#86904)
  ...
@MadameSheema MadameSheema deleted the removes-archives branch July 14, 2021 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants