Results: (Bug Fix) In RevisionRow handle case when the platform icon …

…is not present (mozilla#569)

src/__tests__/CompareResults/RevisionRow.test.tsx

@@ -0,0 +1,56 @@
+import { screen } from '@testing-library/react';
+
+import RevisionRow from '../../components/CompareResults/RevisionRow';
+import { Platform } from '../../types/types';
+import getTestData from '../utils/fixtures';
+import { render } from '../utils/setupTests';
+
+describe('<RevisionRow>', () => {
+  it.each([
+    {
+      platform: 'linux1804-32-shippable-qr',
+      shortName: 'Linux',
+      hasIcon: true,
+    },
+    {
+      platform: 'macosx1014-64-shippable-qr',
+      shortName: 'OSX',
+      hasIcon: true,
+    },
+    {
+      platform: 'windows2012-64-shippable',
+      shortName: 'Windows',
+      hasIcon: true,
+    },
+    {
+      platform: 'android-5-0-aarch64-release',
+      shortName: 'Android',
+      hasIcon: true,
+    },
+    {
+      platform: 'i am not an operating system',
+      shortName: 'Unspecified',
+      hasIcon: false,
+    },
+  ])(
+    'shows correct platform info for platform "$platform"',
+    ({ platform, shortName, hasIcon }) => {
+      const {
+        testCompareData: [rowData],
+      } = getTestData();
+
+      rowData.platform = platform as Platform;
+      render(<RevisionRow themeMode='light' result={rowData} />);
+      const shortNameNode = screen.getByText(shortName);
+      expect(shortNameNode).toBeInTheDocument();
+      const previousNode = shortNameNode.previousSibling;
+      /* eslint-disable jest/no-conditional-expect */
+      if (hasIcon) {
+        expect(previousNode?.nodeName).toBe('svg');
+      } else {
+        expect(previousNode).toBeNull();
+      }
+      /* eslint-enable */
+    },
+  );
+});

src/__tests__/helpers.test.ts

@@ -1,7 +1,6 @@
 import {
   formatDate,
   getLatestCommitMessage,
-  getPlatformInfo,
   setConfidenceClassName,
   truncateHash,
   swapArrayElements,
@@ -94,20 +93,6 @@ describe('formateDate Helper', () => {
   });
 });
 
-describe('getPlatformInfo Helper', () => {
-  it.each([
-    { platform: 'linux-shippable', shortName: 'Linux' },
-    { platform: 'OS X 10.14 Shippable', shortName: 'OSX' },
-    { platform: 'windows10-64-mingwclang', shortName: 'Windows' },
-    { platform: 'Android 5.0 AArch64 Release', shortName: 'Android' },
-    { platform: 'i am not an operating system', shortName: '' },
-  ])('returns correct class name', (test) => {
-    expect(getPlatformInfo(test.platform).shortName).toStrictEqual(
-      test.shortName,
-    );
-  });
-});
-
 describe('setConfidenceClassName', () => {
   it.each([
     { confidenceText: 'low', className: 'low' },

src/components/CompareResults/RevisionRow.tsx

@@ -1,5 +1,6 @@
 import { useState } from 'react';
 
+import AppleIcon from '@mui/icons-material/Apple';
 import ExpandLessIcon from '@mui/icons-material/ExpandLess';
 import ExpandMoreIcon from '@mui/icons-material/ExpandMore';
 import FileDownloadOutlinedIcon from '@mui/icons-material/FileDownloadOutlined';
@@ -9,14 +10,17 @@ import { IconButton } from '@mui/material';
 import Link from '@mui/material/Link';
 import { style } from 'typestyle';
 
+import { Strings } from '../../resources/Strings';
 import { Colors, Spacing } from '../../styles';
 import { ExpandableRowStyles } from '../../styles';
 import type {
   CompareResultsItem,
   PlatformInfo,
   ThemeMode,
 } from '../../types/state';
-import { getPlatformInfo } from '../../utils/helpers';
+import AndroidIcon from '../Shared/Icons/AndroidIcon';
+import LinuxIcon from '../Shared/Icons/LinuxIcon';
+import WindowsIcon from '../Shared/Icons/WindowsIcon';
 import RevisionRowExpandable from './RevisionRowExpandable';
 
 function determineStatus(improvement: boolean, regression: boolean) {
@@ -31,6 +35,25 @@ function determineSign(baseMedianValue: number, newMedianValue: number) {
   return '';
 }
 
+const getPlatformInfo = (platformName: string): PlatformInfo => {
+  if (platformName.toLowerCase().includes('linux'))
+    return { shortName: 'Linux', icon: <LinuxIcon /> };
+  else if (
+    platformName.toLowerCase().includes('osx') ||
+    platformName.toLowerCase().includes('os x')
+  )
+    return { shortName: 'OSX', icon: <AppleIcon /> };
+  else if (platformName.toLowerCase().includes('windows'))
+    return { shortName: 'Windows', icon: <WindowsIcon /> };
+  else if (platformName.toLowerCase().includes('android'))
+    return { shortName: 'Android', icon: <AndroidIcon /> };
+  else
+    return {
+      shortName: Strings.components.revisionRow.platformUndefinedText,
+      icon: '',
+    };
+};
+
 function RevisionRow(props: RevisionRowProps) {
   const { themeMode, result } = props;
   const {
@@ -48,8 +71,7 @@ function RevisionRow(props: RevisionRowProps) {
     graphs_link: graphLink,
   } = result;
 
-  const platformInfo: PlatformInfo = getPlatformInfo(platform);
-  const PlatformIcon = platformInfo.icon as React.ElementType;
+  const platformInfo = getPlatformInfo(platform);
 
   const [expanded, setExpanded] = useState(false);
 
@@ -143,7 +165,7 @@ function RevisionRow(props: RevisionRowProps) {
       <div className={`revisionRow ${styles.revisionRow} ${styles.typography}`}>
         <div className='platform cell'>
           <div className='platform-container'>
-            <PlatformIcon />
+            {platformInfo.icon}
             <span>{platformInfo.shortName}</span>
           </div>
         </div>

src/resources/Strings.tsx

@@ -75,6 +75,9 @@ export const Strings = {
         collapedSelectLabel: 'Time range',
       },
     },
+    revisionRow: {
+      platformUndefinedText: 'Unspecified',
+    },
     expandableRow: {
       singleRun: 'Only one run (consider more runs for greater confidence).',
       Low: "A value of 'low' suggests less confidence that there is a sustained, significant change between the two revisions.",

src/types/state.ts

@@ -128,5 +128,5 @@ export type CompareResultsState = {
 
 export type PlatformInfo = {
   shortName: string;
-  icon: object;
+  icon: React.ReactNode;
 };

src/utils/helpers.ts

@@ -1,18 +1,13 @@
-import AppleIcon from '@mui/icons-material/Apple';
-
 import {
   frameworkMap,
   devToolsFramework,
   baseDocsURL,
   removedOldTestDevTools,
   nonDocumentedTestsDevTools,
+  supportedPerfdocsFrameworks,
 } from '../common/constants';
-import { supportedPerfdocsFrameworks } from '../common/constants';
-import AndroidIcon from '../components/Shared/Icons/AndroidIcon';
-import LinuxIcon from '../components/Shared/Icons/LinuxIcon';
-import WindowsIcon from '../components/Shared/Icons/WindowsIcon';
 import type { Repository, RevisionsList } from '../types/state';
-import { Framework, SupportedPerfdocsFramework } from '../types/types';
+import type { Framework, SupportedPerfdocsFramework } from '../types/types';
 
 const truncateHash = (revision: RevisionsList['revision']) =>
   revision.slice(0, 12);
@@ -48,21 +43,6 @@ const getTreeherderURL = (
 ) =>
   `https://treeherder.mozilla.org/jobs?repo=${repository}&revision=${revision}`;
 
-const getPlatformInfo = (platformName: string) => {
-  if (platformName.toLowerCase().includes('linux'))
-    return { shortName: 'Linux', icon: LinuxIcon };
-  else if (
-    platformName.toLowerCase().includes('osx') ||
-    platformName.toLowerCase().includes('os x')
-  )
-    return { shortName: 'OSX', icon: AppleIcon };
-  else if (platformName.toLowerCase().includes('windows'))
-    return { shortName: 'Windows', icon: WindowsIcon };
-  else if (platformName.toLowerCase().includes('android'))
-    return { shortName: 'Android', icon: AndroidIcon };
-  else return { shortName: '', icon: {} };
-};
-
 const createDevtoolsDocsUrl = (
   supportedFramework: string,
   urlReadySuite: string,
@@ -133,7 +113,6 @@ export {
   getLatestCommitMessage,
   getTreeherderURL,
   setConfidenceClassName,
-  getPlatformInfo,
   swapArrayElements,
   truncateHash,
   getDocsURL,