From 603fb0980b4b2ac4510fbf47e33b85ec8b54e132 Mon Sep 17 00:00:00 2001 From: Nathan L Smith Date: Mon, 8 Jun 2020 15:20:25 -0500 Subject: [PATCH] APM Storybook fixes * Resolve core legacy assets in @kbn/storybook webpack configuration * Ignore stories in Jest coverage * Combine effects in Cytoscape component so handlers are always added before events are triggered * Add mock context to ErrorRateAlertTrigger stories * Disable TransactionDurationAlertTrigger stories Changing the Cytoscape effect behavior is necessary because the layout was not being triggered when the final set of elements is provided as props to the component. When this is used in Kibana we're always starting with empty elements and fetching them, but in the Storybook we're starting out with the full elements. --- .../storybook_config/webpack.config.js | 23 ++++++++++++++++++- x-pack/plugins/apm/jest.config.js | 1 + .../app/ServiceMap/Cytoscape.stories.tsx | 2 +- .../components/app/ServiceMap/Cytoscape.tsx | 15 ++++-------- .../ErrorRateAlertTrigger/index.stories.tsx | 23 +++++++++++++------ .../index.stories.tsx | 16 +++++++++---- 6 files changed, 56 insertions(+), 24 deletions(-) diff --git a/packages/kbn-storybook/storybook_config/webpack.config.js b/packages/kbn-storybook/storybook_config/webpack.config.js index 2dd051882bb4b..543bb47656df8 100644 --- a/packages/kbn-storybook/storybook_config/webpack.config.js +++ b/packages/kbn-storybook/storybook_config/webpack.config.js @@ -17,7 +17,7 @@ * under the License. */ -const { resolve } = require('path'); +const { parse, resolve } = require('path'); const webpack = require('webpack'); const { stringifyRequest } = require('loader-utils'); const CopyWebpackPlugin = require('copy-webpack-plugin'); @@ -95,6 +95,27 @@ module.exports = async ({ config }) => { }, }, }, + { + loader: 'resolve-url-loader', + options: { + // If you don't have arguments (_, __) to the join function, the + // resolve-url-loader fails with a loader misconfiguration error. + // + // eslint-disable-next-line no-unused-vars + join: (_, __) => (uri, base) => { + if (!base || !parse(base).dir.includes('legacy')) { + return null; + } + + // URIs on mixins in src/legacy/public/styles need to be resolved. + if (uri.startsWith('ui/assets')) { + return resolve(REPO_ROOT, 'src/core/server/core_app/', uri.replace('ui/', '')); + } + + return null; + }, + }, + }, { loader: 'sass-loader', options: { diff --git a/x-pack/plugins/apm/jest.config.js b/x-pack/plugins/apm/jest.config.js index c3ae694fe8e14..43bdeb583c819 100644 --- a/x-pack/plugins/apm/jest.config.js +++ b/x-pack/plugins/apm/jest.config.js @@ -31,6 +31,7 @@ module.exports = { collectCoverageFrom: [ '**/*.{js,jsx,ts,tsx}', '!**/{__test__,__snapshots__,__examples__,integration_tests,tests}/**', + '!**/*.stories.{js,ts,tsx}', '!**/*.test.{js,ts,tsx}', '!**/dev_docs/**', '!**/e2e/**', diff --git a/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.stories.tsx b/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.stories.tsx index 1c62d3cc03db0..30031a05304bb 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.stories.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.stories.tsx @@ -346,7 +346,7 @@ storiesOf('app/ServiceMap/Cytoscape', module).add( }, }, ]; - return ; + return ; }, { info: { propTables: false, source: false }, diff --git a/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.tsx b/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.tsx index cb908785d64d8..c57d702b9a546 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceMap/Cytoscape.tsx @@ -121,15 +121,6 @@ export function Cytoscape({ const trackApmEvent = useUiTracker({ app: 'apm' }); - // Trigger a custom "data" event when data changes - useEffect(() => { - if (cy) { - cy.remove(cy.elements()); - cy.add(elements); - cy.trigger('data'); - } - }, [cy, elements]); - // Set up cytoscape event handlers useEffect(() => { const resetConnectedEdgeStyle = (node?: cytoscape.NodeSingular) => { @@ -223,6 +214,10 @@ export function Cytoscape({ cy.on('mouseout', 'edge, node', mouseoutHandler); cy.on('select', 'node', selectHandler); cy.on('unselect', 'node', unselectHandler); + + cy.remove(cy.elements()); + cy.add(elements); + cy.trigger('data'); } return () => { @@ -241,7 +236,7 @@ export function Cytoscape({ } clearTimeout(layoutstopDelayTimeout); }; - }, [cy, height, serviceName, trackApmEvent, width]); + }, [cy, elements, height, serviceName, trackApmEvent, width]); return ( diff --git a/x-pack/plugins/apm/public/components/shared/ErrorRateAlertTrigger/index.stories.tsx b/x-pack/plugins/apm/public/components/shared/ErrorRateAlertTrigger/index.stories.tsx index 8f7ed54f91bd0..ebcb1627984ad 100644 --- a/x-pack/plugins/apm/public/components/shared/ErrorRateAlertTrigger/index.stories.tsx +++ b/x-pack/plugins/apm/public/components/shared/ErrorRateAlertTrigger/index.stories.tsx @@ -7,6 +7,11 @@ import { storiesOf } from '@storybook/react'; import React from 'react'; import { ErrorRateAlertTrigger } from '.'; +import { ApmPluginContextValue } from '../../../context/ApmPluginContext'; +import { + mockApmPluginContextValue, + MockApmPluginContextWrapper, +} from '../../../context/ApmPluginContext/MockApmPluginContext'; storiesOf('app/ErrorRateAlertTrigger', module).add('example', () => { const params = { @@ -15,12 +20,16 @@ storiesOf('app/ErrorRateAlertTrigger', module).add('example', () => { }; return ( -
- undefined} - setAlertProperty={() => undefined} - /> -
+ +
+ undefined} + setAlertProperty={() => undefined} + /> +
+ ); }); diff --git a/x-pack/plugins/apm/public/components/shared/TransactionDurationAlertTrigger/index.stories.tsx b/x-pack/plugins/apm/public/components/shared/TransactionDurationAlertTrigger/index.stories.tsx index e2429d1225442..da9adbb8dfead 100644 --- a/x-pack/plugins/apm/public/components/shared/TransactionDurationAlertTrigger/index.stories.tsx +++ b/x-pack/plugins/apm/public/components/shared/TransactionDurationAlertTrigger/index.stories.tsx @@ -3,18 +3,24 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ +// import { storiesOf } from '@storybook/react'; import { cloneDeep, merge } from 'lodash'; -import { storiesOf } from '@storybook/react'; import React from 'react'; import { TransactionDurationAlertTrigger } from '.'; +import { ApmPluginContextValue } from '../../../context/ApmPluginContext'; import { - MockApmPluginContextWrapper, mockApmPluginContextValue, + MockApmPluginContextWrapper, } from '../../../context/ApmPluginContext/MockApmPluginContext'; import { MockUrlParamsContextProvider } from '../../../context/UrlParamsContext/MockUrlParamsContextProvider'; -import { ApmPluginContextValue } from '../../../context/ApmPluginContext'; -storiesOf('app/TransactionDurationAlertTrigger', module).add('example', () => { +// Disabling this because we currently don't have a way to mock `useEnvironments` +// which is used by this component. Using the fetch-mock module should work, but +// our current storybook setup has core-js-related problems when trying to import +// it. +// storiesOf('app/TransactionDurationAlertTrigger', module).add('example', +// eslint-disable-next-line no-unused-expressions +() => { const params = { threshold: 1500, aggregationType: 'avg' as const, @@ -44,4 +50,4 @@ storiesOf('app/TransactionDurationAlertTrigger', module).add('example', () => { ); -}); +};