fix: don't use muteReason, don't parse html

gemini-testing · Nov 26, 2024 · dd1119d · dd1119d
1 parent 4acb011
commit dd1119d
Show file tree

Hide file tree

Showing 7 changed files with 89 additions and 59 deletions.
diff --git a/lib/adapters/test-result/testplane.ts b/lib/adapters/test-result/testplane.ts
@@ -10,8 +10,7 @@ import {
     hasUnrelatedToScreenshotsErrors,
     isImageDiffError,
     isInvalidRefImageError,
-    isNoRefImageError,
-    wrapLinkByTag
+    isNoRefImageError
 } from '../../common-utils';
 import {
     ErrorDetails,
@@ -50,7 +49,7 @@ const getSkipComment = (suite: TestplaneTestResult | TestplaneSuite): string | n
 };
 
 const wrapSkipComment = (skipComment: string | null | undefined): string => {
-    return skipComment ? wrapLinkByTag(skipComment) : 'Unknown reason';
+    return skipComment ?? 'Unknown reason';
 };
 
 const getHistory = (history?: TestplaneTestResult['history']): TestStepCompressed[] => {

diff --git a/lib/common-utils.ts b/lib/common-utils.ts
@@ -107,12 +107,6 @@ export const getRelativeUrl = (absoluteUrl: string): string => {
     }
 };
 
-export const wrapLinkByTag = (text: string): string => {
-    return text.replace(/https?:\/\/[^\s]*/g, (url) => {
-        return `<a target="_blank" href="${url}">${url}</a>`;
-    });
-};
-
 export const mkTestId = (fullTitle: string, browserId: string): string => {
     return fullTitle + '.' + browserId;
 };

diff --git a/lib/static/components/section/section-browser.jsx b/lib/static/components/section/section-browser.jsx
@@ -2,7 +2,6 @@ import React, {Fragment, useContext, useLayoutEffect} from 'react';
 import {last, isEmpty} from 'lodash';
 import {connect} from 'react-redux';
 import {bindActionCreators} from 'redux';
-import Parser from 'html-react-parser';
 import PropTypes from 'prop-types';
 import * as actions from '../../modules/actions';
 import BrowserTitle from './title/browser';
@@ -11,6 +10,7 @@ import Body from './body';
 import {isSkippedStatus} from '../../../common-utils';
 import {sectionStatusResolver} from './utils';
 import {MeasurementContext} from '../measurement-context';
+import {makeLinksClickable} from '@/static/new-ui/utils';
 
 function SectionBrowser(props) {
     const onToggleSection = () => {
@@ -24,7 +24,7 @@ function SectionBrowser(props) {
             <Fragment>
                 [skipped] {props.browser.name}
                 {skipReason && ', reason: '}
-                {skipReason && Parser(skipReason)}
+                {skipReason && makeLinksClickable(skipReason)}
             </Fragment>
         );
     };

diff --git a/lib/static/new-ui/components/MetaInfo/index.tsx b/lib/static/new-ui/components/MetaInfo/index.tsx
@@ -1,16 +1,17 @@
 import path from 'path';
 
 import {DefinitionList} from '@gravity-ui/components';
-import {mapValues, isObject, omitBy, isEmpty} from 'lodash';
+import {isEmpty, isObject, mapValues, omitBy} from 'lodash';
 import React, {ReactNode} from 'react';
 import {connect} from 'react-redux';
 
-import {isUrl, getUrlWithBase, getRelativeUrl} from '@/common-utils';
+import {getRelativeUrl, getUrlWithBase, isUrl} from '@/common-utils';
 import {ResultEntity, State} from '@/static/new-ui/types/store';
 import {HtmlReporterValues} from '@/plugin-api';
 import {ReporterConfig} from '@/types';
 import styles from './index.module.css';
-import Parser from 'html-react-parser';
+import {makeLinksClickable} from '@/static/new-ui/utils';
+import {TestStatus} from '@/constants';
 
 const serializeMetaValues = (metaInfo: Record<string, unknown>): Record<string, string> =>
     mapValues(metaInfo, (v): string => {
@@ -21,9 +22,9 @@ const serializeMetaValues = (metaInfo: Record<string, unknown>): Record<string,
         return v?.toString() ?? 'No value';
     });
 
-interface MetaInfoItem {
+interface MetaInfoItem<T> {
     label: string;
-    content: string;
+    content: T;
     url?: string;
     copyText?: string;
 }
@@ -59,12 +60,12 @@ function MetaInfoInternal(props: MetaInfoInternalProps): ReactNode {
         ...resolveMetaInfoExtenders()
     });
 
-    const metaInfoItems: MetaInfoItem[] = Object.entries(serializedMetaValues).map(([key, value]) => ({
+    const metaInfoItems: MetaInfoItem<string>[] = Object.entries(serializedMetaValues).map(([key, value]) => ({
         label: key,
         content: value
     }));
 
-    const metaInfoItemsWithResolvedUrls = metaInfoItems.map((item) => {
+    const metaInfoItemsWithResolvedUrls: MetaInfoItem<string | React.JSX.Element>[] = metaInfoItems.map((item) => {
         if (item.label === 'url' || metaInfoBaseUrls[item.label] === 'auto') {
             const url = getUrlWithBase(item.content, baseHost);
             return {
@@ -110,21 +111,12 @@ function MetaInfoInternal(props: MetaInfoInternalProps): ReactNode {
         });
     }
 
-    const shouldAddSkipReason = Boolean(!metaInfoItemsWithResolvedUrls.find(item => item.label === 'muteReason'));
-    if (result.skipReason && shouldAddSkipReason) {
-        const reason = Parser(result.skipReason);
-        if (typeof reason === 'string') {
-            metaInfoItemsWithResolvedUrls.push({
-                label: 'skipReason',
-                content: reason
-            });
-        } else if (!Array.isArray(reason) && typeof reason.props.children === 'string' && typeof reason.props.href === 'string') {
-            metaInfoItemsWithResolvedUrls.push({
-                label: 'skipReason',
-                content: reason.props.children,
-                url: reason.props.href
-            });
-        }
+    if (result.status === TestStatus.SKIPPED && result.skipReason) {
+        metaInfoItemsWithResolvedUrls.push({
+            label: 'skipReason',
+            content: makeLinksClickable(result.skipReason),
+            copyText: result.skipReason
+        });
     }
 
     return <DefinitionList className={styles.metaInfo} qa={props.qa} items={
@@ -135,14 +127,14 @@ function MetaInfoInternal(props: MetaInfoInternalProps): ReactNode {
                     content: <a className={styles.metaInfoValue} data-suite-view-link={item.url} target="_blank" href={item.url} rel="noreferrer">
                         {item.content}
                     </a>,
-                    copyText: item.copyText ?? item.content
+                    copyText: item.copyText ?? item.content as string
                 };
             }
 
             return {
                 name: item.label,
                 content: <span className={styles.metaInfoValue}>{item.content}</span>,
-                copyText: item.copyText ?? item.content
+                copyText: item.copyText ?? item.content as string
             };
         })
     }/>;

diff --git a/lib/static/new-ui/features/suites/components/TreeViewItemSubtitle/index.tsx b/lib/static/new-ui/features/suites/components/TreeViewItemSubtitle/index.tsx
@@ -1,13 +1,13 @@
 import classNames from 'classnames';
 import React, {ReactNode} from 'react';
 import stripAnsi from 'strip-ansi';
-import Parser from 'html-react-parser';
 
 import {TreeViewItemData} from '@/static/new-ui/features/suites/components/SuitesPage/types';
 import {ImageWithMagnifier} from '@/static/new-ui/components/ImageWithMagnifier';
 import {ImageEntityFail} from '@/static/new-ui/types/store';
 import styles from './index.module.css';
 import {getAssertViewStatusMessage} from '@/static/new-ui/utils/assert-view-status';
+import {makeLinksClickable} from '@/static/new-ui/utils';
 
 interface TreeViewItemSubtitleProps {
     item: TreeViewItemData;
@@ -19,7 +19,7 @@ interface TreeViewItemSubtitleProps {
 export function TreeViewItemSubtitle(props: TreeViewItemSubtitleProps): ReactNode {
     if (props.item.skipReason) {
         return <div className={styles.skipReasonContainer}>
-            <div className={styles.skipReason}>Skipped ⋅ {Parser(props.item.skipReason)}</div>
+            <div className={styles.skipReason}>Skipped ⋅ {makeLinksClickable(props.item.skipReason)}</div>
         </div>;
     } else if (props.item.images?.length) {
         return <div>

diff --git a/lib/static/new-ui/utils/index.tsx b/lib/static/new-ui/utils/index.tsx
@@ -8,7 +8,7 @@ import {
     CloudCheck
 } from '@gravity-ui/icons';
 import {Spin} from '@gravity-ui/uikit';
-import React from 'react';
+import React, {ReactNode} from 'react';
 
 import {TestStatus} from '@/constants';
 import {ImageFile} from '@/types';
@@ -34,12 +34,6 @@ export const getIconByStatus = (status: TestStatus): React.JSX.Element => {
     return <CircleDashed />;
 };
 
-export const getFullTitleByTitleParts = (titleParts: string[]): string => {
-    const DELIMITER = ' ';
-
-    return titleParts.join(DELIMITER).trim();
-};
-
 export const getImageDisplayedSize = (image: ImageFile): string => `${image.size.width}×${image.size.height}`;
 
 export const stringify = (value: unknown): string => {
@@ -61,3 +55,35 @@ export const stringify = (value: unknown): string => {
 
     return toString(value);
 };
+
+export const makeLinksClickable = (text: string): React.JSX.Element => {
+    const urlRegex = /https?:\/\/[^\s]*/g;
+
+    const parts = text.split(urlRegex);
+    const urls = text.match(urlRegex) || [];
+
+    return <>{
+        parts.reduce((elements, part, index) => {
+            elements.push(part);
+
+            if (urls[index]) {
+                const href = urls[index].startsWith('www.')
+                    ? `http://${urls[index]}`
+                    : urls[index];
+
+                elements.push(
+                    <a
+                        key={index}
+                        href={href}
+                        target="_blank"
+                        rel="noopener noreferrer"
+                    >
+                        {urls[index]}
+                    </a>
+                );
+            }
+
+            return elements;
+        }, [] as ReactNode[])
+    }</>;
+};
diff --git a/test/unit/lib/static/components/section/section-browser.jsx b/test/unit/lib/static/components/section/section-browser.jsx
@@ -1,5 +1,6 @@
+import {expect} from 'chai';
 import React from 'react';
-import {defaults, set} from 'lodash';
+import {defaults} from 'lodash';
 import proxyquire from 'proxyquire';
 import {SUCCESS, SKIPPED, ERROR} from 'lib/constants/test-statuses';
 import {UNCHECKED} from 'lib/constants/checked-statuses';
@@ -30,7 +31,6 @@ describe('<SectionBrowser/>', () => {
 
         SectionBrowser = proxyquire('lib/static/components/section/section-browser', {
             '../../modules/actions': actionsStub,
-            './title/browser-skipped': {default: BrowserSkippedTitle},
             './title/browser': {default: BrowserTitle},
             './body': {default: Body}
         }).default;
@@ -45,12 +45,9 @@ describe('<SectionBrowser/>', () => {
             const resultsById = mkResult({id: 'res', status: SKIPPED});
             const tree = mkStateTree({browsersById, browsersStateById, resultsById});
 
-            mkSectionBrowserComponent({browserId: 'yabro-1'}, {tree});
+            const section = mkSectionBrowserComponent({browserId: 'yabro-1'}, {tree});
 
-            assert.calledWithMatch(
-                BrowserSkippedTitle,
-                set({}, 'title.props.children', [`[${SKIPPED}] `, 'yabro', undefined, undefined])
-            );
+            expect(section.getByText(/.*?\[skipped\].*?/)).to.exist;
         });
 
         it('should pass skip reason', () => {
@@ -59,12 +56,9 @@ describe('<SectionBrowser/>', () => {
             const resultsById = mkResult({id: 'res', status: SKIPPED, skipReason: 'some-reason'});
             const tree = mkStateTree({browsersById, browsersStateById, resultsById});
 
-            mkSectionBrowserComponent({browserId: 'yabro-1'}, {tree});
+            const section = mkSectionBrowserComponent({browserId: 'yabro-1'}, {tree});
 
-            assert.calledWithMatch(
-                BrowserSkippedTitle,
-                set({}, 'title.props.children', [`[${SKIPPED}] `, 'yabro', ', reason: ', 'some-reason'])
-            );
+            expect(section.getByText(/.*?reason:\s*some-reason.*?/)).to.exist;
         });
 
         it('should not render body even if browser in opened state', () => {
@@ -81,6 +75,13 @@ describe('<SectionBrowser/>', () => {
 
     describe('executed test with fails in retries and skip in result', () => {
         it('should render not skipped title', () => {
+            SectionBrowser = proxyquire('lib/static/components/section/section-browser', {
+                '../../modules/actions': actionsStub,
+                './title/browser-skipped': {default: BrowserSkippedTitle},
+                './title/browser': {default: BrowserTitle},
+                './body': {default: Body}
+            }).default;
+
             const browsersById = mkBrowser(
                 {id: 'yabro-1', name: 'yabro', resultIds: ['res-1', 'res-2'], parentId: 'test'}
             );
@@ -93,10 +94,28 @@ describe('<SectionBrowser/>', () => {
 
             mkSectionBrowserComponent({browserId: 'yabro-1'}, {tree});
 
-            assert.calledWithMatch(
-                BrowserTitle,
-                set({}, 'title.props.children', [`[${SKIPPED}] `, 'yabro', ', reason: ', 'some-reason'])
+            assert.notCalled(BrowserSkippedTitle);
+        });
+
+        it('should render title with skip reason', () => {
+            SectionBrowser = proxyquire('lib/static/components/section/section-browser', {
+                '../../modules/actions': actionsStub,
+                './body': {default: Body}
+            }).default;
+
+            const browsersById = mkBrowser(
+                {id: 'yabro-1', name: 'yabro', resultIds: ['res-1', 'res-2'], parentId: 'test'}
             );
+            const browsersStateById = {'yabro-1': {shouldBeShown: true, shouldBeOpened: true}};
+            const resultsById = {
+                ...mkResult({id: 'res-1', status: ERROR, error: {}}),
+                ...mkResult({id: 'res-2', status: SKIPPED, skipReason: 'some-reason'})
+            };
+            const tree = mkStateTree({browsersById, browsersStateById, resultsById});
+
+            const section = mkSectionBrowserComponent({browserId: 'yabro-1'}, {tree});
+
+            expect(section.getByText(/.*?reason:\s*some-reason.*?/)).to.exist;
         });
 
         it('should render body if browser in opened state', () => {