From 8779efeddd68be94e51085b3717a8590120bc993 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Thu, 30 Jan 2020 15:56:07 +0100 Subject: [PATCH 1/6] Give a bit more time for machines on CI --- x-pack/test/functional/apps/watcher/watcher_test.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x-pack/test/functional/apps/watcher/watcher_test.js b/x-pack/test/functional/apps/watcher/watcher_test.js index a3b955f8fccee..2fe4a6f5e939f 100644 --- a/x-pack/test/functional/apps/watcher/watcher_test.js +++ b/x-pack/test/functional/apps/watcher/watcher_test.js @@ -35,12 +35,15 @@ 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); await PageObjects.common.navigateToApp('watcher'); await testSubjects.find('createWatchButton'); }); }); it('create and save a new watch', async () => { + await PageObjects.common.sleep(500); await PageObjects.watcher.createWatch(watchID, watchName); const watch = await PageObjects.watcher.getWatch(watchID); expect(watch.id).to.be(watchID); From ca9f7a73b47d80e6a7b9021647c554ecafb10710 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Thu, 30 Jan 2020 16:22:00 +0100 Subject: [PATCH 2/6] Remove unnecessary sleep --- x-pack/test/functional/apps/watcher/watcher_test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/test/functional/apps/watcher/watcher_test.js b/x-pack/test/functional/apps/watcher/watcher_test.js index 2fe4a6f5e939f..60bdab70ef881 100644 --- a/x-pack/test/functional/apps/watcher/watcher_test.js +++ b/x-pack/test/functional/apps/watcher/watcher_test.js @@ -43,7 +43,6 @@ export default function({ getService, getPageObjects }) { }); it('create and save a new watch', async () => { - await PageObjects.common.sleep(500); await PageObjects.watcher.createWatch(watchID, watchName); const watch = await PageObjects.watcher.getWatch(watchID); expect(watch.id).to.be(watchID); From 8d46c8f970c9f4ad5072e993930a4026a47f89cc Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 31 Jan 2020 15:04:49 +0100 Subject: [PATCH 3/6] Dummy error logs [do not commit to master] --- x-pack/plugins/watcher/public/application/app.tsx | 6 +++++- x-pack/plugins/watcher/public/plugin.ts | 7 +++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/watcher/public/application/app.tsx b/x-pack/plugins/watcher/public/application/app.tsx index 7ca79bb558baa..86f2aa17cc9a7 100644 --- a/x-pack/plugins/watcher/public/application/app.tsx +++ b/x-pack/plugins/watcher/public/application/app.tsx @@ -57,7 +57,11 @@ export const App = (deps: AppDeps) => { const [{ valid, message }, setLicenseStatus] = useState({ valid: true }); useEffect(() => { - const s = deps.licenseStatus$.subscribe(setLicenseStatus); + const s = deps.licenseStatus$.subscribe(licenseStatus => { + // eslint-disable-next-line no-console + console.error('got license status', JSON.stringify(licenseStatus, null, 2)); + setLicenseStatus(licenseStatus); + }); return () => s.unsubscribe(); }, [deps.licenseStatus$]); diff --git a/x-pack/plugins/watcher/public/plugin.ts b/x-pack/plugins/watcher/public/plugin.ts index a2a0d3cf1c978..d9b3a76516edb 100644 --- a/x-pack/plugins/watcher/public/plugin.ts +++ b/x-pack/plugins/watcher/public/plugin.ts @@ -76,18 +76,17 @@ export class WatcherUIPlugin implements Plugin { }), icon: 'watchesApp', path: '/app/kibana#/management/elasticsearch/watcher/watches', - showOnHomePage: true, + showOnHomePage: false, }; home.featureCatalogue.register(watcherHome); licensing.license$.pipe(first(), map(licenseToLicenseStatus)).subscribe(({ valid }) => { + // eslint-disable-next-line no-console + console.error('Got license', valid); if (valid) { watcherESApp.enable(); watcherHome.showOnHomePage = true; - } else { - watcherESApp.disable(); - watcherHome.showOnHomePage = false; } }); } From 42c1d72881bbf693bce67457b1f8242998cbb29e Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Fri, 31 Jan 2020 15:46:20 +0100 Subject: [PATCH 4/6] 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. --- .../es_ui_shared/public/request/np_ready_request.ts | 8 ++++++-- x-pack/plugins/watcher/public/application/app.tsx | 6 +----- x-pack/plugins/watcher/public/plugin.ts | 2 -- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/plugins/es_ui_shared/public/request/np_ready_request.ts b/src/plugins/es_ui_shared/public/request/np_ready_request.ts index b8f7db1463ab8..790e29b6d3655 100644 --- a/src/plugins/es_ui_shared/public/request/np_ready_request.ts +++ b/src/plugins/es_ui_shared/public/request/np_ready_request.ts @@ -120,7 +120,6 @@ export const useRequest = ( const response = await sendRequest(httpClient, requestBody); const { data: serializedResponseData, error: responseError } = response; - const responseData = deserializer(serializedResponseData); // If an outdated request has resolved, DON'T update state, but DO allow the processData handler // to execute side effects like update telemetry. @@ -129,7 +128,12 @@ export const useRequest = ( } setError(responseError); - setData(responseData); + + if (!responseError) { + const responseData = deserializer(serializedResponseData); + setData(responseData); + } + setIsLoading(false); setIsInitialRequest(false); diff --git a/x-pack/plugins/watcher/public/application/app.tsx b/x-pack/plugins/watcher/public/application/app.tsx index 86f2aa17cc9a7..7ca79bb558baa 100644 --- a/x-pack/plugins/watcher/public/application/app.tsx +++ b/x-pack/plugins/watcher/public/application/app.tsx @@ -57,11 +57,7 @@ export const App = (deps: AppDeps) => { const [{ valid, message }, setLicenseStatus] = useState({ valid: true }); useEffect(() => { - const s = deps.licenseStatus$.subscribe(licenseStatus => { - // eslint-disable-next-line no-console - console.error('got license status', JSON.stringify(licenseStatus, null, 2)); - setLicenseStatus(licenseStatus); - }); + const s = deps.licenseStatus$.subscribe(setLicenseStatus); return () => s.unsubscribe(); }, [deps.licenseStatus$]); diff --git a/x-pack/plugins/watcher/public/plugin.ts b/x-pack/plugins/watcher/public/plugin.ts index d9b3a76516edb..354edd2078676 100644 --- a/x-pack/plugins/watcher/public/plugin.ts +++ b/x-pack/plugins/watcher/public/plugin.ts @@ -82,8 +82,6 @@ export class WatcherUIPlugin implements Plugin { home.featureCatalogue.register(watcherHome); licensing.license$.pipe(first(), map(licenseToLicenseStatus)).subscribe(({ valid }) => { - // eslint-disable-next-line no-console - console.error('Got license', valid); if (valid) { watcherESApp.enable(); watcherHome.showOnHomePage = true; From 6238f135d38b52c81460eda0410b03e1370855c0 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Tue, 4 Feb 2020 11:23:39 +0100 Subject: [PATCH 5/6] Remove common.sleep and rewrite the comment explaining the use of retry.waitFor --- .../functional/apps/watcher/watcher_test.js | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/x-pack/test/functional/apps/watcher/watcher_test.js b/x-pack/test/functional/apps/watcher/watcher_test.js index 60bdab70ef881..358895870f8f1 100644 --- a/x-pack/test/functional/apps/watcher/watcher_test.js +++ b/x-pack/test/functional/apps/watcher/watcher_test.js @@ -33,12 +33,23 @@ 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); - 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 https://github.com/elastic/kibana/issues/55985 for future resolution. + await retry.waitFor('watcher to display in management UI', async () => { + try { + await PageObjects.common.navigateToApp('watcher'); + await testSubjects.find('createWatchButton'); + } catch (e) { + return false; + } + return true; }); }); From f961b5097a4498967db5a618d301a6c5a6178ce8 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Thu, 6 Feb 2020 13:23:33 +0100 Subject: [PATCH 6/6] Fix typo --- x-pack/test/functional/apps/watcher/watcher_test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/test/functional/apps/watcher/watcher_test.js b/x-pack/test/functional/apps/watcher/watcher_test.js index 1747d6655a1c4..d96426997ca8b 100644 --- a/x-pack/test/functional/apps/watcher/watcher_test.js +++ b/x-pack/test/functional/apps/watcher/watcher_test.js @@ -38,13 +38,13 @@ export default function({ getService, getPageObjects }) { await browser.setWindowSize(1600, 1000); // 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 + // creates a situation where 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 https://github.com/elastic/kibana/issues/55985 for future resolution. + // See this issue https://github.com/elastic/kibana/issues/55985. await retry.waitFor('watcher to display in management UI', async () => { try { await PageObjects.common.navigateToApp('watcher');