Skip to content

Commit

Permalink
Merge branch 'main' into tm/184173
Browse files Browse the repository at this point in the history
  • Loading branch information
elasticmachine committed Aug 26, 2024
2 parents 6e2adcc + d1cc0cb commit aa5233b
Show file tree
Hide file tree
Showing 98 changed files with 2,072 additions and 458 deletions.
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
21 changes: 21 additions & 0 deletions packages/kbn-ftr-common-functional-ui-services/services/browser.ts
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
1 change: 1 addition & 0 deletions x-pack/plugins/fleet/common/types/models/agent_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ export interface FullAgentPolicyMonitoring {
enabled: boolean;
metrics: boolean;
logs: boolean;
traces: boolean;
}

export interface FullAgentPolicy {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ export const AgentDiagnosticsTab: React.FunctionComponent<AgentDiagnosticsProps>
<p>
<FormattedMessage
id="xpack.fleet.requestDiagnostics.calloutText"
defaultMessage="Diagnostics files are stored in Elasticsearch, and as such can incur storage costs. By default, files are periodically deleted via an ILM policy."
defaultMessage="Consider changing the log level to debug before requesting a diagnostic. Diagnostics files are stored in Elasticsearch, and as such can incur storage costs. By default, files are deleted periodically through an ILM policy."
/>
</p>
</EuiText>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export const AgentRequestDiagnosticsModal: React.FunctionComponent<Props> = ({
<p>
<FormattedMessage
id="xpack.fleet.requestDiagnostics.description"
defaultMessage="Diagnostics files are stored in Elasticsearch, and as such can incur storage costs. By default, files are periodically deleted via an ILM policy."
defaultMessage="Consider changing the log level to debug before requesting a diagnostic. Diagnostics files are stored in Elasticsearch, and as such can incur storage costs. By default, files are deleted periodically through an ILM policy."
/>
</p>
<p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ describe('Fleet cloud preconfiguration', () => {
enabled: false,
logs: false,
metrics: false,
traces: false,
},
protection: {
enabled: false,
Expand Down
Loading

0 comments on commit aa5233b

Please sign in to comment.