Browse Source

feat(codecov): Add coverage status to stacktrace link (#43667)

![image](https://user-images.githubusercontent.com/16563948/214963698-61b056d1-d856-408e-ad4a-36cf4f365c77.png)
Snigdha Sharma 2 years ago
parent
commit
cb7a589d9c

+ 6 - 6
static/app/components/events/interfaces/frame/context.spec.tsx

@@ -3,7 +3,7 @@ import {render} from 'sentry-test/reactTestingLibrary';
 import ProjectsStore from 'sentry/stores/projectsStore';
 import {Coverage, Frame, LineCoverage} from 'sentry/types';
 
-import Context, {getCoverageColors} from './context';
+import Context, {getCoverageColorClass} from './context';
 
 describe('Frame - Context', function () {
   const org = TestStubs.Organization();
@@ -35,11 +35,11 @@ describe('Frame - Context', function () {
   ];
 
   it('converts coverage data to the right colors', function () {
-    expect(getCoverageColors(lines, lineCoverage)).toEqual([
-      'yellow100',
-      'green100',
-      'transparent',
-      'red100',
+    expect(getCoverageColorClass(lines, lineCoverage, 233)).toEqual([
+      'partial',
+      'covered',
+      'active',
+      'uncovered',
     ]);
   });
 

+ 90 - 12
static/app/components/events/interfaces/frame/context.tsx

@@ -21,7 +21,6 @@ import {
 } from 'sentry/types';
 import {Event} from 'sentry/types/event';
 import {defined} from 'sentry/utils';
-import {Color} from 'sentry/utils/theme';
 import useProjects from 'sentry/utils/useProjects';
 import withOrganization from 'sentry/utils/withOrganization';
 
@@ -52,27 +51,38 @@ type Props = {
   registersMeta?: Record<any, any>;
 };
 
-export function getCoverageColors(
+export function getCoverageColorClass(
   lines: [number, string][],
-  lineCov: LineCoverage[]
-): Array<Color | 'transparent'> {
+  lineCov: LineCoverage[],
+  activeLineNo: number
+): Array<string> {
   const lineCoverage = keyBy(lineCov, 0);
   return lines.map(([lineNo]) => {
     const coverage = lineCoverage[lineNo]
       ? lineCoverage[lineNo][1]
       : Coverage.NOT_APPLICABLE;
+
+    let color = '';
     switch (coverage) {
       case Coverage.COVERED:
-        return 'green100';
+        color = 'covered';
+        break;
       case Coverage.NOT_COVERED:
-        return 'red100';
+        color = 'uncovered';
+        break;
       case Coverage.PARTIAL:
-        return 'yellow100';
+        color = 'partial';
+        break;
       case Coverage.NOT_APPLICABLE:
       // fallthrough
       default:
-        return 'transparent';
+        break;
+    }
+
+    if (activeLineNo !== lineNo) {
+      return color;
     }
+    return color === '' ? 'active' : `active ${color}`;
   });
 }
 
@@ -163,9 +173,9 @@ const Context = ({
   const hasCoverageData =
     !isLoading && data?.codecov?.status === CodecovStatusCode.COVERAGE_EXISTS;
 
-  const lineColors: Array<Color | 'transparent'> =
-    hasCoverageData && data!.codecov?.lineCoverage!
-      ? getCoverageColors(contextLines, data!.codecov?.lineCoverage)
+  const lineColors: Array<string> =
+    hasCoverageData && data!.codecov?.lineCoverage && !!frame.lineNo!
+      ? getCoverageColorClass(contextLines, data!.codecov?.lineCoverage, frame.lineNo)
       : [];
 
   return (
@@ -190,7 +200,7 @@ const Context = ({
               key={index}
               line={line}
               isActive={isActive}
-              color={isActive ? 'transparent' : lineColors[index] ?? 'transparent'}
+              colorClass={lineColors[index] ?? ''}
             >
               {hasComponents && (
                 <ErrorBoundary mini>
@@ -252,6 +262,74 @@ const StyledContextLine = styled(ContextLine)`
   padding: 0;
   text-indent: 20px;
   z-index: 1000;
+  position: relative;
+
+  &:before {
+    content: '';
+    display: block;
+    height: 24px;
+    width: 50px;
+    left: 0;
+    top: 0;
+    bottom: 0;
+    background: transparent;
+    position: absolute;
+    z-index: 1;
+  }
+
+  &:after {
+    content: '';
+    display: block;
+    width: 2px;
+    border-color: transparent;
+    position: absolute;
+    left: 50px;
+    top: 0;
+    bottom: 0;
+    z-index: 9999;
+    height: 24px;
+  }
+
+  &.covered:before {
+    background: ${p => p.theme.green100};
+  }
+
+  &.covered:after {
+    border-style: solid;
+    border-color: ${p => p.theme.green300};
+  }
+
+  &.uncovered:before {
+    background: ${p => p.theme.red100};
+  }
+
+  &.partial:before {
+    background: ${p => p.theme.yellow100};
+  }
+
+  &.partial:after {
+    border-style: dashed;
+    border-color: ${p => p.theme.yellow300};
+  }
+
+  &.active {
+    background-color: ${p => p.theme.gray400};
+  }
+
+  &.active.partial:before {
+    mix-blend-mode: screen;
+    background: ${p => p.theme.yellow200};
+  }
+
+  &.active.covered:before {
+    mix-blend-mode: screen;
+    background: ${p => p.theme.green200};
+  }
+
+  &.active.uncovered:before {
+    mix-blend-mode: screen;
+    background: ${p => p.theme.red200};
+  }
 `;
 
 const Wrapper = styled('ol')`

+ 9 - 8
static/app/components/events/interfaces/frame/contextLine.tsx

@@ -1,31 +1,32 @@
 import {Fragment} from 'react';
-import {useTheme} from '@emotion/react';
 import styled from '@emotion/styled';
 import classNames from 'classnames';
 
-import {Color} from 'sentry/utils/theme';
-
 interface Props {
-  color: Color | 'transparent';
+  colorClass: string;
   isActive: boolean;
   line: [number, string];
   children?: React.ReactNode;
   className?: string;
 }
 
-const ContextLine = function ({line, isActive, children, className, color}: Props) {
+const ContextLine = function ({line, isActive, children, className, colorClass}: Props) {
   let lineWs = '';
   let lineCode = '';
   if (typeof line[1] === 'string') {
     [, lineWs, lineCode] = line[1].match(/^(\s*)(.*?)$/m)!;
   }
   const Component = !children ? Fragment : Context;
-  const theme = useTheme();
+  const hasCoverage = colorClass !== '';
+
   return (
     <li
-      className={classNames(className, 'expandable', {active: isActive})}
+      className={classNames(
+        className,
+        'expandable',
+        hasCoverage ? colorClass : {active: isActive}
+      )}
       key={line[0]}
-      style={{backgroundColor: theme[color]}}
     >
       <Component>
         <span className="ws">{lineWs}</span>

+ 3 - 2
static/app/components/events/interfaces/frame/stacktraceLink.spec.tsx

@@ -213,6 +213,7 @@ describe('StacktraceLink', function () {
         integrations: [integration],
         codecov: {
           status: CodecovStatusCode.COVERAGE_EXISTS,
+          lineCoverage: [[233, 0]],
           coverageUrl: 'https://app.codecov.io/gh/path/to/file.py',
         },
       },
@@ -222,12 +223,12 @@ describe('StacktraceLink', function () {
       organization,
     });
 
-    expect(await screen.findByText('View Coverage Tests on Codecov')).toHaveAttribute(
+    expect(await screen.findByText('Open in Codecov')).toHaveAttribute(
       'href',
       'https://app.codecov.io/gh/path/to/file.py#L233'
     );
 
-    userEvent.click(await screen.findByText('View Coverage Tests on Codecov'));
+    userEvent.click(await screen.findByText('Open in Codecov'));
     expect(analyticsSpy).toHaveBeenCalledWith(
       'integrations.stacktrace_codecov_link_clicked',
       expect.anything()

+ 73 - 14
static/app/components/events/interfaces/frame/stacktraceLink.tsx

@@ -14,13 +14,15 @@ import ExternalLink from 'sentry/components/links/externalLink';
 import Link from 'sentry/components/links/link';
 import Placeholder from 'sentry/components/placeholder';
 import type {PlatformKey} from 'sentry/data/platformCategories';
-import {IconClose, IconWarning} from 'sentry/icons';
+import {IconCircle, IconCircleFill, IconClose, IconWarning} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
 import {
   CodecovStatusCode,
+  Coverage,
   Event,
   Frame,
+  LineCoverage,
   Organization,
   Project,
   StacktraceLinkResult,
@@ -124,18 +126,59 @@ function shouldshowCodecovFeatures(
 
 interface CodecovLinkProps {
   event: Event;
+  lineNo: number | null;
   organization: Organization;
-  codecovStatusCode?: CodecovStatusCode;
-  codecovUrl?: string;
+  coverageUrl?: string;
+  lineCoverage?: LineCoverage[];
+  status?: CodecovStatusCode;
+}
+
+function getCoverageIcon(lineCoverage, lineNo) {
+  const covIndex = lineCoverage.findIndex(line => line[0] === lineNo);
+  if (covIndex === -1) {
+    return null;
+  }
+  switch (lineCoverage[covIndex][1]) {
+    case Coverage.COVERED:
+      return (
+        <CoverageIcon>
+          <IconCircleFill size="xs" color="green100" style={{position: 'absolute'}} />
+          <IconCircle size="xs" color="green300" />
+          {t('Covered')}
+        </CoverageIcon>
+      );
+    case Coverage.PARTIAL:
+      return (
+        <CoverageIcon>
+          <IconCircleFill size="xs" color="yellow100" style={{position: 'absolute'}} />
+          <IconCircle size="xs" color="yellow300" />
+          {t('Partially Covered')}
+        </CoverageIcon>
+      );
+    case Coverage.NOT_COVERED:
+      return (
+        <CodecovContainer>
+          <CoverageIcon>
+            <IconCircleFill size="xs" color="red100" style={{position: 'absolute'}} />
+            <IconCircle size="xs" color="red300" />
+          </CoverageIcon>
+          {t('Not Covered')}
+        </CodecovContainer>
+      );
+    default:
+      return null;
+  }
 }
 
 function CodecovLink({
-  codecovUrl,
-  codecovStatusCode,
+  coverageUrl,
+  status = CodecovStatusCode.COVERAGE_EXISTS,
+  lineCoverage,
+  lineNo,
   organization,
   event,
 }: CodecovLinkProps) {
-  if (codecovStatusCode === CodecovStatusCode.NO_COVERAGE_DATA) {
+  if (status === CodecovStatusCode.NO_COVERAGE_DATA) {
     return (
       <CodecovWarning>
         {t('Code Coverage not found')}
@@ -144,8 +187,8 @@ function CodecovLink({
     );
   }
 
-  if (codecovStatusCode === CodecovStatusCode.COVERAGE_EXISTS) {
-    if (!codecovUrl) {
+  if (status === CodecovStatusCode.COVERAGE_EXISTS) {
+    if (!coverageUrl || !lineCoverage || !lineNo) {
       return null;
     }
 
@@ -158,10 +201,13 @@ function CodecovLink({
     };
 
     return (
-      <OpenInLink href={codecovUrl} openInNewTab onClick={onOpenCodecovLink}>
-        {t('View Coverage Tests on Codecov')}
-        <StyledIconWrapper>{getIntegrationIcon('codecov', 'sm')}</StyledIconWrapper>
-      </OpenInLink>
+      <CodecovContainer>
+        {getCoverageIcon(lineCoverage, lineNo)}
+        <OpenInLink href={coverageUrl} openInNewTab onClick={onOpenCodecovLink}>
+          <StyledIconWrapper>{getIntegrationIcon('codecov', 'sm')}</StyledIconWrapper>
+          {t('Open in Codecov')}
+        </OpenInLink>
+      </CodecovContainer>
     );
   }
   return null;
@@ -297,8 +343,10 @@ export function StacktraceLink({frame, event, line}: StacktraceLinkProps) {
         </OpenInLink>
         {shouldshowCodecovFeatures(organization, match) && (
           <CodecovLink
-            codecovUrl={`${match.codecov?.coverageUrl}#L${frame.lineNo}`}
-            codecovStatusCode={match.codecov?.status}
+            coverageUrl={`${match.codecov?.coverageUrl}#L${frame.lineNo}`}
+            status={match.codecov?.status}
+            lineCoverage={match.codecov?.lineCoverage}
+            lineNo={frame.lineNo}
             organization={organization}
             event={event}
           />
@@ -414,3 +462,14 @@ const CodecovWarning = styled('div')`
   gap: ${space(0.75)};
   align-items: center;
 `;
+
+const CodecovContainer = styled('span')`
+  display: flex;
+  gap: ${space(0.75)};
+`;
+
+const CoverageIcon = styled('span')`
+  display: flex;
+  gap: ${space(0.75)};
+  align-items: center;
+`;

+ 2 - 7
static/less/group-detail.less

@@ -478,10 +478,6 @@ div.traceback > ul {
       margin: 0;
       padding: 8px 0;
 
-      > li {
-        background: inherit;
-      }
-
       table.key-value {
         border-top: 1px solid @trim;
         padding: 0;
@@ -663,7 +659,6 @@ div.traceback > ul {
     > li {
       font-family: @font-family-code;
       color: #222;
-      background-color: #f6f7f8;
       line-height: 24px;
       font-size: 12px;
       white-space: pre;
@@ -673,7 +668,6 @@ div.traceback > ul {
     }
 
     > li.active {
-      background-color: #f6f7f8;
       list-style-type: none;
       border-radius: 2px;
 
@@ -706,7 +700,8 @@ div.traceback > ul {
       }
 
       > li.active {
-        background-color: @purple;
+        position: relative;
+        z-index: 0;
         color: #fff;
         list-style-type: inherit;
         border-radius: 0;