diff --git a/src/common/formatters.ts b/src/common/formatters.ts index ce94cab7e..0011c0459 100644 --- a/src/common/formatters.ts +++ b/src/common/formatters.ts @@ -144,3 +144,12 @@ export function ensureUrlWithProtocol(url: string): string { } return url; } + +/** Formats a number into a string with leading zeros to ensure a consistent + * width. + * Example: 1 will be '01' + * 10 will be '10' + */ +export function leftPaddedNumber(value: number, length: number): string { + return value.toString().padStart(length, '0'); +} diff --git a/src/common/test/formatters.spec.ts b/src/common/test/formatters.spec.ts index 2e82700d7..718ce5db0 100644 --- a/src/common/test/formatters.spec.ts +++ b/src/common/test/formatters.spec.ts @@ -1,5 +1,4 @@ import { millisecondsToDuration } from 'common/utils'; -import { Admin } from 'flyteidl'; import { subSecondString, unknownValueString, @@ -10,10 +9,10 @@ import { dateFromNow, dateWithFromNow, ensureUrlWithProtocol, - fixedRateToString, formatDate, formatDateLocalTimezone, formatDateUTC, + leftPaddedNumber, millisecondsToHMS, protobufDurationToHMS } from '../formatters'; @@ -177,3 +176,24 @@ describe('ensureUrlWithProtocol', () => { }) ); }); + +describe('leftPaddedNumber', () => { + const cases: [number, number, string][] = [ + [1, 0, '1'], + [10, 0, '10'], + [0, 1, '0'], + [0, 2, '00'], + [1, 1, '1'], + [1, 2, '01'], + [1, 3, '001'], + [10, 1, '10'], + [10, 2, '10'], + [10, 3, '010'] + ]; + + cases.forEach(([value, width, expected]) => + it(`should produce ${expected} with input (${value}, ${width})`, () => { + expect(leftPaddedNumber(value, width)).toEqual(expected); + }) + ); +}); diff --git a/src/components/Executions/ExecutionDetails/NodeExecutionDetails.tsx b/src/components/Executions/ExecutionDetails/NodeExecutionDetails.tsx index a933af4ad..b869ae77f 100644 --- a/src/components/Executions/ExecutionDetails/NodeExecutionDetails.tsx +++ b/src/components/Executions/ExecutionDetails/NodeExecutionDetails.tsx @@ -35,6 +35,9 @@ const useStyles = makeStyles((theme: Theme) => { content: { overflowY: 'auto' }, + displayId: { + marginBottom: theme.spacing(1) + }, header: { borderBottom: `${theme.spacing(1)}px solid ${theme.palette.divider}` }, @@ -60,8 +63,7 @@ const useStyles = makeStyles((theme: Theme) => { title: { alignItems: 'flex-start', display: 'flex', - justifyContent: 'space-between', - marginBottom: theme.spacing(1) + justifyContent: 'space-between' } }; }); @@ -189,6 +191,16 @@ export const NodeExecutionDetails: React.FC = ({ + + {execution.displayId} + {statusContent} diff --git a/src/components/Executions/ExecutionDetails/test/NodeExecutionDetails.test.tsx b/src/components/Executions/ExecutionDetails/test/NodeExecutionDetails.test.tsx new file mode 100644 index 000000000..7002576b0 --- /dev/null +++ b/src/components/Executions/ExecutionDetails/test/NodeExecutionDetails.test.tsx @@ -0,0 +1,45 @@ +import { render, waitFor } from '@testing-library/react'; +import { mockAPIContextValue } from 'components/data/__mocks__/apiContext'; +import { APIContext } from 'components/data/apiContext'; +import { + DetailedNodeExecution, + NodeExecutionDisplayType +} from 'components/Executions/types'; +import { createMockNodeExecutions } from 'models/Execution/__mocks__/mockNodeExecutionsData'; +import { listTaskExecutions } from 'models/Execution/api'; +import * as React from 'react'; +import { NodeExecutionDetails } from '../NodeExecutionDetails'; + +describe('NodeExecutionDetails', () => { + let execution: DetailedNodeExecution; + let mockListTaskExecutions: jest.Mock>; + beforeEach(() => { + const { executions } = createMockNodeExecutions(1); + execution = { + ...executions[0], + displayType: NodeExecutionDisplayType.PythonTask, + displayId: 'com.flyte.testTask', + cacheKey: 'abcdefg' + }; + mockListTaskExecutions = jest.fn().mockResolvedValue({ entities: [] }); + }); + + const renderComponent = () => + render( + + + + ); + + it('renders displayId', async () => { + const { queryByText } = renderComponent(); + await waitFor(() => {}); + expect(queryByText(execution.displayId)).toBeInTheDocument(); + }); +}); diff --git a/src/components/Executions/Tables/nodeExecutionColumns.tsx b/src/components/Executions/Tables/nodeExecutionColumns.tsx index 519405542..2082bbc7b 100644 --- a/src/components/Executions/Tables/nodeExecutionColumns.tsx +++ b/src/components/Executions/Tables/nodeExecutionColumns.tsx @@ -49,7 +49,14 @@ export function generateColumns( ): NodeExecutionColumnDefinition[] { return [ { - cellRenderer: props => , + cellRenderer: props => ( + <> + + + {props.execution.displayId} + + + ), className: styles.columnName, key: 'name', label: 'node' diff --git a/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx b/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx new file mode 100644 index 000000000..f7b372f04 --- /dev/null +++ b/src/components/Executions/Tables/test/NodeExecutionsTable.test.tsx @@ -0,0 +1,76 @@ +import { render, waitFor } from '@testing-library/react'; +import { mapNodeExecutionDetails } from 'components'; +import { mockAPIContextValue } from 'components/data/__mocks__/apiContext'; +import { APIContext } from 'components/data/apiContext'; +import { DetailedNodeExecution } from 'components/Executions/types'; +import { + createMockWorkflow, + createMockWorkflowClosure +} from 'models/__mocks__/workflowData'; +import { createMockNodeExecutions } from 'models/Execution/__mocks__/mockNodeExecutionsData'; +import { listTaskExecutions } from 'models/Execution/api'; +import { mockTasks } from 'models/Task/__mocks__/mockTaskData'; +import * as React from 'react'; +import { + NodeExecutionsTable, + NodeExecutionsTableProps +} from '../NodeExecutionsTable'; + +describe('NodeExecutionsTable', () => { + let props: NodeExecutionsTableProps; + let mockNodeExecutions: DetailedNodeExecution[]; + let mockListTaskExecutions: jest.Mock>; + + beforeEach(() => { + mockListTaskExecutions = jest.fn().mockResolvedValue({ entities: [] }); + const { + executions: mockExecutions, + nodes: mockNodes + } = createMockNodeExecutions(1); + + const mockWorkflow = createMockWorkflow('SampleWorkflow'); + const mockWorkflowClosure = createMockWorkflowClosure(); + const compiledWorkflow = mockWorkflowClosure.compiledWorkflow!; + const { + primary: { template }, + tasks + } = compiledWorkflow; + template.nodes = template.nodes.concat(mockNodes); + compiledWorkflow.tasks = tasks.concat(mockTasks); + mockWorkflow.closure = mockWorkflowClosure; + + mockNodeExecutions = mapNodeExecutionDetails( + mockExecutions, + mockWorkflow + ); + + props = { + value: mockNodeExecutions, + lastError: null, + loading: false, + moreItemsAvailable: false, + fetch: jest.fn() + }; + }); + + const renderTable = () => + render( + + + + ); + + it('renders displayId for nodes', async () => { + const { queryByText } = renderTable(); + await waitFor(() => {}); + + const node = mockNodeExecutions[0]; + expect(queryByText(node.displayId)).toBeInTheDocument(); + }); +}); diff --git a/src/components/Executions/TaskExecutionsList/TaskExecutionsListItem.tsx b/src/components/Executions/TaskExecutionsList/TaskExecutionsListItem.tsx index 0229cb2f5..52356de1e 100644 --- a/src/components/Executions/TaskExecutionsList/TaskExecutionsListItem.tsx +++ b/src/components/Executions/TaskExecutionsList/TaskExecutionsListItem.tsx @@ -11,7 +11,7 @@ import { ExecutionStatusBadge } from '../ExecutionStatusBadge'; import { TaskExecutionDetails } from './TaskExecutionDetails'; import { TaskExecutionError } from './TaskExecutionError'; import { TaskExecutionLogs } from './TaskExecutionLogs'; -import { getUniqueTaskExecutionName } from './utils'; +import { formatRetryAttempt } from './utils'; const useStyles = makeStyles((theme: Theme) => ({ detailsLink: { @@ -43,7 +43,7 @@ export const TaskExecutionsListItem: React.FC = ({ const styles = useStyles(); const { closure, isParent } = taskExecution; const { error } = closure; - const headerText = getUniqueTaskExecutionName(taskExecution); + const headerText = formatRetryAttempt(taskExecution.id.retryAttempt); const taskHasStarted = closure.phase >= TaskExecutionPhase.QUEUED; return (
diff --git a/src/components/Executions/TaskExecutionsList/test/utils.spec.ts b/src/components/Executions/TaskExecutionsList/test/utils.spec.ts index a4711a2f4..616581479 100644 --- a/src/components/Executions/TaskExecutionsList/test/utils.spec.ts +++ b/src/components/Executions/TaskExecutionsList/test/utils.spec.ts @@ -1,5 +1,5 @@ import { obj } from 'test/utils'; -import { getUniqueTaskExecutionName } from '../utils'; +import { formatRetryAttempt, getUniqueTaskExecutionName } from '../utils'; describe('getUniqueTaskExecutionName', () => { const cases: [{ name: string; retryAttempt: number }, string][] = [ @@ -20,3 +20,16 @@ describe('getUniqueTaskExecutionName', () => { ).toEqual(expected)) ); }); + +describe('formatRetryAttempt', () => { + const cases: [number, string][] = [ + [0, 'Attempt 01'], + [1, 'Attempt 02'], + [2, 'Attempt 03'] + ]; + + cases.forEach(([input, expected]) => + it(`should return ${expected} for ${input}`, () => + expect(formatRetryAttempt(input)).toEqual(expected)) + ); +}); diff --git a/src/components/Executions/TaskExecutionsList/utils.ts b/src/components/Executions/TaskExecutionsList/utils.ts index 84e54c685..407a29026 100644 --- a/src/components/Executions/TaskExecutionsList/utils.ts +++ b/src/components/Executions/TaskExecutionsList/utils.ts @@ -1,3 +1,4 @@ +import { leftPaddedNumber } from 'common/formatters'; import { TaskExecution } from 'models'; /** Generates a unique name for a task execution, suitable for display in a @@ -11,3 +12,8 @@ export function getUniqueTaskExecutionName({ id }: TaskExecution) { const suffix = retryAttempt > 0 ? ` (${retryAttempt + 1})` : ''; return `${name}${suffix}`; } + +export function formatRetryAttempt(attempt: number): string { + // Retry attempts are zero-based, so incrementing before formatting + return `Attempt ${leftPaddedNumber(attempt + 1, 2)}`; +} diff --git a/tsconfig.json b/tsconfig.json index 705b4e972..427464ed6 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,28 +1,28 @@ { - "compilerOptions": { - "allowSyntheticDefaultImports": true, - "allowJs": true, - "baseUrl": "src", - "checkJs": false, - "downlevelIteration": true, - "emitDecoratorMetadata": true, - "experimentalDecorators": true, - "importHelpers": true, - "jsx": "react", - "lib": ["es2015", "es2017", "dom", "dom.iterable"], - "module": "commonjs", - "moduleResolution": "node", - "noFallthroughCasesInSwitch": true, - "noImplicitReturns": true, - "noUnusedLocals": false, - "noUnusedParameters": false, - "outDir": "./dist", - "removeComments": false, - "resolveJsonModule": true, - "skipLibCheck": true, - "sourceMap": true, - "strict": true, - "target": "es6", - "types": ["node", "webpack-env", "jest"] - } + "compilerOptions": { + "allowSyntheticDefaultImports": true, + "allowJs": true, + "baseUrl": "src", + "checkJs": false, + "downlevelIteration": true, + "emitDecoratorMetadata": true, + "experimentalDecorators": true, + "importHelpers": true, + "jsx": "react", + "lib": ["es2015", "es2017", "dom", "dom.iterable"], + "module": "commonjs", + "moduleResolution": "node", + "noFallthroughCasesInSwitch": true, + "noImplicitReturns": true, + "noUnusedLocals": false, + "noUnusedParameters": false, + "outDir": "./dist", + "removeComments": false, + "resolveJsonModule": true, + "skipLibCheck": true, + "sourceMap": true, + "strict": true, + "target": "es6", + "types": ["node", "webpack-env", "jest"] + } }