Improve display of skipped tasks

When a task is skipped ensure this is communicated to the user instead of leaving it showing as 'pending'. In the case of when expressions, surface details of the when expression so the user can undertsand the reason for the task being skipped. Also remove elements that will never have content in this case such as the pod tab on the TaskRun details view, and replace the log container with a message indicating that the task was skipped and no logs will be available. Update the status icons and labels to differentiate between pending / not run and skipped. Apply similar changes for step-level when expressions which rely on the step actions feature being enabled.
tektoncd · Oct 22, 2024 · 6b21f54 · 6b21f54
1 parent c51115a
commit 6b21f54
Show file tree

Hide file tree

Showing 39 changed files with 488 additions and 48 deletions.
diff --git a/packages/components/src/components/Actions/Actions.jsx b/packages/components/src/components/Actions/Actions.jsx
@@ -11,7 +11,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { useState } from 'react';
+import { Fragment, useState } from 'react';
 import { useIntl } from 'react-intl';
 import {
   Button,
@@ -105,7 +105,7 @@ export default function Actions({ items, kind, resource }) {
             } = item;
             const disabled = disable && disable(resource);
             return (
-              <>
+              <Fragment key={actionText}>
                 {hasDivider && <MenuItemDivider />}
                 <MenuItem
                   disabled={disabled}
@@ -116,7 +116,7 @@ export default function Actions({ items, kind, resource }) {
                     handleClick(() => itemAction(resource), modalProperties)
                   }
                 />
-              </>
+              </Fragment>
             );
           })}
         </MenuButton>

