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.

Sign up for GitHub

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

Jump to bottom

WebElementWrapper: add findByTestSubject/findAllByTestSubject to search with data-test-subj #60568

Merged
merged 8 commits into from
Mar 20, 2020
Merged

WebElementWrapper: add findByTestSubject/findAllByTestSubject to search with data-test-subj #60568

merged 8 commits into from
Mar 20, 2020

Conversation

dmlemeshko
Copy link
Member

@dmlemeshko dmlemeshko commented Mar 18, 2020 •
edited
Loading

Summary

To avoid using [data-test-subj="_value_"]'); while searching for child element(s) I'm adding find/findAll to WebElementWrapper

before:

const toggleBtn = await comboBoxElement.findByCssSelector('[data-test-subj="comboBoxToggleListButton"]');

after:

const toggleBtn = await comboBoxElement.findByTestSubject('comboBoxToggleListButton');

@dmlemeshko dmlemeshko added release_note:skip Skip the PR/issue when compiling release notes test_ui_functional v7.7.0 v8.0.0 labels Mar 18, 2020
LeeDr
LeeDr suggested changes Mar 18, 2020
View reviewed changes
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.

I love it, but think ByTestSubject would be good to add to the method names to make them more obvious? But I could be talked out of it.

test/functional/page_objects/common_page.ts Outdated
log.debug(`currentUrl = ${currentUrl}\n appUrl = ${appUrl}`);
await find.byCssSelector('[data-test-subj="kibanaChrome"]', 6 * defaultFindTimeout); // 60 sec waiting
log.debug(`currentUrl = ${currentUrl}\n appUrl = ${appUrl}`); // 60 sec waiting
await testSubjects.find('kibanaChrome', 6 * defaultFindTimeout);
Copy link

Choose a reason for hiding this comment

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

you probably didn't move the comment intentionally?

test/functional/services/lib/web_element_wrapper/web_element_wrapper.ts Show resolved Hide resolved
test/functional/services/lib/web_element_wrapper/web_element_wrapper.ts Show resolved Hide resolved
@dmlemeshko dmlemeshko marked this pull request as ready for review March 19, 2020 07:17
@dmlemeshko dmlemeshko requested a review from a team as a code owner March 19, 2020 07:17
@dmlemeshko dmlemeshko requested a review from LeeDr March 19, 2020 09:25
mikecote
mikecote approved these changes Mar 19, 2020
View reviewed changes
Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

Alerting changes LGTM

@dmlemeshko
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@LeeDr LeeDr changed the title WebElementWrapper: add find/findAll to search with data-test-subj WebElementWrapper: add findByTestSubject/findAllByTestSubject to search with data-test-subj Mar 20, 2020
LeeDr
LeeDr approved these changes Mar 20, 2020
View reviewed changes
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 - great change

