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

Alerting/fix flaky instance test #58994

Merged
merged 19 commits into from
Mar 11, 2020

Conversation

gmmorris
Copy link
Contributor

@gmmorris gmmorris commented Mar 1, 2020

Summary

The Alert Instances list calculates duration on each page load, which makes it hard for the test runner to know what the correct value should be.
In the PR we expose the epoch used by the duration calculation in such a way that the test runner can read it and asses the duration correctly.

closes #57426

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@gmmorris
Copy link
Contributor Author

gmmorris commented Mar 2, 2020

@elasticmachine merge upstream

1 similar comment
@gmmorris
Copy link
Contributor Author

gmmorris commented Mar 2, 2020

@elasticmachine merge upstream

@mikecote
Copy link
Contributor

mikecote commented Mar 2, 2020

@elasticmachine merge upstream

@gmmorris
Copy link
Contributor Author

gmmorris commented Mar 3, 2020

@elasticmachine merge upstream

@gmmorris
Copy link
Contributor Author

gmmorris commented Mar 3, 2020

@elasticmachine merge upstream

elasticmachine and others added 4 commits March 3, 2020 00:40
* master:
  Dashboard a11y tests (elastic#58122)
  Downgrade "setting up plugin" log to debug (elastic#58776)
  [CI] Pipeline refactoring (elastic#56447)
  [Advanced Settings] Fix a11y of unsaved indicator (elastic#58511)
  put params into short url instead of behind it (elastic#58846)
  show timepicker in timelion and tsvb (elastic#58857)
  improve graph missing workspace error message (elastic#58876)
…s/kibana into alerting/fix-flaky-instance-test

* 'alerting/fix-flaky-instance-test' of github.com:gmmorris/kibana:
  [Endpoint] Alert Details Overview (elastic#58412)
  Service map language icons (elastic#58633)
  [SIEM] [Case] Comments to case view (elastic#58315)
  Remove appBasePath from docs + add mock for AppMountParameters (elastic#58775)
  [kbn/optimizer] fix ui/* url rewrites in dist (elastic#58627)
@gmmorris
Copy link
Contributor Author

gmmorris commented Mar 9, 2020

@elasticmachine merge upstream

@gmmorris
Copy link
Contributor Author

@elasticmachine merge upstream

elasticmachine and others added 4 commits March 10, 2020 06:03
…s/kibana into alerting/fix-flaky-instance-test

* 'alerting/fix-flaky-instance-test' of github.com:gmmorris/kibana: (176 commits)
  Generate docs from data plugin (elastic#56955)
  Fixes elastic#59513 by hiding one of the symmetric edges rather than omiting it (elastic#59514)
  Harden creation of child processes (elastic#55697)
  [Alerting] replace watcher http APIs used by index threshold Alerting (elastic#59475)
  [Maps][docs] add more details to Quantitative data driven styling docs (elastic#59553)
  chore: 🤖 hide Drilldowns in master (elastic#59698)
  [Discover] Migrate AppState/GlobalState to new app state helpers (elastic#57175)
  Use HTTP request schemas to create types, use those types in the client (elastic#59340)
  [Maps] Support categorical styling for numbers and dates (elastic#57908)
  [ML] Functional API tests - bucket span estimation with custom search.max_buckets (elastic#59665)
  Fix slm_ui setting by changing camel case back to snake case. (elastic#59663)
  removes beta tag (elastic#59618)
  [DOCS] Removed spatial references (elastic#59595)
  fix outdated docs (elastic#58729)
  [ML] Fixes bucket span estimators loading of max_buckets setting (elastic#59639)
  [ML] Refactoring anomaly detector job types (elastic#59556)
  [Upgrade Assistant] Better handling of closed indices (elastic#58890)
  additional visualizations plugin cleanup before moving to NP (elastic#59318)
  In scripted fields, unable to switch the `Type` - getting a console error which says - Class constructor DecoratedFieldFormat cannot be invoked without 'new' (elastic#59285)
  [Visualize] Remove global state in visualize (elastic#58352)
  ...
* master:
  [Logs / Metrics UI] Link handling / stop page reloads (elastic#58478)
  Add SavedObject management section registration in core  (elastic#59291)
@gmmorris gmmorris marked this pull request as ready for review March 10, 2020 14:22
@gmmorris gmmorris requested a review from a team as a code owner March 10, 2020 14:22
@gmmorris gmmorris added Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.7.0 v8.0.0 labels Mar 10, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@gmmorris gmmorris added release_note:fix and removed release_note:skip Skip the PR/issue when compiling release notes labels Mar 10, 2020
Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

Seems like we're bending over backwards to get the test to not be flaky, at the expense of adding an additional level of redirection with the durationSince function that returns a function.

I have a sneaking suspicion this is all moment's fault, and if we just eradicated it from this code, everything would work :-)

I could be convinced to approve this though; I think I'd want a new code-debt issue opened to try a cleaner fix (ie, remove moment usage). Convince me!

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM. Absolutely great test coverage - that's why such a kind of issues happening :)!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@gmmorris gmmorris merged commit fa16b2b into elastic:master Mar 11, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 11, 2020
The Alert Instances list calculates duration on each page load, which makes it hard for the test runner to know what the correct value should be.
In the PR we expose the epoch used by the duration calculation in such a way that the test runner can read it and asses the duration correctly.
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 11, 2020
* master:
  [ML] Transforms: Use EuiInMemoryTable instead of custom typed table. (elastic#59782)
  Alerting/fix flaky instance test (elastic#58994)
  ci: disable all Mocha rules for tape tests (elastic#59798)
  Fix UX in alerting UI forms when errors occur (elastic#59444)
  [DOCS] Updated and added jump tables (elastic#59774)
  [DOCS] Moved rolled up index content (elastic#59372)
  Regenerate core api docs (elastic#59814)
  [Lens] remove react warnings (elastic#59574)
  The scripts/backport.js file isn't an executable (elastic#59800)
  [Alerting] add more alert properties to action parameter templating (elastic#59718)
  [Design] Branding changes in Elastic to focus more towards the Elastic brand (elastic#58160)
  [SIEM] Adds 'Create new rule' Cypress test (elastic#59790)
  Updating svgo -> css-tree -> mdn-data, all so we get mdn-data > 2.0 (elastic#58913)
  Use EUI test environment build with Jest (elastic#55877)
  update typescript version in all packages to avoid warnings (elastic#59787)
  [SIEM] [Case] Insert timeline into case textarea (elastic#59586)
  [ML] Functional tests - stabilize saved search tests (elastic#59652)
gmmorris added a commit that referenced this pull request Mar 11, 2020
The Alert Instances list calculates duration on each page load, which makes it hard for the test runner to know what the correct value should be.
In the PR we expose the epoch used by the duration calculation in such a way that the test runner can read it and asses the duration correctly.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
simianhacker pushed a commit to simianhacker/kibana that referenced this pull request Mar 12, 2020
The Alert Instances list calculates duration on each page load, which makes it hard for the test runner to know what the correct value should be.
In the PR we expose the epoch used by the duration calculation in such a way that the test runner can read it and asses the duration correctly.
jkelastic pushed a commit to jkelastic/kibana that referenced this pull request Mar 12, 2020
The Alert Instances list calculates duration on each page load, which makes it hard for the test runner to know what the correct value should be.
In the PR we expose the epoch used by the duration calculation in such a way that the test runner can read it and asses the duration correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported Feature:Alerting release_note:fix Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v7.7.0 v8.0.0
Projects
None yet
6 participants