diff --git a/packages/components/src/components/DetailsHeader/DetailsHeader.jsx b/packages/components/src/components/DetailsHeader/DetailsHeader.jsx
@@ -65,6 +65,13 @@ export default function DetailsHeader({
   function getStatusLabel() {
     const { reason: taskReason, status: taskStatus } = getStatus(taskRun);
 
+    if (stepStatus?.terminationReason === 'Skipped') {
+      return intl.formatMessage({
+        id: 'dashboard.taskRun.status.skipped',
+        defaultMessage: 'Skipped'
+      });
+    }
+
     if (
       status === 'cancelled' ||
       (status === 'terminated' &&
@@ -126,11 +133,16 @@ export default function DetailsHeader({
   if (type === 'taskRun') {
     ({ reason: reasonToUse, status: statusToUse } = getStatus(taskRun));
     statusLabel =
-      reasonToUse ||
-      intl.formatMessage({
-        id: 'dashboard.taskRun.status.pending',
-        defaultMessage: 'Pending'
-      });
+      reason === 'tkn-dashboard:skipped'
+        ? intl.formatMessage({
+            id: 'dashboard.taskRun.status.skipped',
+            defaultMessage: 'Skipped'
+          })
+        : reasonToUse ||
+          intl.formatMessage({
+            id: 'dashboard.taskRun.status.pending',
+            defaultMessage: 'Pending'
+          });
   } else {
     statusLabel = getStatusLabel();
   }
@@ -140,14 +152,20 @@ export default function DetailsHeader({
       className="tkn--step-details-header"
       data-status={statusToUse}
       data-reason={reasonToUse}
+      data-termination-reason={stepStatus?.terminationReason}
     >
       <h2 className="tkn--details-header--heading">
         <StatusIcon
           DefaultIcon={props => <DefaultIcon size={24} {...props} />}
           hasWarning={hasWarning}
-          reason={reasonToUse}
+          reason={reason === 'tkn-dashboard:skipped' ? reason : reasonToUse}
           status={statusToUse}
-          {...(type === 'step' ? { type: 'inverse' } : null)}
+          {...(type === 'step'
+            ? {
+                terminationReason: stepStatus?.terminationReason,
+                type: 'inverse'
+              }
+            : null)}
         />
         <span className="tkn--run-details-name" title={displayName}>
           {displayName}

diff --git a/packages/components/src/components/DetailsHeader/DetailsHeader.stories.js b/packages/components/src/components/DetailsHeader/DetailsHeader.stories.js
@@ -13,12 +13,13 @@ limitations under the License.
 
 import DetailsHeader from './DetailsHeader';
 
-const getTaskRun = ({ reason, status }) => ({
+const getTaskRun = ({ reason, status, terminationReason }) => ({
   status: {
     conditions: [
       {
         reason,
         status,
+        terminationReason,
         type: 'Succeeded'
       }
     ]
@@ -71,6 +72,35 @@ export const CompletedWithWarning = {
   name: 'Completed with warning'
 };
 
+export const SkippedTask = {
+  args: {
+    reason: 'tkn-dashboard:skipped',
+    displayName: 'build',
+    taskRun: {},
+    type: 'taskRun'
+  },
+  argTypes: {
+    type: {
+      control: false
+    }
+  }
+};
+
+export const SkippedStep = {
+  args: {
+    reason: 'Completed',
+    status: 'terminated',
+    stepStatus: { terminationReason: 'Skipped' },
+    displayName: 'build',
+    type: 'step'
+  },
+  argTypes: {
+    type: {
+      control: false
+    }
+  }
+};
+
 export const Failed = {
   args: {
     displayName: 'build',

diff --git a/packages/components/src/components/DetailsHeader/DetailsHeader.test.jsx b/packages/components/src/components/DetailsHeader/DetailsHeader.test.jsx
@@ -104,6 +104,43 @@ describe('DetailsHeader', () => {
     expect(queryByText(/pending/i)).toBeTruthy();
   });
 
+  it('renders the skipped state for a step', () => {
+    const taskRun = {
+      status: {
+        conditions: [
+          {
+            reason: 'Completed',
+            status: 'True',
+            type: 'Succeeded'
+          }
+        ]
+      }
+    };
+
+    const { queryByText } = render(
+      <DetailsHeader
+        {...props}
+        stepStatus={{ terminationReason: 'Skipped' }}
+        taskRun={taskRun}
+      />
+    );
+    expect(queryByText(/skipped/i)).toBeTruthy();
+  });
+
+  it('renders the skipped state for a TaskRun', () => {
+    const taskRun = {};
+
+    const { queryByText } = render(
+      <DetailsHeader
+        {...props}
+        reason="tkn-dashboard:skipped"
+        taskRun={taskRun}
+        type="taskRun"
+      />
+    );
+    expect(queryByText(/skipped/i)).toBeTruthy();
+  });
+
   it('renders no duration for a running step', () => {
     const stepStatus = {
       running: {

diff --git a/packages/components/src/components/DetailsHeader/_DetailsHeader.scss b/packages/components/src/components/DetailsHeader/_DetailsHeader.scss
@@ -86,7 +86,7 @@ header.tkn--step-details-header {
       color: $support-info;
     }
   }
-  &[data-status='terminated'][data-reason='Completed'],
+  &[data-status='terminated'][data-reason='Completed']:not([data-termination-reason='Skipped']),
   &[data-status='True'] {
     .tkn--status-label {
       color: $support-success;

diff --git a/packages/components/src/components/Log/Log.jsx b/packages/components/src/components/Log/Log.jsx
@@ -276,9 +276,16 @@ export class LogContainer extends Component {
     );
   };
 
-  getTrailerMessage = ({ exitCode, reason }) => {
+  getTrailerMessage = ({ exitCode, reason, terminationReason }) => {
     const { forcePolling, intl } = this.props;
 
+    if (terminationReason === 'Skipped') {
+      return intl.formatMessage({
+        id: 'dashboard.pipelineRun.stepSkipped',
+        defaultMessage: 'Step skipped'
+      });
+    }
+
     if (reason && forcePolling) {
       return (
         <>
@@ -397,7 +404,11 @@ export class LogContainer extends Component {
   logTrailer = () => {
     const { forcePolling, stepStatus } = this.props;
     const { exitCode, reason } = (stepStatus && stepStatus.terminated) || {};
-    const trailer = this.getTrailerMessage({ exitCode, reason });
+    const trailer = this.getTrailerMessage({
+      exitCode,
+      reason,
+      terminationReason: stepStatus?.terminationReason
+    });
     if (!trailer) {
       return null;
     }

diff --git a/packages/components/src/components/Log/Log.stories.jsx b/packages/components/src/components/Log/Log.stories.jsx
@@ -97,6 +97,16 @@ export const Performance = {
   name: 'performance test (<20,000 lines with ANSI)'
 };
 
+export const Skipped = {
+  args: {
+    fetchLogs: () => 'This step was skipped',
+    stepStatus: {
+      terminated: { reason: 'Completed', exitCode: 0 },
+      terminationReason: 'Skipped'
+    }
+  }
+};
+
 export const Toolbar = {
   args: {
     fetchLogs: () => 'A log message',

diff --git a/packages/components/src/components/Log/Log.test.jsx b/packages/components/src/components/Log/Log.test.jsx
@@ -61,6 +61,19 @@ describe('Log', () => {
     await waitFor(() => getByText(/step failed/i));
   });
 
+  it('renders skipped trailer', async () => {
+    const { getByText } = render(
+      <Log
+        stepStatus={{
+          terminated: { reason: 'Completed' },
+          terminationReason: 'Skipped'
+        }}
+        fetchLogs={() => 'testing'}
+      />
+    );
+    await waitFor(() => getByText(/step skipped/i));
+  });
+
   it('renders pending trailer when step complete and forcePolling is true', async () => {
     const { getByText, queryByText } = render(
       <Log

diff --git a/packages/components/src/components/PipelineRun/PipelineRun.jsx b/packages/components/src/components/PipelineRun/PipelineRun.jsx
@@ -299,6 +299,11 @@ export default /* istanbul ignore next */ function PipelineRun({
     taskRun
   });
 
+  const skippedTasks = pipelineRun.status?.skippedTasks || [];
+  const skippedTask = skippedTasks.find(
+    skipped => skipped.name === selectedTaskId
+  );
+
   return (
     <>
       <RunHeader
@@ -324,13 +329,15 @@ export default /* istanbul ignore next */ function PipelineRun({
             selectedStepId={selectedStepId}
             selectedTaskId={selectedTaskId}
             selectedTaskRunName={selectedTaskRunName}
+            skippedTasks={skippedTasks}
             taskRuns={taskRunsToUse}
           />
           {(selectedStepId && (
             <StepDetails
               definition={definition}
               logContainer={logContainer}
               onViewChange={onViewChange}
+              skippedTask={skippedTask}
               stepName={selectedStepId}
               stepStatus={stepStatus}
               taskRun={taskRun}
@@ -341,6 +348,7 @@ export default /* istanbul ignore next */ function PipelineRun({
               <TaskRunDetails
                 onViewChange={onViewChange}
                 pod={pod}
+                skippedTask={skippedTask}
                 task={task}
                 taskRun={taskRun}
                 view={view}

diff --git a/packages/components/src/components/PipelineRun/PipelineRun.stories.jsx b/packages/components/src/components/PipelineRun/PipelineRun.stories.jsx
@@ -92,6 +92,19 @@ const taskRunWithWarning = getTaskRun({
   pipelineTaskName: 'task2'
 });
 
+const taskRunSkipped = getTaskRun({
+  name: 'sampleTaskRunName3',
+  pipelineTaskName: 'task3'
+});
+delete taskRunSkipped.status.conditions;
+delete taskRunSkipped.status.steps[0].terminated;
+
+const taskRunWithSkippedStep = getTaskRun({
+  name: 'sampleTaskRunName4',
+  pipelineTaskName: 'task4'
+});
+taskRunWithSkippedStep.status.steps[0].terminationReason = 'Skipped';
+
 const pipelineRun = {
   metadata: {
     name: 'pipeline-run',
@@ -113,6 +126,9 @@ const pipelineRun = {
         type: 'Succeeded'
       }
     ],
+    skippedTasks: [
+      { name: 'task3', reason: 'When Expressions evaluated to false' }
+    ],
     startTime: '2019-08-21T17:12:20Z',
     taskRuns: {
       sampleTaskRunName: {
@@ -122,6 +138,14 @@ const pipelineRun = {
       sampleTaskRunName2: {
         pipelineTaskName: 'task2',
         status: taskRunWithWarning.status
+      },
+      sampleTaskRunName3: {
+        pipelineTaskName: 'task3',
+        status: taskRunSkipped.status
+      },
+      sampleTaskRunName4: {
+        pipelineTaskName: 'task4',
+        status: taskRunWithSkippedStep.status
       }
     }
   }
@@ -188,7 +212,12 @@ export const Default = args => {
       }}
       onViewChange={selectedView => updateArgs({ view: selectedView })}
       pipelineRun={pipelineRun}
-      taskRuns={[taskRun, taskRunWithWarning]}
+      taskRuns={[
+        taskRun,
+        taskRunWithWarning,
+        taskRunSkipped,
+        taskRunWithSkippedStep
+      ]}
       tasks={[task]}
     />
   );

diff --git a/packages/components/src/components/StatusIcon/StatusIcon.jsx b/packages/components/src/components/StatusIcon/StatusIcon.jsx
@@ -18,6 +18,8 @@ import {
   CloseFilled,
   CloseOutline,
   Time as Pending,
+  Undefined,
+  UndefinedFilled,
   WarningAltFilled as WarningFilled
 } from '@carbon/react/icons';
 import { classNames, isRunning } from '@tektoncd/dashboard-utils';
@@ -30,6 +32,7 @@ const icons = {
     error: CloseOutline,
     pending: Pending,
     running: Spinner,
+    skipped: Undefined,
     success: CheckmarkOutline,
     warning: WarningFilled
   },
@@ -38,6 +41,7 @@ const icons = {
     error: CloseFilled,
     pending: Pending,
     running: Spinner,
+    skipped: UndefinedFilled,
     success: CheckmarkFilled,
     warning: CheckmarkFilledWarning
   }
@@ -63,11 +67,17 @@ export default function StatusIcon({
   isCustomTask,
   reason,
   status,
+  terminationReason,
   title,
   type = 'normal'
 }) {
   let statusClass;
   if (
+    (!status && reason === 'tkn-dashboard:skipped') ||
+    terminationReason === 'Skipped'
+  ) {
+    statusClass = 'skipped';
+  } else if (
     (!status && !DefaultIcon) ||
     (status === 'Unknown' && reason === 'Pending')
   ) {