@dmlemeshko dmlemeshko merged commit 992c502 into elastic:master Mar 20, 2020
@dmlemeshko dmlemeshko mentioned this pull request Mar 20, 2020
Merged
dmlemeshko added a commit to dmlemeshko/kibana that referenced this pull request Mar 20, 2020
@dmlemeshko @elasticmachine
WebElementWrapper: add findByTestSubject/findAllByTestSubject to sear…
77ecae6 
…ch with data-test-subj (elastic#60568)

* [web_element_wrapper] add find/findAll to search with data-test-subj

* fixes

* fix wrong function call

* review fixes

* simplify test

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 20, 2020
@gmmorris
Merge branch 'master' into alerting/api-constant
377acfc 
* master: (52 commits)
  [SIEM] Fix types in rules tests (elastic#60736)
  [Alerting] prevent flickering when fields are updated in an alert (elastic#60666)
  License checks for actions plugin (elastic#59070)
  Implemented ability to clear and properly validate alert interval (elastic#60571)
  WebElementWrapper: add findByTestSubject/findAllByTestSubject to search with data-test-subj (elastic#60568)
  [Maps] Update layer dependencies to NP (elastic#59585)
  [Discover] Remove StateManagementConfigProvider (elastic#60221)
  [ML] Listing all categorization wizard checks (elastic#60502)
  [Upgrade Assistant] First iteration of batch reindex docs (elastic#59887)
  [SIEM] Export timeline (elastic#58368)
  [SIEM] Add support for actions and throttle in Rules (elastic#59641)
  Fix ace a11y listener (elastic#60639)
  Add addInfo toast to core notifications service (elastic#60574)
  fix test description (elastic#60638)
  [SIEM] Cypress screenshots upload to google cloud (elastic#60556)
  [canvas/shareable_runtime] sync sass loaders with kbn/optimizer (elastic#60653)
  [SIEM] Fixes Modification of ML Rules (elastic#60662)
  [SIEM] [Case] Bulk status update, add comment avatar, id => title in breadcrumbs (elastic#60410)
  [Alerting] add functional tests for index threshold alertType (elastic#60597)
  [Ingest]EMT-248: add post action request handler and resources (elastic#60581)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 20, 2020
@gmmorris
Merge branch 'master' into alerting/empty-list
e8c7be8 
* master: (55 commits)
  Update dependency @elastic/charts to v18.1.0 (elastic#60578)
  Only set timezone when user setting is a valid timezone (elastic#57850)
  [NP] Remove `ui/agg_types` dependencies and move paginated table to kibana_legacy (elastic#60276)
  [SIEM] Fix types in rules tests (elastic#60736)
  [Alerting] prevent flickering when fields are updated in an alert (elastic#60666)
  License checks for actions plugin (elastic#59070)
  Implemented ability to clear and properly validate alert interval (elastic#60571)
  WebElementWrapper: add findByTestSubject/findAllByTestSubject to search with data-test-subj (elastic#60568)
  [Maps] Update layer dependencies to NP (elastic#59585)
  [Discover] Remove StateManagementConfigProvider (elastic#60221)
  [ML] Listing all categorization wizard checks (elastic#60502)
  [Upgrade Assistant] First iteration of batch reindex docs (elastic#59887)
  [SIEM] Export timeline (elastic#58368)
  [SIEM] Add support for actions and throttle in Rules (elastic#59641)
  Fix ace a11y listener (elastic#60639)
  Add addInfo toast to core notifications service (elastic#60574)
  fix test description (elastic#60638)
  [SIEM] Cypress screenshots upload to google cloud (elastic#60556)
  [canvas/shareable_runtime] sync sass loaders with kbn/optimizer (elastic#60653)
  [SIEM] Fixes Modification of ML Rules (elastic#60662)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 20, 2020
@gmmorris
Merge branch 'master' into alerting/tls-warning
5a5c8a4 
* master:
  Only set timezone when user setting is a valid timezone (elastic#57850)
  [NP] Remove `ui/agg_types` dependencies and move paginated table to kibana_legacy (elastic#60276)
  [SIEM] Fix types in rules tests (elastic#60736)
  [Alerting] prevent flickering when fields are updated in an alert (elastic#60666)
  License checks for actions plugin (elastic#59070)
  Implemented ability to clear and properly validate alert interval (elastic#60571)
  WebElementWrapper: add findByTestSubject/findAllByTestSubject to search with data-test-subj (elastic#60568)
  [Maps] Update layer dependencies to NP (elastic#59585)
  [Discover] Remove StateManagementConfigProvider (elastic#60221)
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 21, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

LeeDr pushed a commit that referenced this pull request Mar 23, 2020
@dmlemeshko @elasticmachine
WebElementWrapper: add findByTestSubject/findAllByTestSubject to sear…
4c7a447 
…ch with data-test-subj (#60568) (#60756)

* [web_element_wrapper] add find/findAll to search with data-test-subj

* fixes

* fix wrong function call

* review fixes

* simplify test

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 23, 2020
@dmlemeshko dmlemeshko deleted the ftr/find+findAll-on-webElementWrapper branch January 31, 2022 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LeeDr LeeDr LeeDr approved these changes

@mikecote mikecote mikecote approved these changes

Assignees
No one assigned
Labels
release_note:skip Skip the PR/issue when compiling release notes test_ui_functional v7.7.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@dmlemeshko @kibanamachine @mikecote @LeeDr @elasticmachine