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

[EuiProvider / Functional tests] Check for EuiProvider Dev Warning #189018

Merged
merged 17 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions dev_docs/operations/writing_stable_functional_tests.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<p data-test-sub="core-chrome-euiDevProviderWarning-toast">
<p>
<FormattedMessage
id="core.chrome.euiDevProviderWarning"
defaultMessage="Kibana components must be wrapped in a React Context provider for full functionality and proper theming support. See {link}."
Expand All @@ -201,6 +211,7 @@ export class ChromeService {
/>
</p>
),
'data-test-subj': 'core-chrome-euiDevProviderWarning-toast',
toastLifeTimeMs: 60 * 60 * 1000, // keep message visible for up to an hour
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>}
*/
public async getSessionStorageItem(key: string): Promise<string | null> {
return await this.driver.executeScript<string>(
`return window.sessionStorage.getItem("${key}");`
);
}

/**
* Removes a value in session storage for the focused window/frame.
*
* @param {string} key
* @return {Promise<void>}
*/
public async removeSessionStorageItem(key: string): Promise<void> {
await this.driver.executeScript('return window.sessionStorage.removeItem(arguments[0]);', key);
}

/**
* Clears session storage for the focused window/frame.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>(`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();`);
Expand Down Expand Up @@ -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');
Expand Down
1 change: 1 addition & 0 deletions test/plugin_functional/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
]
}
}
Original file line number Diff line number Diff line change
@@ -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"
}
}
Original file line number Diff line number Diff line change
@@ -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(
<EuiPageTemplate restrictWidth="1000px">
<EuiPageTemplate.Header>
<EuiTitle size="l">
<h1>EuiProvider is missing</h1>
</EuiTitle>
</EuiPageTemplate.Header>
<EuiPageTemplate.Section>
<EuiTitle>
<h2>Goal of this page</h2>
</EuiTitle>
<EuiText>
<p>
The goal of this page is to create a UI that attempts to render EUI React components
without wrapping the rendering tree in EuiProvider.
</p>
</EuiText>
</EuiPageTemplate.Section>
</EuiPageTemplate>,
element
);

return () => ReactDOM.unmountComponentAtNode(element);
};
Original file line number Diff line number Diff line change
@@ -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();
}
Original file line number Diff line number Diff line change
@@ -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<EuiProviderDevWarningPluginSetup, EuiProviderDevWarningPluginStart>
{
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<EuiProviderDevWarningPlugin['setup']>;
export type EuiProviderDevWarningPluginStart = ReturnType<EuiProviderDevWarningPlugin['start']>;
Original file line number Diff line number Diff line change
@@ -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"
]
}
45 changes: 45 additions & 0 deletions test/plugin_functional/test_suites/shared_ux/eui_provider.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
}
15 changes: 15 additions & 0 deletions test/plugin_functional/test_suites/shared_ux/index.ts
Original file line number Diff line number Diff line change
@@ -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'));
});
}
2 changes: 2 additions & 0 deletions tsconfig.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down
4 changes: 4 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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 ""
Expand Down