Skip to content

test: add missing coverage for getTaskDuration#198

Open
Harshil-Malisetty wants to merge 2 commits intoNetflix:masterfrom
Harshil-Malisetty:test/getTaskDuration
Open

test: add missing coverage for getTaskDuration#198
Harshil-Malisetty wants to merge 2 commits intoNetflix:masterfrom
Harshil-Malisetty:test/getTaskDuration

Conversation

@Harshil-Malisetty
Copy link
Copy Markdown

@Harshil-Malisetty Harshil-Malisetty commented Mar 27, 2026

Summary

Requirements for a pull request

  • Unit tests related to the change have been updated
  • Documentation related to the change has been updated

Adds 10 Cypress component-level tests for the two pure utility functions in src/utils/task.ts:

  • getTaskId — resolves a display-friendly task identifier
  • getTaskDuration — returns a duration in milliseconds, with live-elapsed-time handling for running tasks

No production code was changed. This is a test-only PR.


Why this matters

getTaskDuration is called on every task row in the Timeline, DAG, and Task List views. A subtle regression (e.g. returning NaN or a negative number) would silently break duration columns across the UI. These tests lock down the existing behaviour so future changes — like adding killed status support — can be made safely with regression coverage.


What was added

# Test case Input Expected
1 getTaskId returns task_name when present { task_name: 'hello', task_id: 123 } 'hello'
2 getTaskId falls back to task_id { task_id: 123 } '123'
3 Completed task → stored duration { status: 'completed', duration: 5000 } 5000
4 Completed task, no duration or timing fields { status: 'completed', duration: undefined, finished_at: undefined } null
5 Failed task, no duration { status: 'failed', duration: undefined } null
6 Running task with started_at → elapsed time { status: 'running', started_at: Date.now() - 3000 } ≈ 3000 (±200 ms tolerance)
7 Running task, no started_at { status: 'running', started_at: undefined } null
8 Completed task ignores started_at { status: 'completed', duration: 1000, started_at: Date.now() - 9999 } 1000
9 Killed task → stored duration { status: 'killed', duration: 4200 } 4200
10 Killed task, no duration { status: 'killed', duration: undefined } null

Test infrastructure

  • Test helper: Uses createTask() from src/utils/testhelper.ts — a factory that spreads partial overrides onto a default Task object, keeping each test focused on the fields under test.
  • Runner: Cypress component tests (cypress.config.ts), executed via npx cypress run --component.
  • Assertions: Chai-style expect(...).to.equal(...) with greaterThan / lessThan for the elapsed-time tolerance window.

Files changed

File Change
src/utils/__tests__/task.test.cypress.ts New — 10 tests covering getTaskId and getTaskDuration

How to verify

npx cypress run --component --spec src/utils/__tests__/task.test.cypress.ts

All 10 tests should pass green.


Design decisions

  1. Killed-status tests (cases 9–10): The current getTaskDuration implementation doesn't branch on 'killed' explicitly — killed tasks fall through to the task.duration ? task.duration : null path. The tests document this existing behaviour so that when killed is added as a first-class status (PR feat: distinguish externally killed tasks from code failures (#84) #187), any logic change is caught by CI.

  2. Elapsed-time tolerance (±200 ms): Test 6 asserts the running-task elapsed duration is between 2800–3200 ms rather than an exact value, accounting for CI/test-execution jitter.

  3. No mocking of Date.now(): The tolerance approach was chosen over mocking to keep the test simple and closer to real runtime behaviour.

getTaskDuration had no test coverage despite handling multiple cases:
completed tasks, running tasks (elapsed time), tasks with no timing
fields, and the edge case where a completed task should not use elapsed
time even when started_at is present.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant