Skip to content

[History Server] Add unit tests for timeline pure functions - #5231

Open
AlterHoodie wants to merge 1 commit into
ray-project:masterfrom
AlterHoodie:unit_tests_timeline
Open

[History Server] Add unit tests for timeline pure functions#5231
AlterHoodie wants to merge 1 commit into
ray-project:masterfrom
AlterHoodie:unit_tests_timeline

Conversation

@AlterHoodie

Copy link
Copy Markdown
Contributor

Why are these changes needed?

Adds unit tests for the History Server timeline pure functions. getTasksTimeline reaches 97.4% coverage. The remaining lines are defensive guards for PID/TID lookup failures that cannot be hit with the current two pass design, since both passes use the same filteredTasks data.

image

Closes Sub-Issue 3 in #5222

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(
  cd historyserver
  go test ./pkg/historyserver/ -run 'TestExtractActorIDFromTaskID|TestGetChromeTraceColor|TestGetTasksTimeline' -v
  go test ./pkg/historyserver/ -coverprofile=cover.out \
    -run 'TestExtractActorIDFromTaskID|TestGetChromeTraceColor|TestGetTasksTimeline'
  go tool cover -func=cover.out | grep timeline

@AlterHoodie

Copy link
Copy Markdown
Contributor Author

@win5923 On a side note,
found that durationUs is computed as (EndTime - StartTime) / 1000 with no validation. If EndTime < StartTime, the output would have a negative duration.

// Convert nanoseconds to microseconds
startTimeUs := float64(profEvent.StartTime) / 1000.0
durationUs := float64(profEvent.EndTime-profEvent.StartTime) / 1000.0

Should be fine as long as Ray profiling data does not produce this in practice.

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