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

fix flaky TSVB test #44505

Conversation

dmlemeshko
Copy link
Member

@dmlemeshko dmlemeshko commented Aug 30, 2019

Summary

There is a test that constantly fails locally and somehow more stable on CI.

The issue is failing click on dropdown option on TSVB Series Option tab, it fails with StaleElementException.
Kibana 2019-08-30 18-48-43

           └- ✖ fail: "visualize app  visual builder Time Series should show the correct count in the legend with "Human readable" duration formatter"
           │      StaleElementReferenceError: stale element reference: element is not attached to the page document
           │   (Session info: chrome=76.0.3809.100)
           │   (Driver info: chromedriver=76.0.3809.68 (420c9498db8ce8fcd190a954d51297672c1515d5-refs/branch-heads/3809@{#864}),platform=Mac OS X 10.14.6 x86_64)

The only working option for me was to click with a mouse (works on both Chrome and Firefox)

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@dmlemeshko
Copy link
Member Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@dmlemeshko dmlemeshko force-pushed the fix/tsvb-time-series-duration-formatter-test branch from f4ccd04 to dce2452 Compare September 2, 2019 16:52
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@dmlemeshko dmlemeshko added release_note:skip Skip the PR/issue when compiling release notes v7.5.0 v8.0.0 labels Sep 3, 2019
@dmlemeshko dmlemeshko marked this pull request as ready for review September 3, 2019 12:53
Copy link
Contributor

@maryia-lapata maryia-lapata left a comment

Choose a reason for hiding this comment

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

LGTM,
I run tests locally (Mac)

Copy link

@LeeDr LeeDr left a comment

Choose a reason for hiding this comment

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

LGTM - I ran all 'visual builder' tests locally on Windows with Chrome browser and all 26 tests passed. I made one comment about finding data-test-subj's that start with the value parameter but it's not really a change in this PR.

Just for fun I re-ran the 'visual builder' tests with this change (= instead of ^=) and they all passed;

diff --git a/test/functional/services/combo_box.ts b/test/functional/services/combo_box.ts
index 2e44e0248c..65270eac98 100644
--- a/test/functional/services/combo_box.ts
+++ b/test/functional/services/combo_box.ts
@@ -74,7 +74,7 @@ export function ComboBoxProvider({ getService, getPageObjects }: FtrProviderCont

       if (value !== undefined) {
         const selectOptions = await find.allByCssSelector(
-          `.euiFilterSelectItem[title^="${value.toString().trim()}"]`,
+          `.euiFilterSelectItem[title="${value.toString().trim()}"]`,
           WAIT_FOR_EXISTS_TIME
         );

But I didn't run all tests so not fully tested.

@@ -65,21 +73,22 @@ export function ComboBoxProvider({ getService, getPageObjects }: FtrProviderCont
await this.openOptionsList(comboBoxElement);

if (value !== undefined) {
const options = await find.allByCssSelector(
const selectOptions = await find.allByCssSelector(
`.euiFilterSelectItem[title^="${value.toString().trim()}"]`,
Copy link

Choose a reason for hiding this comment

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

I guess this isn't a change in this PR, but I don't understand why we're only looking for the case where the element's title attribute starts with value. I would think we would want to find the element who's title equals value? Maybe this was an issue with data-test-subj's with spaces in them? If that issue is fixed maybe we could improve this here for exact match?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is used to match drop-down option that starts with provided text. I remember testing related PR and may confirm we need it for some tests

@dmlemeshko dmlemeshko merged commit 96cd619 into elastic:master Sep 4, 2019
dmlemeshko added a commit to dmlemeshko/kibana that referenced this pull request Sep 4, 2019
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 4, 2019
…-to-np-ready

* 'master' of github.com:elastic/kibana: (45 commits)
  Fixing flaky copy-to-space test (elastic#44656)
  [ML] Enable data visualizer link on job types page for non-time index (elastic#44667)
  [ML] Adding index pattern title to wizard summary step (elastic#44662)
  Convert remaining data.indexPatterns items to use TS (elastic#44654)
  [visualize/_tsvb_time_series] fix flaky test (elastic#44505)
  [SIEM] Updates fields in map tooltip and fixes filtering on undefined values (elastic#44718)
  [SIEM] Remove deprecation warning from momentJS test (elastic#44732)
  [SIEM] Fixes the rendering order and adds unit tests to prevent rendering to get out of order (elastic#44728)
  [optimizer] use more precise cache-busting strategy (elastic#44724)
  [Maps] Reverse the attribution order so left-to-right order matches top-to-bottom (elastic#44415)
  Update sinon related packages (elastic#43952)
  [SIEM] change flag to drag and drop field (elastic#44705)
  Update dependency @types/object-hash to ^1.3.0 (elastic#43954)
  mark eslint config as root (elastic#44699)
  Adds geo_point to normalizeType helper (elastic#44410)
  [Uptime]Feature/38542  align uptime help menu with other apps (elastic#44536)
  Removed unused remove icon component (elastic#44698)
  Update unit tests to remove warnings from test output. (elastic#41840)
  Fix rawChecksums in chromium build and rename for clarity (elastic#44665)
  [Uptime]removed extra slash from integeration urls (elastic#44469)
  ...

# Conflicts:
#	rfcs/text/0004_application_service_mounting.md
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 4, 2019
…ana into chore/console-to-np-ready

* 'chore/console-to-np-ready' of github.com:jloleysens/kibana: (45 commits)
  Fixing flaky copy-to-space test (elastic#44656)
  [ML] Enable data visualizer link on job types page for non-time index (elastic#44667)
  [ML] Adding index pattern title to wizard summary step (elastic#44662)
  Convert remaining data.indexPatterns items to use TS (elastic#44654)
  [visualize/_tsvb_time_series] fix flaky test (elastic#44505)
  [SIEM] Updates fields in map tooltip and fixes filtering on undefined values (elastic#44718)
  [SIEM] Remove deprecation warning from momentJS test (elastic#44732)
  [SIEM] Fixes the rendering order and adds unit tests to prevent rendering to get out of order (elastic#44728)
  [optimizer] use more precise cache-busting strategy (elastic#44724)
  [Maps] Reverse the attribution order so left-to-right order matches top-to-bottom (elastic#44415)
  Update sinon related packages (elastic#43952)
  [SIEM] change flag to drag and drop field (elastic#44705)
  Update dependency @types/object-hash to ^1.3.0 (elastic#43954)
  mark eslint config as root (elastic#44699)
  Adds geo_point to normalizeType helper (elastic#44410)
  [Uptime]Feature/38542  align uptime help menu with other apps (elastic#44536)
  Removed unused remove icon component (elastic#44698)
  Update unit tests to remove warnings from test output. (elastic#41840)
  Fix rawChecksums in chromium build and rename for clarity (elastic#44665)
  [Uptime]removed extra slash from integeration urls (elastic#44469)
  ...
dmlemeshko added a commit to dmlemeshko/kibana that referenced this pull request Sep 6, 2019
dmlemeshko added a commit to dmlemeshko/kibana that referenced this pull request Sep 6, 2019
@dmlemeshko dmlemeshko deleted the fix/tsvb-time-series-duration-formatter-test branch January 31, 2022 12:18
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 v7.5.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants