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

[RsponseOps] Fix flaky rules list test #131567

Merged

Conversation

JiaweiWu
Copy link
Contributor

@JiaweiWu JiaweiWu commented May 4, 2022

Summary

Resolves: #131535

Waits for the rules list to re-render before assertions

@JiaweiWu JiaweiWu added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.3.0 labels May 4, 2022
await assertRulesLength(1);

// Select disabled
await testSubjects.click('ruleStatusFilterOption-enabled');
await testSubjects.click('ruleStatusFilterOption-disabled');
await new Promise((res) => setTimeout(res, 1000));
Copy link
Contributor

@spalger spalger May 4, 2022

Choose a reason for hiding this comment

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

Is there anything on the page you can check for before asserting the rules length? Using a sleep like this is only going to work in very specific timing conditions which change constantly, and extending the sleep to work in more conditions just extends the length of CI unnecessarily.

If there's a loading message in the UI that tells users the list is updating, then we should use the same indication to tell the tests that the list is still loading, like:

await testSubjects.waitForDeleted('ruleTableLoading');
await assertRulesLength(1);

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, checkout my wait functions in this test

@JiaweiWu
Copy link
Contributor Author

JiaweiWu commented May 4, 2022

passing flaky test runner build: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/558

@JiaweiWu JiaweiWu marked this pull request as ready for review May 5, 2022 17:19
@JiaweiWu JiaweiWu requested a review from a team as a code owner May 5, 2022 17:19
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@JiaweiWu JiaweiWu requested a review from spalger May 5, 2022 17:19
@JiaweiWu
Copy link
Contributor Author

JiaweiWu commented May 5, 2022

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

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

Nice work!

@JiaweiWu JiaweiWu requested a review from spalger May 5, 2022 23:05
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

LGTM

@JiaweiWu JiaweiWu merged commit 37443e9 into elastic:main May 6, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label May 6, 2022
jloleysens added a commit to jloleysens/kibana that referenced this pull request May 9, 2022
…hromium-to-print-pdf-part-1

* 'main' of github.com:elastic/kibana: (59 commits)
  [Cloud Posture] Enabled findings group by feature (elastic#131780)
  [EBT] Fix `userId` generation (elastic#131701)
  [RAM] Add shareable rule tag filter (elastic#130710)
  Optimize package installation performance, phase 2 (elastic#131627)
  [Screenshotting] instrument for benchmark tests using new EventLogger class (elastic#130356)
  [Connector] Adding internal route for requesting ad-hoc ServiceNow access token (elastic#131171)
  [ci] bump kibana-buildkite-library (elastic#131754)
  [Synthetics] UI clean up (elastic#131598)
  [RsponseOps] Fix flaky rules list test (elastic#131567)
  [Cases] Add severity field to create case (elastic#131626)
  [Discover] Monospace font in Document Explorer (elastic#131513)
  Sessions tab improvements (elastic#131583)
  Add cloud icon "ess-icon" at the end of the config keys in "alerting" documentation (elastic#131735)
  [DOCS] Updates deprecation text for legacy APIs (elastic#131741)
  [ci] break out skip patterns so they can change without triggering CI (elastic#131726)
  Adjust search session management page font size (elastic#131291)
  [Unified search] Fix uptime css problem (elastic#131730)
  [Actionable Observability] Link to filtered rules page (elastic#131629)
  Add openAPI specifications for cases endpoint (elastic#131275)
  Display rule API key owner to users who can manage API keys (elastic#131662)
  ...

# Conflicts:
#	x-pack/plugins/screenshotting/server/formats/pdf/index.ts
#	x-pack/plugins/screenshotting/server/screenshots/observable.ts
kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022
* Add delay to make test less flaky

* Addressed comments

* Addressed comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.3.0
Projects
None yet
6 participants