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

Wrap leadfoot elements #26406

Merged
merged 10 commits into from
Dec 6, 2018

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Nov 29, 2018

Based on #26394

Wrap elements that come from leadfoot so we can expose a limited version of the Leadfoot API and adapt it for a different library like selenium webdriver.

@spalger spalger added the WIP Work in progress label Nov 29, 2018
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the implement/leadfoot-element-wrapper branch 2 times, most recently from 2b4e72d to 9d9c3f1 Compare November 29, 2018 11:43
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the implement/leadfoot-element-wrapper branch 3 times, most recently from e9e6756 to 8312a1a Compare November 29, 2018 12:59
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the implement/leadfoot-element-wrapper branch from 8312a1a to c5d734e Compare November 29, 2018 15:36
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the implement/leadfoot-element-wrapper branch from c5d734e to 985f0f0 Compare November 30, 2018 04:35
@elasticmachine
Copy link
Contributor

💔 Build Failed

@spalger spalger force-pushed the implement/leadfoot-element-wrapper branch 2 times, most recently from f096988 to 2fd7aee Compare November 30, 2018 06:33
@spalger spalger force-pushed the implement/leadfoot-element-wrapper branch from 2fd7aee to b4dfed8 Compare November 30, 2018 06:51
@elasticmachine
Copy link
Contributor

💔 Build Failed

cuff-links
cuff-links previously approved these changes Dec 4, 2018
Copy link
Contributor

@cuff-links cuff-links left a comment

Choose a reason for hiding this comment

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

Other than the --bail flag, all looks and works well. Ran this on every test suite and the only tests that failed are known issues. This is good to go, IMHO.

tasks/config/run.js Outdated Show resolved Hide resolved
@dmlemeshko
Copy link
Member

LGTM

@spalger spalger force-pushed the implement/leadfoot-element-wrapper branch from b4dfed8 to 3ff31c1 Compare December 4, 2018 18:03
@elasticmachine
Copy link
Contributor

💔 Build Failed

dmlemeshko
dmlemeshko previously approved these changes Dec 4, 2018
Copy link
Member

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

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

This element wrapper API looks nice and the most of it will be easy for migration to WebElement API

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@spalger spalger dismissed stale reviews from dmlemeshko and cuff-links December 5, 2018 17:33

need to get tests passing first

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@dmlemeshko dmlemeshko requested a review from LeeDr December 5, 2018 18:18
*
* @return {Promise<void>}
*/
async moveMouseTo(...args) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we list the argument here explicitly and document then in the doc block too?

@elasticmachine
Copy link
Contributor

💔 Build Failed

@spalger
Copy link
Contributor Author

spalger commented Dec 5, 2018

Test is flaky, failed 3 times on master recently, will disable. Jenkins, test this please

@elasticmachine
Copy link
Contributor

💔 Build Failed

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

There's just one unrelated test failure in the last Jenkins job;
X-Pack Phantom API Reporting Tests.x-pack/test/reporting/api/bwc_existing_indexes·js.phantom BWC report generation into existing indexes existing 6_2 index multiple jobs posted (from (TEST-X-Pack Phantom API Reporting Tests.xml))

@spalger spalger added review test v7.0.0 v6.7.0 and removed WIP Work in progress labels Dec 6, 2018
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger spalger merged commit 502f1b1 into elastic:master Dec 6, 2018
spalger pushed a commit to spalger/kibana that referenced this pull request Dec 6, 2018
* [ftr] wrap all elements so we can swap out leadfoot without disturbing tests

* save

* [visualize/pie_chart] fix chart legend locator

* [services/leadfoot_element_wrapper] add getTagName function

* [services/browser] adjust moveMouseTo function

* [leadfoot/element] remove old args, document new ones
spalger pushed a commit that referenced this pull request Dec 6, 2018
* Wrap leadfoot elements (#26406)

* [ftr] wrap all elements so we can swap out leadfoot without disturbing tests

* save

* [visualize/pie_chart] fix chart legend locator

* [services/leadfoot_element_wrapper] add getTagName function

* [services/browser] adjust moveMouseTo function

* [leadfoot/element] remove old args, document new ones

* [leadfootElementWrapper] add getSpecAttribute() method
@spalger
Copy link
Contributor Author

spalger commented Dec 6, 2018

6.x/6.6: b51987b

@spalger spalger deleted the implement/leadfoot-element-wrapper branch December 6, 2018 18:59
@spalger spalger added v6.6.0 and removed v6.7.0 labels Dec 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants