From dc7e3ec999176bdaf85bc0fbef8799f55f5b7cf8 Mon Sep 17 00:00:00 2001 From: Tim Sullivan Date: Mon, 26 Aug 2024 13:08:32 -0700 Subject: [PATCH] [EuiProvider / Functional tests] Check for EuiProvider Dev Warning (#189018) ## Summary Follows https://github.com/elastic/kibana/pull/184608 Closes https://github.com/elastic/kibana-team/issues/805 ![image](https://github.com/user-attachments/assets/eaee5b81-c1e9-4e81-9018-db57652236dc) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .github/CODEOWNERS | 1 + .../writing_stable_functional_tests.mdx | 4 ++ package.json | 1 + .../src/chrome_service.tsx | 15 ++++++- .../services/browser.ts | 21 +++++++++ .../services/remote/remote.ts | 36 +++++++++++++++ test/plugin_functional/config.ts | 1 + .../eui_provider_dev_warning/kibana.jsonc | 13 ++++++ .../eui_provider_dev_warning/package.json | 14 ++++++ .../public/application.tsx | 38 ++++++++++++++++ .../eui_provider_dev_warning/public/index.ts | 13 ++++++ .../eui_provider_dev_warning/public/plugin.ts | 35 +++++++++++++++ .../eui_provider_dev_warning/tsconfig.json | 18 ++++++++ .../test_suites/shared_ux/eui_provider.ts | 45 +++++++++++++++++++ .../test_suites/shared_ux/index.ts | 15 +++++++ tsconfig.base.json | 2 + yarn.lock | 4 ++ 17 files changed, 274 insertions(+), 2 deletions(-) create mode 100644 test/plugin_functional/plugins/eui_provider_dev_warning/kibana.jsonc create mode 100644 test/plugin_functional/plugins/eui_provider_dev_warning/package.json create mode 100644 test/plugin_functional/plugins/eui_provider_dev_warning/public/application.tsx create mode 100644 test/plugin_functional/plugins/eui_provider_dev_warning/public/index.ts create mode 100644 test/plugin_functional/plugins/eui_provider_dev_warning/public/plugin.ts create mode 100644 test/plugin_functional/plugins/eui_provider_dev_warning/tsconfig.json create mode 100644 test/plugin_functional/test_suites/shared_ux/eui_provider.ts create mode 100644 test/plugin_functional/test_suites/shared_ux/index.ts diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 41b3617cc52b9..277ac73bedf0c 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -425,6 +425,7 @@ src/plugins/esql_datagrid @elastic/kibana-esql packages/kbn-esql-utils @elastic/kibana-esql packages/kbn-esql-validation-autocomplete @elastic/kibana-esql examples/esql_validation_example @elastic/kibana-esql +test/plugin_functional/plugins/eui_provider_dev_warning @elastic/appex-sharedux packages/kbn-event-annotation-common @elastic/kibana-visualizations packages/kbn-event-annotation-components @elastic/kibana-visualizations src/plugins/event_annotation_listing @elastic/kibana-visualizations diff --git a/dev_docs/operations/writing_stable_functional_tests.mdx b/dev_docs/operations/writing_stable_functional_tests.mdx index 9403b9144260d..42aadf702ba92 100644 --- a/dev_docs/operations/writing_stable_functional_tests.mdx +++ b/dev_docs/operations/writing_stable_functional_tests.mdx @@ -75,6 +75,10 @@ await testSubjects.existsOrFail('savedItemDetailPage') Even if you are very careful, the more UI automation you do the more likely you are to make a mistake and write a flaky test. If there is any way to do setup work for your test via the Kibana or Elasticsearch APIs rather than interacting with the UI, then take advantage of that opportunity to write less UI automation. +## Incorrect usage of EUI components in React code will cause a functional test failure + +For EUI to support theming and internationalization, EUI components in your React application must be wrapped in `EuiProvider` (more preferably, use the `KibanaRenderContextProvider` wrapper). The functional test runner treats EUI as a first-class citizen and will throw an error when incorrect usage of EUI is detected. However, experiencing this type of failure in a test run is unlikely: in dev mode, a toast message alerts developers of incorrect EUI usage in real-time. + ## Do you really need a functional test for this? Once you've invested a lot of time and energy into figuring out how to write functional tests well it can be tempting to use them for all sorts of things which might not justify the cost of a functional test. Make sure that your test is validating something that couldn't be validated by a series of unit tests on a component+store+API. diff --git a/package.json b/package.json index 5ece731ab84d0..c07597726ca47 100644 --- a/package.json +++ b/package.json @@ -479,6 +479,7 @@ "@kbn/esql-utils": "link:packages/kbn-esql-utils", "@kbn/esql-validation-autocomplete": "link:packages/kbn-esql-validation-autocomplete", "@kbn/esql-validation-example-plugin": "link:examples/esql_validation_example", + "@kbn/eui-provider-dev-warning": "link:test/plugin_functional/plugins/eui_provider_dev_warning", "@kbn/event-annotation-common": "link:packages/kbn-event-annotation-common", "@kbn/event-annotation-components": "link:packages/kbn-event-annotation-components", "@kbn/event-annotation-listing-plugin": "link:src/plugins/event_annotation_listing", diff --git a/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx b/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx index 5f9a17713861c..0b7f0e8afd958 100644 --- a/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx +++ b/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx @@ -180,14 +180,24 @@ export class ChromeService { if (isDev) { setEuiDevProviderWarning((providerError) => { const errorObject = new Error(providerError.toString()); - // show a stack trace in the console + // 1. show a stack trace in the console // eslint-disable-next-line no-console console.error(errorObject); + // 2. store error in sessionStorage so it can be detected in testing + const storedError = { + message: providerError.toString(), + stack: errorObject.stack ?? 'undefined', + pageHref: window.location.href, + pageTitle: document.title, + }; + sessionStorage.setItem('dev.euiProviderWarning', JSON.stringify(storedError)); + + // 3. error toast / popup notifications.toasts.addDanger({ title: '`EuiProvider` is missing', text: mountReactNode( -

+

), + 'data-test-subj': 'core-chrome-euiDevProviderWarning-toast', toastLifeTimeMs: 60 * 60 * 1000, // keep message visible for up to an hour }); }); diff --git a/packages/kbn-ftr-common-functional-ui-services/services/browser.ts b/packages/kbn-ftr-common-functional-ui-services/services/browser.ts index c017433446d75..8ac46821c60bf 100644 --- a/packages/kbn-ftr-common-functional-ui-services/services/browser.ts +++ b/packages/kbn-ftr-common-functional-ui-services/services/browser.ts @@ -590,6 +590,27 @@ class BrowserService extends FtrService { await this.driver.executeScript('return window.localStorage.clear();'); } + /** + * Adds a value in session storage for the focused window/frame. + * + * @return {Promise} + */ + public async getSessionStorageItem(key: string): Promise { + return await this.driver.executeScript( + `return window.sessionStorage.getItem("${key}");` + ); + } + + /** + * Removes a value in session storage for the focused window/frame. + * + * @param {string} key + * @return {Promise} + */ + public async removeSessionStorageItem(key: string): Promise { + await this.driver.executeScript('return window.sessionStorage.removeItem(arguments[0]);', key); + } + /** * Clears session storage for the focused window/frame. * diff --git a/packages/kbn-ftr-common-functional-ui-services/services/remote/remote.ts b/packages/kbn-ftr-common-functional-ui-services/services/remote/remote.ts index 6eb10984eeb66..a4c45e7b24e7c 100644 --- a/packages/kbn-ftr-common-functional-ui-services/services/remote/remote.ts +++ b/packages/kbn-ftr-common-functional-ui-services/services/remote/remote.ts @@ -18,6 +18,16 @@ export async function RemoteProvider({ getService }: FtrProviderContext) { const browserType: Browsers = config.get('browser.type'); type BrowserStorage = 'sessionStorage' | 'localStorage'; + const getSessionStorageItem = async (key: string) => { + try { + return await driver.executeScript(`return window.sessionStorage.getItem("${key}");`); + } catch (error) { + if (!error.message.includes(`Failed to read the 'sessionStorage' property from 'Window'`)) { + throw error; + } + } + }; + const clearBrowserStorage = async (storageType: BrowserStorage) => { try { await driver.executeScript(`window.${storageType}.clear();`); @@ -93,6 +103,32 @@ export async function RemoteProvider({ getService }: FtrProviderContext) { lifecycle.afterTestSuite.add(async () => { await tryWebDriverCall(async () => { + // collect error message stashed in SessionStorage that indicate EuiProvider implementation error + const euiProviderWarning = await getSessionStorageItem('dev.euiProviderWarning'); + if (euiProviderWarning != null) { + let errorMessage: string; + let errorStack: string; + let pageHref: string; + let pageTitle: string; + try { + ({ + message: errorMessage, + stack: errorStack, + pageHref, + pageTitle, + } = JSON.parse(euiProviderWarning)); + } catch (error) { + throw new Error(`Found EuiProvider dev error, but the details could not be parsed`); + } + + log.error(`pageTitle: ${pageTitle}`); + log.error(`pageHref: ${pageHref}`); + log.error(`Error: ${errorMessage}`); + log.error(`Error stack: ${errorStack}`); + throw new Error(`Found EuiProvider dev error on: ${pageHref}`); + } + + // global cleanup const { width, height } = windowSizeStack.shift()!; await driver.manage().window().setRect({ width, height }); await clearBrowserStorage('sessionStorage'); diff --git a/test/plugin_functional/config.ts b/test/plugin_functional/config.ts index 3f59cc4ff3ac5..5ed34a69bae01 100644 --- a/test/plugin_functional/config.ts +++ b/test/plugin_functional/config.ts @@ -27,6 +27,7 @@ export default async function ({ readConfigFile }: FtrConfigProviderContext) { require.resolve('./test_suites/data_plugin'), require.resolve('./test_suites/saved_objects_management'), require.resolve('./test_suites/saved_objects_hidden_type'), + require.resolve('./test_suites/shared_ux'), ], services: { ...functionalConfig.get('services'), diff --git a/test/plugin_functional/plugins/eui_provider_dev_warning/kibana.jsonc b/test/plugin_functional/plugins/eui_provider_dev_warning/kibana.jsonc new file mode 100644 index 0000000000000..970c25a20d596 --- /dev/null +++ b/test/plugin_functional/plugins/eui_provider_dev_warning/kibana.jsonc @@ -0,0 +1,13 @@ +{ + "type": "plugin", + "id": "@kbn/eui-provider-dev-warning", + "owner": "@elastic/appex-sharedux", + "plugin": { + "id": "euiProviderDevWarning", + "server": false, + "browser": true, + "configPath": [ + "eui_provider_dev_warning" + ] + } +} diff --git a/test/plugin_functional/plugins/eui_provider_dev_warning/package.json b/test/plugin_functional/plugins/eui_provider_dev_warning/package.json new file mode 100644 index 0000000000000..97def81d81579 --- /dev/null +++ b/test/plugin_functional/plugins/eui_provider_dev_warning/package.json @@ -0,0 +1,14 @@ +{ + "name": "@kbn/eui-provider-dev-warning", + "version": "1.0.0", + "main": "target/test/plugin_functional/plugins/eui_provider_dev_warning", + "kibana": { + "version": "kibana", + "templateVersion": "1.0.0" + }, + "license": "SSPL-1.0 OR Elastic License 2.0", + "scripts": { + "kbn": "node ../../../../scripts/kbn.js", + "build": "rm -rf './target' && ../../../../node_modules/.bin/tsc" + } +} diff --git a/test/plugin_functional/plugins/eui_provider_dev_warning/public/application.tsx b/test/plugin_functional/plugins/eui_provider_dev_warning/public/application.tsx new file mode 100644 index 0000000000000..64541566c26a5 --- /dev/null +++ b/test/plugin_functional/plugins/eui_provider_dev_warning/public/application.tsx @@ -0,0 +1,38 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import React from 'react'; +import { EuiPageTemplate, EuiTitle, EuiText } from '@elastic/eui'; +import ReactDOM from 'react-dom'; +import { AppMountParameters, CoreStart } from '@kbn/core/public'; + +export const renderApp = (_core: CoreStart, { element }: AppMountParameters) => { + ReactDOM.render( + + + +

EuiProvider is missing

+ + + + +

Goal of this page

+ + +

+ The goal of this page is to create a UI that attempts to render EUI React components + without wrapping the rendering tree in EuiProvider. +

+ + + , + element + ); + + return () => ReactDOM.unmountComponentAtNode(element); +}; diff --git a/test/plugin_functional/plugins/eui_provider_dev_warning/public/index.ts b/test/plugin_functional/plugins/eui_provider_dev_warning/public/index.ts new file mode 100644 index 0000000000000..8241ab91ba378 --- /dev/null +++ b/test/plugin_functional/plugins/eui_provider_dev_warning/public/index.ts @@ -0,0 +1,13 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { EuiProviderDevWarningPlugin } from './plugin'; + +export function plugin() { + return new EuiProviderDevWarningPlugin(); +} diff --git a/test/plugin_functional/plugins/eui_provider_dev_warning/public/plugin.ts b/test/plugin_functional/plugins/eui_provider_dev_warning/public/plugin.ts new file mode 100644 index 0000000000000..a524ff6c2095c --- /dev/null +++ b/test/plugin_functional/plugins/eui_provider_dev_warning/public/plugin.ts @@ -0,0 +1,35 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { AppMountParameters, CoreSetup, Plugin } from '@kbn/core/public'; + +export class EuiProviderDevWarningPlugin + implements Plugin +{ + public setup(core: CoreSetup) { + core.application.register({ + id: 'euiProviderDevWarning', + title: 'EUI Provider Dev Warning', + async mount(params: AppMountParameters) { + const { renderApp } = await import('./application'); + const [coreStart] = await core.getStartServices(); + coreStart.chrome.docTitle.change('EuiProvider test'); + return renderApp(coreStart, params); + }, + }); + + // Return methods that should be available to other plugins + return {}; + } + + public start() {} + public stop() {} +} + +export type EuiProviderDevWarningPluginSetup = ReturnType; +export type EuiProviderDevWarningPluginStart = ReturnType; diff --git a/test/plugin_functional/plugins/eui_provider_dev_warning/tsconfig.json b/test/plugin_functional/plugins/eui_provider_dev_warning/tsconfig.json new file mode 100644 index 0000000000000..db7cf2bb2089d --- /dev/null +++ b/test/plugin_functional/plugins/eui_provider_dev_warning/tsconfig.json @@ -0,0 +1,18 @@ +{ + "extends": "../../../../tsconfig.base.json", + "compilerOptions": { + "outDir": "target/types" + }, + "include": [ + "index.ts", + "public/**/*.ts", + "public/**/*.tsx", + "../../../../typings/**/*" + ], + "exclude": [ + "target/**/*" + ], + "kbn_references": [ + "@kbn/core" + ] +} diff --git a/test/plugin_functional/test_suites/shared_ux/eui_provider.ts b/test/plugin_functional/test_suites/shared_ux/eui_provider.ts new file mode 100644 index 0000000000000..b378939741f34 --- /dev/null +++ b/test/plugin_functional/test_suites/shared_ux/eui_provider.ts @@ -0,0 +1,45 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import expect from '@kbn/expect'; +import { PluginFunctionalProviderContext } from '../../services'; + +export default function ({ getPageObjects, getService }: PluginFunctionalProviderContext) { + const PageObjects = getPageObjects(['common', 'header']); + const testSubjects = getService('testSubjects'); + const browser = getService('browser'); + + describe('EUI Provider Dev Warning', () => { + it('shows error toast to developer', async () => { + const pageTitle = 'EuiProvider test - Elastic'; + + await PageObjects.common.navigateToApp('euiProviderDevWarning'); + await PageObjects.header.waitUntilLoadingHasFinished(); + expect(await browser.getTitle()).eql(pageTitle); + await testSubjects.existOrFail('core-chrome-euiDevProviderWarning-toast'); + + // check that the error has been detected and stored in session storage + const euiProviderWarning = await browser.getSessionStorageItem('dev.euiProviderWarning'); + const { + message: errorMessage, + stack: errorStack, + pageHref: errorPageHref, + pageTitle: errorPageTitle, + } = JSON.parse(euiProviderWarning!); + expect(errorMessage).to.not.be.empty(); + expect(errorStack).to.not.be.empty(); + expect(errorPageHref).to.not.be.empty(); + expect(errorPageTitle).to.be(pageTitle); + }); + + after(async () => { + // clean up to ensure test suite will pass + await browser.removeSessionStorageItem('dev.euiProviderWarning'); + }); + }); +} diff --git a/test/plugin_functional/test_suites/shared_ux/index.ts b/test/plugin_functional/test_suites/shared_ux/index.ts new file mode 100644 index 0000000000000..a42a855ea4b3f --- /dev/null +++ b/test/plugin_functional/test_suites/shared_ux/index.ts @@ -0,0 +1,15 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { PluginFunctionalProviderContext } from '../../services'; + +export default function ({ loadTestFile }: PluginFunctionalProviderContext) { + describe('SharedUX', () => { + loadTestFile(require.resolve('./eui_provider')); + }); +} diff --git a/tsconfig.base.json b/tsconfig.base.json index b8a8baf855e88..846a51191ab72 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -844,6 +844,8 @@ "@kbn/esql-validation-autocomplete/*": ["packages/kbn-esql-validation-autocomplete/*"], "@kbn/esql-validation-example-plugin": ["examples/esql_validation_example"], "@kbn/esql-validation-example-plugin/*": ["examples/esql_validation_example/*"], + "@kbn/eui-provider-dev-warning": ["test/plugin_functional/plugins/eui_provider_dev_warning"], + "@kbn/eui-provider-dev-warning/*": ["test/plugin_functional/plugins/eui_provider_dev_warning/*"], "@kbn/event-annotation-common": ["packages/kbn-event-annotation-common"], "@kbn/event-annotation-common/*": ["packages/kbn-event-annotation-common/*"], "@kbn/event-annotation-components": ["packages/kbn-event-annotation-components"], diff --git a/yarn.lock b/yarn.lock index 2dcc9ac31d36f..127cdf45ea4ae 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4943,6 +4943,10 @@ version "0.0.0" uid "" +"@kbn/eui-provider-dev-warning@link:test/plugin_functional/plugins/eui_provider_dev_warning": + version "0.0.0" + uid "" + "@kbn/event-annotation-common@link:packages/kbn-event-annotation-common": version "0.0.0" uid ""