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

FTR: fix 10 sec timeout in waitForDeleted #33313

Merged

Conversation

dmlemeshko
Copy link
Member

@dmlemeshko dmlemeshko commented Mar 15, 2019

Summary

@LeeDr reported a bug with FTR waiting for the element to be removed out of DOM always takes 10 sec.

[16:59:44.442124500] │ debg Find.waitForDeletedByCssSelector('euiLoadingSpinner') with timeout=2345
[16:59:54.481053100] │ debg step 1 -------------------------------------------------------

Why it happened:
waitForDeletedByCssSelector relies on driver.findElements, which uses implicitWait timeout for element to be found: by default timeout= timeouts.find (10 sec), so driver.findElements tries to find element up to 10 sec.

Fix: implicit wait is reset to 1000 (1 sec) on start to rely only on explicitTImeout so that findElements stops execution on the first attempt and returns the result if elements not found.

Since we no longer use Leadfoot, I fixed the following mismatch:
find service has waitForDeletedByCssSelector
web_element_wrapper has waitForDeletedByClassName

Checklist

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

- [ ] This was checked for cross-browser compatibility, including a check against IE11
- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials

For maintainers

- [ ] This was checked for breaking API changes and was labeled appropriately
- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately

@dmlemeshko dmlemeshko changed the title FTR: fix 10 sec wait in waitForDeletedByClassName FTR: fix 10 sec timeout in waitForDeleted Mar 15, 2019
@dmlemeshko dmlemeshko requested a review from LeeDr March 15, 2019 14:10
@dmlemeshko dmlemeshko requested a review from cuff-links March 15, 2019 14:13
@elasticmachine
Copy link
Contributor

elasticmachine commented Mar 15, 2019

💔 Build Failed

X-Pack Functional Tests.x-pack/test/functional/apps/maps/es_search_source·js.maps app elasticsearch document layer query bar should apply query to fit to bounds

setting implicitWait to 0 might be not the best approach, I will investigate

@dmlemeshko dmlemeshko added the WIP Work in progress label Mar 15, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@dmlemeshko dmlemeshko self-assigned this Mar 21, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

elasticmachine commented Mar 21, 2019

💔 Build Failed

The only failure:
JOB-xpack-intake: X-Pack Jest Tests.x-pack/plugins/rollup/jest/client_integration.Create Rollup Job, step 5: Metrics save() should call the "create" Api server endpoint

xpack-ciGroup3 passed 20x times 💪

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@dmlemeshko dmlemeshko removed the WIP Work in progress label Mar 22, 2019
@dmlemeshko
Copy link
Member Author

dmlemeshko commented Mar 22, 2019

@LeeDr @silne30 I really tried to avoid using sleep for the map test: I took DOM snapshots before, during and after canvas map loading - there were no changes we can use as the condition to wait for.
Running 20x times ciGroup3 showed that sleep(5000) is stable and I don't see a better way for now.

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 only code reviewed and checked Jenkins results. The test I initially saw the problem is was
kibana/test/functional/apps/context/_filters.js (in ciGroup1)
We can see from the screenshots below that this PR saves almost 1 minute off that test.

Closes: 33168

Other PR which doesn't have this fix;
image

This PR with the fix;
image

@LeeDr
Copy link

LeeDr commented Mar 26, 2019

Closes: #33168

@dmlemeshko dmlemeshko merged commit 8b0d120 into elastic:master Mar 26, 2019
dmlemeshko added a commit to dmlemeshko/kibana that referenced this pull request Mar 26, 2019
* use css selector instead of className, set implicit wait to 0

* set implicitWait to 2 sec

* set default implicitWait to WAIT_FOR_EXISTS_TIME

* set timeout to 1 sec, retry for query bar test

* sleep 5 sec waiting zoom is finished

* sleep is the only way to wait

* run x-pack-ciGroup3 20x times

* Revert "run x-pack-ciGroup3 20x times"

This reverts commit 55482de.
dmlemeshko added a commit that referenced this pull request Mar 27, 2019
* use css selector instead of className, set implicit wait to 0

* set implicitWait to 2 sec

* set default implicitWait to WAIT_FOR_EXISTS_TIME

* set timeout to 1 sec, retry for query bar test

* sleep 5 sec waiting zoom is finished

* sleep is the only way to wait

* run x-pack-ciGroup3 20x times

* Revert "run x-pack-ciGroup3 20x times"

This reverts commit 55482de.
dmlemeshko added a commit to dmlemeshko/kibana that referenced this pull request Mar 27, 2019
* use css selector instead of className, set implicit wait to 0

* set implicitWait to 2 sec

* set default implicitWait to WAIT_FOR_EXISTS_TIME

* set timeout to 1 sec, retry for query bar test

* sleep 5 sec waiting zoom is finished

* sleep is the only way to wait

* run x-pack-ciGroup3 20x times

* Revert "run x-pack-ciGroup3 20x times"

This reverts commit 55482de.
dmlemeshko added a commit that referenced this pull request Mar 27, 2019
* use css selector instead of className, set implicit wait to 1 sec
@dmlemeshko
Copy link
Member Author

Backport
7.0 0b55ee9
7.x 0caf154

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

joelgriffith pushed a commit that referenced this pull request Mar 27, 2019
* use css selector instead of className, set implicit wait to 0

* set implicitWait to 2 sec

* set default implicitWait to WAIT_FOR_EXISTS_TIME

* set timeout to 1 sec, retry for query bar test

* sleep 5 sec waiting zoom is finished

* sleep is the only way to wait

* run x-pack-ciGroup3 20x times

* Revert "run x-pack-ciGroup3 20x times"

This reverts commit 55482de.
@dmlemeshko dmlemeshko deleted the ftr-fix-waiting-for-element-not-present branch May 23, 2019 08:49
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.

3 participants