[Watcher] Fix flaky functional test #56393

jloleysens · 2020-01-30T15:39:09Z

Summary

Fixes #56014 introduced with #54752.

Additional Context

See meta this issue: #55985

Related Issues:

Failing test: Firefox XPack UI Functional Tests.x-pack/test/functional/apps/watcher/watcher_test·js - watcher app watcher_test create and save a new watch #56014

Previous work:

TODOs before review

~~Currently this PR is still trying to figure out the cause of the flakiness and is not ready to be merged. Consider the current changes all to be tentative (i.e., this should be a draft PR).~~

elasticmachine · 2020-01-30T15:39:12Z

Pinging @elastic/es-ui (Team:Elasticsearch UI)

Also only update data (and call serializer) on a success response, not on an error response.

cuff-links · 2020-01-31T17:59:02Z

i appreciate you getting the jump on this issue, @jloleysens.

cuff-links · 2020-01-31T18:00:52Z

x-pack/test/functional/apps/watcher/watcher_test.js

@@ -35,6 +35,8 @@ export default function({ getService, getPageObjects }) {
      await browser.setWindowSize(1600, 1000);
      // TODO: Remove the retry.try wrapper once https://github.com/elastic/kibana/issues/55985 is resolved
      retry.try(async () => {
+        // Try to give the license time to come through.
+        await PageObjects.common.sleep(2000);


We need to remove this hard coded sleep. It's been my experience that this sometimes fails in CI. We might want to try using the retry.waitFor() function and figure out a reasonable condition to wait for. We can wait for anything that can return true of false so if we need to wait for a license to be changed or something, we can hit the license API using the ES client. What do you think about that?

Hey @cuff-links , that sounds reasonable :)

…ry.waitFor

jloleysens · 2020-02-04T10:24:55Z

@elasticmachine merge upstream

jloleysens · 2020-02-05T11:44:25Z

@elasticmachine merge upstream

cuff-links · 2020-02-06T23:55:34Z

x-pack/test/functional/apps/watcher/watcher_test.js

+      //
+      // See this issue https://github.com/elastic/kibana/issues/55985 for future resolution.
+      await retry.waitFor('watcher to display in management UI', async () => {
+        try {


Hi, Jean-Louis. This is looking good so far. You can remove this extra try because retry.waitFor() is just a wrapper function around try. So you don't need the second one. Other than that, we I think we are looking good.

Ahh. Gotcha. I reran the test and you are correct. LGTM.

jloleysens · 2020-02-10T08:47:26Z

Thanks for taking a look again John! The behavior without the try is different, though. If the element is not found by webdriver it throws. This causes the wait for loop to also throw, not retrying. Returning a Boolean here gives us the wait for behaviour of retrying until a condition becomes true. Let me know if this could be achieved differently though!

…

On Fri, 07 Feb 2020 at 00:55, John Dorlus ***@***.***> wrote: ***@***.**** requested changes on this pull request. ------------------------------ In x-pack/test/functional/apps/watcher/watcher_test.js <#56393 (comment)>: > - retry.try(async () => { - // Try to give the license time to come through. - await PageObjects.common.sleep(2000); - await PageObjects.common.navigateToApp('watcher'); - await testSubjects.find('createWatchButton'); + + // License values are emitted ES -> Kibana Server -> Kibana Public. The current implementation + // creates a situation the Watcher plugin may not have received a minimum required license at setup time + // so the public app may not have registered in the UI. + // + // For functional testing this is a problem. The temporary solution is we wait for watcher + // to be visible. + // + // See this issue #55985 for future resolution. + await retry.waitFor('watcher to display in management UI', async () => { + try { Hi, Jean-Louis. This is looking good so far. You can remove this extra try because retry.waitFor() is just a wrapper function around try. So you don't need the second one. Other than that, we I think we are looking good. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#56393?email_source=notifications&email_token=AB6G67ARQZDCG7CCVHRFAK3RBSPP5A5CNFSM4KNYEM5KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCUTKM2Q#pullrequestreview-354854506>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB6G67FSILXDPKRYRZ3YR5TRBSPP5ANCNFSM4KNYEM5A> .

cuff-links · 2020-02-11T00:40:20Z

x-pack/test/functional/apps/watcher/watcher_test.js

+      //
+      // See this issue https://github.com/elastic/kibana/issues/55985 for future resolution.
+      await retry.waitFor('watcher to display in management UI', async () => {
+        try {


Ahh. Gotcha. I reran the test and you are correct. LGTM.

cuff-links · 2020-02-11T00:40:43Z

@elasticmachine merge upstream

* Give a bit more time for machines on CI * Remove unnecessary sleep * Dummy error logs [do not commit to master] * Revert "Dummy error logs [do not commit to master]" Also only update data (and call serializer) on a success response, not on an error response. * Remove common.sleep and rewrite the comment explaining the use of retry.waitFor * Fix typo Co-authored-by: Elastic Machine <[email protected]>

kibanamachine · 2020-02-11T10:06:32Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 0becbe6

History

💚 Build #25785 succeeded 0becbe6
💔 Build #24889 failed f961b50
💚 Build #24550 succeeded 7128fc1
💚 Build #24261 succeeded 9dc7316
💚 Build #23729 succeeded 42c1d72

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

* Give a bit more time for machines on CI * Remove unnecessary sleep * Dummy error logs [do not commit to master] * Revert "Dummy error logs [do not commit to master]" Also only update data (and call serializer) on a success response, not on an error response. * Remove common.sleep and rewrite the comment explaining the use of retry.waitFor * Fix typo Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

jloleysens added 2 commits January 30, 2020 16:33

Give a bit more time for machines on CI

8779efe

Remove unnecessary sleep

ca9f7a7

jloleysens added Feature:Watcher v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Jan 30, 2020

jloleysens requested review from sebelga and removed request for sebelga January 30, 2020 15:39

jloleysens added 2 commits January 31, 2020 15:04

Dummy error logs [do not commit to master]

8d46c8f

Revert "Dummy error logs [do not commit to master]"

42c1d72

Also only update data (and call serializer) on a success response, not on an error response.

cuff-links self-requested a review January 31, 2020 17:58

cuff-links suggested changes Jan 31, 2020

View reviewed changes

Remove common.sleep and rewrite the comment explaining the use of ret…

6238f13

…ry.waitFor

Merge branch 'master' into fix/watcher/flaky-test

9dc7316

elasticmachine and others added 2 commits February 5, 2020 06:44

Merge branch 'master' into fix/watcher/flaky-test

7128fc1

Fix typo

f961b50

cuff-links suggested changes Feb 6, 2020

View reviewed changes

cuff-links self-requested a review February 11, 2020 00:39

cuff-links approved these changes Feb 11, 2020

View reviewed changes

Merge branch 'master' into fix/watcher/flaky-test

0becbe6

jloleysens merged commit b67b1c3 into elastic:master Feb 11, 2020

jloleysens deleted the fix/watcher/flaky-test branch February 11, 2020 08:39

jloleysens mentioned this pull request Feb 11, 2020

[7.x] [Watcher] Fix flaky functional test (#56393) #57292

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Watcher] Fix flaky functional test #56393

[Watcher] Fix flaky functional test #56393

jloleysens commented Jan 30, 2020 •

edited

Loading

elasticmachine commented Jan 30, 2020

cuff-links commented Jan 31, 2020

cuff-links Jan 31, 2020 •

edited

Loading

jloleysens Feb 1, 2020

jloleysens commented Feb 4, 2020

jloleysens commented Feb 5, 2020

cuff-links Feb 6, 2020

cuff-links Feb 11, 2020

jloleysens commented Feb 10, 2020 via email

cuff-links Feb 11, 2020

cuff-links commented Feb 11, 2020

kibanamachine commented Feb 11, 2020

[Watcher] Fix flaky functional test #56393

[Watcher] Fix flaky functional test #56393

Conversation

jloleysens commented Jan 30, 2020 • edited Loading

Summary

Additional Context

TODOs before review

elasticmachine commented Jan 30, 2020

cuff-links commented Jan 31, 2020

cuff-links Jan 31, 2020 • edited Loading

Choose a reason for hiding this comment

jloleysens Feb 1, 2020

Choose a reason for hiding this comment

jloleysens commented Feb 4, 2020

jloleysens commented Feb 5, 2020

cuff-links Feb 6, 2020

Choose a reason for hiding this comment

cuff-links Feb 11, 2020

Choose a reason for hiding this comment

jloleysens commented Feb 10, 2020 via email

cuff-links Feb 11, 2020

Choose a reason for hiding this comment

cuff-links commented Feb 11, 2020

kibanamachine commented Feb 11, 2020

💚 Build Succeeded

History

jloleysens commented Jan 30, 2020 •

edited

Loading

cuff-links Jan 31, 2020 •

edited

Loading