Skip to content

Fix Lens smokescreen flaky tests#78566

Merged
dej611 merged 4 commits intoelastic:masterfrom
dej611:fix/77969-2
Sep 28, 2020
Merged

Fix Lens smokescreen flaky tests#78566
dej611 merged 4 commits intoelastic:masterfrom
dej611:fix/77969-2

Conversation

@dej611
Copy link
Copy Markdown
Contributor

@dej611 dej611 commented Sep 28, 2020

Summary

This PR addresses some race conditions in the test helper used for Filter operation in Lens.
It also refactors the configureDimension function to auto-close by default, but offering a keepOpen option for special scenarios.

Flaky test runner result

Fixes #77969

Checklist

@dej611 dej611 added Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Lens v7.10.0 labels Sep 28, 2020
@dej611 dej611 requested a review from flash1293 September 28, 2020 10:43
mbondyra
mbondyra approved these changes Sep 28, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@mbondyra mbondyra left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me. Let me know when it's ready to be re-approved :)

@dej611 dej611 marked this pull request as ready for review September 28, 2020 12:40
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Sep 28, 2020

Pinging @elastic/kibana-app (Team:KibanaApp)

@dej611
Copy link
Copy Markdown
Contributor Author

dej611 commented Sep 28, 2020

@elasticmachine merge upstream

@dej611 dej611 requested review from mbondyra and removed request for flash1293 September 28, 2020 14:40
wylieconlon
wylieconlon approved these changes Sep 28, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@wylieconlon wylieconlon left a comment

Choose a reason for hiding this comment

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

Can you explain why there was a race condition, and how this fixes it? The code change looks fine, but I'm still confused.

x-pack/test/functional/page_objects/lens_page.ts
await PageObjects.common.pressEnterKey();
// Press Tab 2 twice instead of Enter to avoid race condition with the dropdown
await PageObjects.common.pressTabKey();
await PageObjects.common.pressTabKey();
Copy link
Copy Markdown
Contributor

@wylieconlon wylieconlon Sep 28, 2020

Choose a reason for hiding this comment

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

I don't understand why this fixes the issue, what was the race condition? Can we fix that some other way?

Copy link
Copy Markdown
Contributor

@wylieconlon wylieconlon Sep 28, 2020

Choose a reason for hiding this comment

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

@dej611 Are you aware of the queryInput service which is used for dashboard tests? I think it might simplify this whole test.

Copy link
Copy Markdown
Contributor Author

@dej611 dej611 Sep 28, 2020

Choose a reason for hiding this comment

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

I've improved the comment here to explain the context of the bug.
Perhaps some other fix is possible, but the double Tab one is the most consistent from tests, as it safely closes the dropdown without auto-completing the query field.

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Sep 28, 2020

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@dej611 dej611 merged commit c285287 into elastic:master Sep 28, 2020
@dej611 dej611 deleted the fix/77969-2 branch September 28, 2020 16:50
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 28, 2020
@gmmorris
Merge branch 'master' into task-manager/health
6b81d56 
* master:
  Fix APM lodash imports (elastic#78438)
  Add deprecated message to tile_map and region_map visualizations. (elastic#77683)
  Fix Lens smokescreen flaky tests (elastic#78566)
  updated discover with alt text (elastic#77660)
  Fix types (elastic#78619)
  Update tutorial-visualizing.asciidoc (elastic#76977)
  Update tutorial-discovering.asciidoc (elastic#76976)
  [Search] Error notification alignment (elastic#77788)
  Update tutorial-define-index.asciidoc (elastic#76975)
  [Lens] Fieldless operations (elastic#78080)
  [Usage Collection] [schema] Explicit "array" definition (elastic#78141)
  Update tutorial-define-index.asciidoc (elastic#76973)
  Fix --no-basepath references in doc (elastic#78570)
  Move StubIndexPattern to data plugin and convert to TS. (elastic#78518)
  Index pattern class - remove unused methods (elastic#78538)
  [Security Solution] [ALL] Eliminates all console.error and console.warn from Jest output (elastic#78523)
  [Actions] avoids setting a default dedupKey on PagerDuty (elastic#77773)
  First stab at developer-focussed saved objects docs (elastic#71430)
  remove unnecessary config validations (elastic#78527)
@dej611 dej611 mentioned this pull request Sep 29, 2020
Merged
dej611 added a commit to dej611/kibana that referenced this pull request Sep 29, 2020
@dej611 @elasticmachine
Fix Lens smokescreen flaky tests (elastic#78566)
998c1fd 
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
dej611 added a commit that referenced this pull request Sep 29, 2020
@dej611 @elasticmachine
[7.x] Fix Lens smokescreen flaky tests (#78566) (#78783)
73dcb9c 
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

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

@mbondyra mbondyra mbondyra approved these changes

+1 more reviewer

@wylieconlon wylieconlon wylieconlon approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.10.0 v8.0.0

Projects

None yet

Milestone

No milestone

5 participants

@dej611 @elasticmachine @kibanamachine @wylieconlon @mbondyra