From 1712aa92c5c618a58bf81b49ff3606d2bc31ce2d Mon Sep 17 00:00:00 2001 From: Trevor McMaster Date: Tue, 26 Nov 2024 13:18:35 -0700 Subject: [PATCH 1/4] Fix multiple test results bug (#269) * Fixed the bug with multiple results files and switching between - The null pointer throws on redraw. All the calculations and freeing of memory must be done within a setState block while the data is static * Updated eslint to include storybook * Updated eslint to include storybook * Fixed eslint files --- controller/.storybook/main.ts | 20 +++--- controller/.storybook/preview.js | 4 +- controller/.storybook/tsconfig.json | 2 + controller/components/TestResults/index.tsx | 67 ++++++++++++------- controller/test/singleTestResult.js | 2 +- eslint.config.mjs | 6 +- .../src/components/TestResults/index.tsx | 42 +++++++----- 7 files changed, 85 insertions(+), 58 deletions(-) diff --git a/controller/.storybook/main.ts b/controller/.storybook/main.ts index 7400b6d4..e224693c 100644 --- a/controller/.storybook/main.ts +++ b/controller/.storybook/main.ts @@ -20,22 +20,22 @@ const config: StorybookConfig = { propFilter: prop => prop.parent ? !/node_modules/.test(prop.parent.fileName) : true } }, - webpackFinal: (config: Configuration) => { - if (!config.resolve) { config.resolve = {}; } - config.resolve.fallback = { - ...(config.resolve.fallback || {}), + webpackFinal: (webpackConfig: Configuration) => { + if (!webpackConfig.resolve) { webpackConfig.resolve = {}; } + webpackConfig.resolve.fallback = { + ...(webpackConfig.resolve.fallback || {}), "fs": false, "util": false, "path": false, "assert": false, - "crypto": false, + "crypto": false }; - if (!config.experiments) { config.experiments = {}; } - config.experiments.asyncWebAssembly = true; - if (!config.output) { config.output = {}; } + if (!webpackConfig.experiments) { webpackConfig.experiments = {}; } + webpackConfig.experiments.asyncWebAssembly = true; + if (!webpackConfig.output) { webpackConfig.output = {}; } // config.output.publicPath = "/"; - config.output.webassemblyModuleFilename = 'static/wasm/[modulehash].wasm'; - return config; + webpackConfig.output.webassemblyModuleFilename = "static/wasm/[modulehash].wasm"; + return webpackConfig; }, framework: { name: "@storybook/nextjs", diff --git a/controller/.storybook/preview.js b/controller/.storybook/preview.js index b8840744..f43ce9a6 100644 --- a/controller/.storybook/preview.js +++ b/controller/.storybook/preview.js @@ -5,8 +5,8 @@ import { RouterContext } from "next/dist/shared/lib/router-context.shared-runtim /** @type { import('@storybook/react').Preview } */ const preview = { nextRouter: { - Provider: RouterContext.Provider, - }, + Provider: RouterContext.Provider + } // Can't get this to work // https://storybook.js.org/blog/integrate-nextjs-and-storybook-automatically/ // nextjs: { diff --git a/controller/.storybook/tsconfig.json b/controller/.storybook/tsconfig.json index 0ef833af..1859a57e 100644 --- a/controller/.storybook/tsconfig.json +++ b/controller/.storybook/tsconfig.json @@ -17,6 +17,8 @@ "jsx": "react" }, "include": [ + "./*.ts", + "./*.js", "../types/**/*.ts", "../components/**/*.ts", "../components/**/*.tsx" diff --git a/controller/components/TestResults/index.tsx b/controller/components/TestResults/index.tsx index 8aa6254b..da9e9346 100644 --- a/controller/components/TestResults/index.tsx +++ b/controller/components/TestResults/index.tsx @@ -69,12 +69,17 @@ export interface TestResultProps { export interface TestResultState { defaultMessage: string; + /** Filters the results Data by a tag equalling this value. I.e. 'method', 'url', '_id' */ summaryTagFilter: string; + /** Filters the results Data by a summaryTagFilter's value containing this value */ summaryTagValueFilter: string; resultsPath: string | undefined; resultsText: string | undefined; + /** All endpoints from the results file */ resultsData: ParsedFileEntry[] | undefined; + /** Filtered list based on the values from summaryTagFilter and summaryTagValueFilter */ filteredData: ParsedFileEntry[] | undefined; + /** Overall merged stats from all filteredData */ summaryData: ParsedFileEntry | undefined; minMaxTime: MinMaxTime | undefined; error: string | undefined; @@ -115,7 +120,6 @@ const minMaxTime = (testResults: any) => { let startTime2 = Infinity; let endTime2 = -Infinity; - for (const [_, dataPoints] of testResults) { for (const point of dataPoints) { if (point.startTime) { @@ -183,6 +187,7 @@ const freeHistograms = (resultsData: ParsedFileEntry[] | undefined, summaryData: } } } + log("freeHistograms finished", LogLevel.DEBUG, { resultsData: resultsData?.length || -1, summaryData: summaryData !== undefined ? 1 : 0 }); }; const mergeAllDataPoints = (...dataPoints: DataPoint[]): DataPoint[] => { @@ -211,7 +216,7 @@ const getFilteredEndpoints = ({ }): ParsedFileEntry[] | undefined => { if (!summaryTagFilter) { log("getFilteredEndpoints no filter", LogLevel.DEBUG, { summaryTagFilter }); - return resultsData; + return undefined; } if (resultsData && resultsData.length > 0) { const filteredEntries: ParsedFileEntry[] = []; @@ -314,8 +319,6 @@ export const TestResults = ({ testData }: TestResultProps) => { .map((s) => JSON.parse(s)); const model = await import("./model"); let resultsData: ParsedFileEntry[]; - // Free the old ones - freeHistograms(state.resultsData, state.summaryData); const testStartKeys = ["test", "bin", "bucketSize"]; const isOnlyTestStart: boolean = results.length === 1 && Object.keys(results[0]).length === testStartKeys.length @@ -328,22 +331,28 @@ export const TestResults = ({ testData }: TestResultProps) => { // new stats format resultsData = model.processNewJson(results); } + setState((oldState: TestResultState) => { + // Free the old ones + freeHistograms(oldState.resultsData, oldState.summaryData); + const startEndTime: MinMaxTime = minMaxTime(resultsData); - const { summaryTagFilter, summaryTagValueFilter } = state; - const filteredData = getFilteredEndpoints({ resultsData: state.resultsData, summaryTagFilter, summaryTagValueFilter }); + const { summaryTagFilter, summaryTagValueFilter } = oldState; + const filteredData = getFilteredEndpoints({ resultsData, summaryTagFilter, summaryTagValueFilter }); const summaryData = getSummaryData({ filteredData: filteredData || resultsData, summaryTagFilter, summaryTagValueFilter }); log("updateResultsData", LogLevel.DEBUG, { filteredData: filteredData?.length, resultsData: resultsData?.length, summaryData }); - updateState({ + return { + ...oldState, resultsData, filteredData, resultsText, summaryData, error: undefined, minMaxTime: startEndTime + }; }); } catch (error) { - log("Error Fetching Data: " + state.resultsPath, LogLevel.ERROR, error); + log("Error Fetching Data: " + s3ResultPath, LogLevel.ERROR, error); updateState({ error: formatError(error) }); @@ -361,17 +370,20 @@ export const TestResults = ({ testData }: TestResultProps) => { if (event.target.selectedIndex !== 0) { await fetchData(event.target.value); } else { - // Free the old data - freeHistograms(state.resultsData, state.summaryData); - updateState({ - defaultMessage: defaultMessage(), - resultsPath: undefined, - resultsData: undefined, - filteredData: undefined, - resultsText: undefined, - summaryData: undefined, - error: undefined, - minMaxTime: undefined + setState((oldState: TestResultState) => { + // Free the old data + freeHistograms(oldState.resultsData, oldState.summaryData); + return { + ...oldState, + defaultMessage: defaultMessage(), + resultsPath: undefined, + resultsData: undefined, + filteredData: undefined, + resultsText: undefined, + summaryData: undefined, + error: undefined, + minMaxTime: undefined + }; }); } }; @@ -404,13 +416,16 @@ export const TestResults = ({ testData }: TestResultProps) => { } log("filteredData changed", LogLevel.DEBUG, { oldFilteredData, filteredData }); - // Free the old data (only the summary) - freeHistograms(undefined, state.summaryData); - const summaryData = getSummaryData({ filteredData: filteredData || state.resultsData, summaryTagFilter, summaryTagValueFilter }); - updateState({ - [stateName]: newValue, - filteredData, - summaryData + setState((oldState: TestResultState) => { + // Free the old data (only the summary) + freeHistograms(undefined, oldState.summaryData); + const summaryData = getSummaryData({ filteredData: filteredData || oldState.resultsData, summaryTagFilter, summaryTagValueFilter }); + return { + ...oldState, + [stateName]: newValue, + filteredData, + summaryData + }; }); }; diff --git a/controller/test/singleTestResult.js b/controller/test/singleTestResult.js index c0d5bfa4..a514a1fc 100644 --- a/controller/test/singleTestResult.js +++ b/controller/test/singleTestResult.js @@ -199,4 +199,4 @@ export const testJsonResponse = [ } ] ] -] \ No newline at end of file +]; \ No newline at end of file diff --git a/eslint.config.mjs b/eslint.config.mjs index bcd1ad44..b1cd3949 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -25,8 +25,7 @@ export default [{ "common/lib", "controller/lib", "controller/storybook-static", - "controller/.storybook", - "controller/**/**.js", + "controller/test/**.js", "controller/next-env.d.ts", "eslint.config.mjs" ], @@ -50,7 +49,8 @@ export default [{ "./common/tsconfig.json", "./agent/tsconfig.json", "./controller/tsconfig.json", - "./controller/tsconfig.test.json" + "./controller/tsconfig.test.json", + "./controller/.storybook/tsconfig.json", ], }, }, diff --git a/guide/results-viewer-react/src/components/TestResults/index.tsx b/guide/results-viewer-react/src/components/TestResults/index.tsx index 2a913ffb..7e00c280 100644 --- a/guide/results-viewer-react/src/components/TestResults/index.tsx +++ b/guide/results-viewer-react/src/components/TestResults/index.tsx @@ -155,6 +155,7 @@ const minMaxTime = (testResults: any) => { const startTime3: Date = new Date(startTime2); const endTime3: Date = new Date(endTime2); + const includeDateWithStart = startTime3.toLocaleDateString() === endTime3.toLocaleDateString(); testTimes.startTime = dateToString(startTime3, includeDateWithStart); testTimes.endTime = dateToString(endTime3, false); @@ -196,6 +197,7 @@ const freeHistograms = (resultsData: ParsedFileEntry[] | undefined, summaryData: } } } + log("freeHistograms finished", LogLevel.DEBUG, { resultsData: resultsData?.length || -1, summaryData: summaryData !== undefined ? 1 : 0 }); }; const mergeAllDataPoints = (...dataPoints: DataPoint[]): DataPoint[] => { @@ -224,7 +226,7 @@ const getFilteredEndpoints = ({ }): ParsedFileEntry[] | undefined => { if (!summaryTagFilter) { log("getFilteredEndpoints no filter", LogLevel.DEBUG, { summaryTagFilter }); - return resultsData; + return undefined; } if (resultsData && resultsData.length > 0) { const filteredEntries: ParsedFileEntry[] = []; @@ -300,19 +302,17 @@ export const TestResults = ({ resultsText }: TestResultProps) => { const updateState = (newState: Partial) => setState((oldState: TestResultState) => ({ ...oldState, ...newState })); - const updateResultsData = async (resultsPath: string): Promise => { + const updateResultsData = async (text: string): Promise => { updateState({ defaultMessage: "Results Loading..." }); try { // if there are multiple jsons (new format), split them up and parse them separately - const results = resultsPath.replace(/}{/g, "}\n{") + const results = text.replace(/}{/g, "}\n{") .split("\n") .map((s) => JSON.parse(s)); const model = await import("./model"); let resultsData: ParsedFileEntry[]; - // Free the old ones - freeHistograms(state.resultsData, state.summaryData); const testStartKeys = ["test", "bin", "bucketSize"]; const isOnlyTestStart: boolean = results.length === 1 && Object.keys(results[0]).length === testStartKeys.length @@ -325,19 +325,25 @@ export const TestResults = ({ resultsText }: TestResultProps) => { // new stats format resultsData = model.processNewJson(results); } + setState((oldState: TestResultState) => { + // Free the old ones + freeHistograms(oldState.resultsData, oldState.summaryData); + const startEndTime: MinMaxTime = minMaxTime(resultsData); - const { summaryTagFilter, summaryTagValueFilter } = state; - const filteredData = getFilteredEndpoints({ resultsData: state.resultsData, summaryTagFilter, summaryTagValueFilter }); + const { summaryTagFilter, summaryTagValueFilter } = oldState; + const filteredData = getFilteredEndpoints({ resultsData, summaryTagFilter, summaryTagValueFilter }); const summaryData = getSummaryData({ filteredData: filteredData || resultsData, summaryTagFilter, summaryTagValueFilter }); log("updateResultsData", LogLevel.DEBUG, { filteredData: filteredData?.length, resultsData: resultsData?.length, summaryData }); - updateState({ + return { + ...oldState, defaultMessage, resultsData, filteredData, summaryData, error: undefined, minMaxTime: startEndTime + }; }); } catch (error) { log("Error parsing Data", LogLevel.ERROR, error); @@ -376,13 +382,16 @@ export const TestResults = ({ resultsText }: TestResultProps) => { } log("filteredData changed", LogLevel.DEBUG, { oldFilteredData, filteredData }); - // Free the old data (only the summary) - freeHistograms(undefined, state.summaryData); - const summaryData = getSummaryData({ filteredData, summaryTagFilter, summaryTagValueFilter }); - updateState({ - [stateName]: newValue, - filteredData, - summaryData + setState((oldState: TestResultState) => { + // Free the old data (only the summary) + freeHistograms(undefined, oldState.summaryData); + const summaryData = getSummaryData({ filteredData: filteredData || oldState.resultsData, summaryTagFilter, summaryTagValueFilter }); + return { + ...oldState, + [stateName]: newValue, + filteredData, + summaryData + }; }); }; @@ -437,7 +446,7 @@ export const TestResults = ({ resultsText }: TestResultProps) => { })} ) : ( -

{state.defaultMessage}

+

{state.defaultMessage}

)} ); @@ -577,6 +586,7 @@ const Endpoint = ({ bucketId, dataPoints }: EndpointProps) => {
    {Object.entries(bucketId).map(([key, value], idx) => { + if (key !== "method" && key !== "url") { return (
  • From 319f9d5fcfa8b792a71479e7f3c932e38b2deb94 Mon Sep 17 00:00:00 2001 From: blonde-mike Date: Wed, 27 Nov 2024 13:50:47 -0700 Subject: [PATCH 2/4] Michael/perf 1699 (#270) * Updated Request tool to get it online * Removed unused component * Updates to remove lint errors --- guide/results-viewer-react/package-lock.json | 56 ++- guide/results-viewer-react/package.json | 1 + .../src/components/Modal/index.tsx | 19 +- .../components/YamlQuestionBubble/index.tsx | 4 +- .../src/components/YamlUrls/index.tsx | 338 +++++++++++------- .../src/components/YamlUrls/story.tsx | 24 +- 6 files changed, 302 insertions(+), 140 deletions(-) diff --git a/guide/results-viewer-react/package-lock.json b/guide/results-viewer-react/package-lock.json index 6b0ae943..be2e8954 100644 --- a/guide/results-viewer-react/package-lock.json +++ b/guide/results-viewer-react/package-lock.json @@ -10,6 +10,7 @@ "license": "ISC", "dependencies": { "@fs/hdr-histogram-wasm": "file:./lib/hdr-histogram-wasm", + "axios": "^1.7.8", "chart.js": "~4.4.0", "chartjs-adapter-date-fns": "^3.0.0", "date-fns": "^3.0.0", @@ -5400,6 +5401,11 @@ "node": ">=4" } }, + "node_modules/asynckit": { + "version": "0.4.0", + "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", + "integrity": "sha512-Oei9OH4tRh0YqU3GxhX79dM/mwVgvbZJaSNaRk+bshkj0S5cfHcgYakreBjrHwatXKbz+IoIdYLxrKim2MjW0Q==" + }, "node_modules/attr-accept": { "version": "2.2.5", "resolved": "https://registry.npmjs.org/attr-accept/-/attr-accept-2.2.5.tgz", @@ -5425,6 +5431,16 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/axios": { + "version": "1.7.8", + "resolved": "https://registry.npmjs.org/axios/-/axios-1.7.8.tgz", + "integrity": "sha512-Uu0wb7KNqK2t5K+YQyVCLM76prD5sRFjKHbJYCP1J7JFGEQ6nN7HWn9+04LAeiJ3ji54lgS/gZCH1oxyrf1SPw==", + "dependencies": { + "follow-redirects": "^1.15.6", + "form-data": "^4.0.0", + "proxy-from-env": "^1.1.0" + } + }, "node_modules/babel-loader": { "version": "9.2.1", "resolved": "https://registry.npmjs.org/babel-loader/-/babel-loader-9.2.1.tgz", @@ -6357,6 +6373,17 @@ "dev": true, "license": "MIT" }, + "node_modules/combined-stream": { + "version": "1.0.8", + "resolved": "https://registry.npmjs.org/combined-stream/-/combined-stream-1.0.8.tgz", + "integrity": "sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg==", + "dependencies": { + "delayed-stream": "~1.0.0" + }, + "engines": { + "node": ">= 0.8" + } + }, "node_modules/commander": { "version": "8.3.0", "resolved": "https://registry.npmjs.org/commander/-/commander-8.3.0.tgz", @@ -6909,6 +6936,14 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/delayed-stream": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/delayed-stream/-/delayed-stream-1.0.0.tgz", + "integrity": "sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ==", + "engines": { + "node": ">=0.4.0" + } + }, "node_modules/depd": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/depd/-/depd-2.0.0.tgz", @@ -7974,7 +8009,6 @@ "version": "1.15.9", "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.15.9.tgz", "integrity": "sha512-gew4GsXizNgdoRyqmyfMHyAmXsZDk6mHkSxZFCzW9gwlbtOW44CDtYavM+y+72qD/Vq2l550kMF52DT8fOLJqQ==", - "dev": true, "funding": [ { "type": "individual", @@ -8062,6 +8096,19 @@ "node": ">=10" } }, + "node_modules/form-data": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/form-data/-/form-data-4.0.1.tgz", + "integrity": "sha512-tzN8e4TX8+kkxGPK8D5u0FNmjPUjw3lwC9lSLxxoB/+GtsJG91CO8bSWy73APlgAZzZbXEYZJuxjkHH2w+Ezhw==", + "dependencies": { + "asynckit": "^0.4.0", + "combined-stream": "^1.0.8", + "mime-types": "^2.1.12" + }, + "engines": { + "node": ">= 6" + } + }, "node_modules/forwarded": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/forwarded/-/forwarded-0.2.0.tgz", @@ -9524,7 +9571,6 @@ "version": "1.52.0", "resolved": "https://registry.npmjs.org/mime-db/-/mime-db-1.52.0.tgz", "integrity": "sha512-sPU4uV7dYlvtWJxwwxHD0PuihVNiE7TyAbQ5SWxDCB9mUYvOgroQOwYQQOKPJ8CIbE+1ETVlOoK1UC2nU3gYvg==", - "dev": true, "license": "MIT", "engines": { "node": ">= 0.6" @@ -9534,7 +9580,6 @@ "version": "2.1.35", "resolved": "https://registry.npmjs.org/mime-types/-/mime-types-2.1.35.tgz", "integrity": "sha512-ZDY+bPm5zTTF+YpCrAU9nK0UgICYPT0QtT1NZWFv4s++TNkcgVaT0g6+4R2uI4MjQjzysHB1zxuWL50hzaeXiw==", - "dev": true, "license": "MIT", "dependencies": { "mime-db": "1.52.0" @@ -10679,6 +10724,11 @@ "node": ">= 0.10" } }, + "node_modules/proxy-from-env": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/proxy-from-env/-/proxy-from-env-1.1.0.tgz", + "integrity": "sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg==" + }, "node_modules/public-encrypt": { "version": "4.0.3", "resolved": "https://registry.npmjs.org/public-encrypt/-/public-encrypt-4.0.3.tgz", diff --git a/guide/results-viewer-react/package.json b/guide/results-viewer-react/package.json index b3cb6ae8..c35f1e05 100644 --- a/guide/results-viewer-react/package.json +++ b/guide/results-viewer-react/package.json @@ -29,6 +29,7 @@ "homepage": "https://github.com/FamilySearch/pewpew/tree/master/lib/config-wasm#readme", "dependencies": { "@fs/hdr-histogram-wasm": "file:./lib/hdr-histogram-wasm", + "axios": "^1.7.8", "chart.js": "~4.4.0", "chartjs-adapter-date-fns": "^3.0.0", "date-fns": "^3.0.0", diff --git a/guide/results-viewer-react/src/components/Modal/index.tsx b/guide/results-viewer-react/src/components/Modal/index.tsx index 0868b546..d20d0d2c 100644 --- a/guide/results-viewer-react/src/components/Modal/index.tsx +++ b/guide/results-viewer-react/src/components/Modal/index.tsx @@ -92,6 +92,7 @@ interface ModalProps { submitText?: string; children?: React.ReactNode; isReady?: boolean; + scrollable?: boolean; } export interface ModalObject { @@ -134,7 +135,8 @@ export const Modal = forwardRef(({ onSubmit, submitText = "submit", children, - isReady + isReady, + scrollable }: ModalProps, ref: Ref) => { const [display, setDisplay] = useState(false); let windowOffset: number = 0; @@ -177,10 +179,15 @@ export const Modal = forwardRef(({ }; const submit = (event?: React.MouseEvent) => { - const scrollY = document.getElementById("root")!.style.top; - document.getElementById("root")!.style.position = ""; - document.getElementById("root")!.style.top = ""; - window.scrollTo(0, parseInt(scrollY || "0", 10) * -1); + const root = document.getElementById("root"); + if (root) { + const scrollY = root.style.top; + root.style.position = ""; + root.style.top = ""; + window.scrollTo(0, parseInt(scrollY || "0", 10) * -1); + } else { + log("Cannot find element #root", LogLevel.DEBUG); + } if (onSubmit) { onSubmit(event).finally(() => setDisplay(false)); } else { @@ -195,7 +202,7 @@ export const Modal = forwardRef(({ {title && {title}} - {children}{/* Any elements that are children of modal will be rendered here */} + {children}{/* Any elements that are children of modal will be rendered here */} {onSubmit && -

    Endpoint must be in the form "https://www.(url)" or "http://www.(url)"

    - - - - - - changeUrl("hitRate", event.target.value)} name={data.id} value={data.hitRate} id="urlHitrate" title={getHitRateTitle(invalidHitRate)} /> - - - - - - - -
    - - - - - - - - - - {headers.map((header: PewPewHeader, index: number) => { - // This maps out all of the headers uploaded from har file - return ( - - - -
    NameValue
    changeHeader(index, "name", event.target.value)} />