Skip to content

Add unit tests for DPC nodeData scheduling primitives - #665

Open
omerap12 wants to merge 1 commit into
llm-d-incubation:mainfrom
omerap12:dpc-nodedata-scheduling-tests
Open

Add unit tests for DPC nodeData scheduling primitives#665
omerap12 wants to merge 1 commit into
llm-d-incubation:mainfrom
omerap12:dpc-nodedata-scheduling-tests

Conversation

@omerap12

Copy link
Copy Markdown
Contributor

Added unit tests for #626.

Before:

~/Desktop/omerap12/llm-d-fast-model-actuation/ [main] make test      
Running unit tests...
go test $(go list ./pkg/... | grep -Ev '/pkg/(api|generated|spi)(/|$)') -coverprofile=cover.out
	github.com/llm-d-incubation/llm-d-fast-model-actuation/pkg/common		coverage: 0.0% of statements
?   	github.com/llm-d-incubation/llm-d-fast-model-actuation/pkg/controller/common	[no test files]
	github.com/llm-d-incubation/llm-d-fast-model-actuation/pkg/controller/dual-pods		coverage: 0.0% of statements

After:

~/Desktop/omerap12/llm-d-fast-model-actuation/ [dpc-nodedata-scheduling-tests] make test
Running unit tests...
go test $(go list ./pkg/... | grep -Ev '/pkg/(api|generated|spi)(/|$)') -coverprofile=cover.out
	github.com/llm-d-incubation/llm-d-fast-model-actuation/pkg/common		coverage: 0.0% of statements
?   	github.com/llm-d-incubation/llm-d-fast-model-actuation/pkg/controller/common	[no test files]
ok  	github.com/llm-d-incubation/llm-d-fast-model-actuation/pkg/controller/dual-pods	1.042s	coverage: 4.3% of statements

Signed-off-by: Omer Aplatony <omerap12@gmail.com>
@MikeSpreitzer

MikeSpreitzer commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

(1) This is a bunch of checksum tests --- tests of an individual function. It is better to test the behavior at the interface of the package.

(2) This is a large collection of very specific scenarios, but even more could be imagined. I think that it would be more maintainable to have one randomized test. Or axiomitize the subject under test and have a test per axiom, with many randomly generated settings for what the axiom ∀-quantifies over.

(3) I do not know a way to write behavioral unit tests of Kubernetes controllers, because the generated fake clientsets do not properly implement object update. The simplest sort of test that I know how to write is an integration test, which makes a test-local apiserver and uses that for the test. There are examples of such things.

(4) See the unit tests for the launcher population controller for how to exercise more precise and repeatable control over time. See https://github.com/kubernetes/apiserver/blob/v0.34.9/pkg/util/flowcontrol/fairqueuing/queueset/queueset_test.go if you want to go further.

@MikeSpreitzer

Copy link
Copy Markdown
Collaborator

Also, see #667 and #668 .

@MikeSpreitzer

Copy link
Copy Markdown
Collaborator

PR #687 will hopefully provide this coverage data and more.

@github-actions

Copy link
Copy Markdown

This PR is marked as stale after 21d of inactivity. After an additional 14d of inactivity (7d to become rotten, then 7d more), it will be closed. To prevent this PR from being closed, add a comment or remove the lifecycle/stale label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants