feat(test): add unit tests to improve coverage for autoscaler, config, and router debug modules by kube-gopher · Pull Request #903 · volcano-sh/kthena

kube-gopher · 2026-04-21T02:43:24Z

Summary

This PR adds unit tests for four previously uncovered packages, improving
overall test coverage from 49.2% toward a more robust baseline.

Motivation

The following packages had 0% or near-zero coverage and contain core business
logic that is critical to system correctness:

pkg/autoscaler/algorithm/revision.go — scaling correction logic
pkg/autoscaler/util/common.go — shared utility functions used across autoscaler
pkg/model-booster-controller/config/config.go — image config accessors
pkg/kthena-router/debug/handlers.go — debug HTTP handlers (was 10.4%)

Changes

Package	File	Coverage Before	Coverage After (est.)
autoscaler/algorithm	revision.go	0%	~99.1%
autoscaler/util	common.go	0%	~76.4%
model-booster-controller/config	config.go	0%	~100%
kthena-router/debug	handlers.go	10.4%	~96.3%

Test Approach

Used table-driven tests throughout for maintainability
Mocked external dependencies (Kubernetes client, HTTP) via interfaces
No new production code changes — test files only

How to Verify

go test ./pkg/autoscaler/algorithm/... -v -cover
go test ./pkg/autoscaler/util/... -v -cover
go test ./pkg/model-booster-controller/config/... -v -cover
go test ./pkg/kthena-router/debug/... -v -cover

Full coverage report

go test ./pkg/... -coverprofile=coverage.out
go tool cover -func=coverage.out | tail -1

Signed-off-by: kube-gopher <jzlyy68@gmail.com>

volcano-sh-bot · 2026-04-21T02:43:30Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign hzxuzhonghu for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

pkg/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gemini-code-assist

Code Review

This pull request introduces a new test file, pkg/autoscaler/algorithm/revision_test.go, which provides comprehensive unit tests for the GetCorrectedInstances function across various scenarios including stable scaling, panic mode, and boundary limits. The review feedback identifies a mathematical discrepancy in the neverExpireMs constant and suggests adding a test case for CurrentInstances = 0 to ensure the algorithm correctly handles scaling from zero.

gemini-code-assist · 2026-04-21T02:46:08Z

+// Must stay well below MaxInt64/2 because LineChartSlidingWindow internally
+// computes maxDriftingMilliseconds = 2 * freshMilliseconds; overflow would
+// cause drifting values to expire immediately.
+const neverExpireMs = int64(1e15) // ~31 years in milliseconds


The comment indicates 1e15 milliseconds is approximately 31 years, but 1e15 ms is actually about 31,709 years. It is likely that 1e12 ms (which is ~31.7 years) or 1e15 microseconds (which is also ~31.7 years) was intended. While a larger TTL doesn't break the test, this discrepancy suggests a potential confusion regarding time units, which can be a source of bugs in distributed systems.

Suggested change

const neverExpireMs = int64(1e15) // ~31 years in milliseconds

const neverExpireMs = int64(1e12) // ~31 years in milliseconds

gemini-code-assist · 2026-04-21T02:46:08Z

+			},
+			expectedCorrected: 10,
+		},
+	}


Consider adding a test case for CurrentInstances = 0. This is a critical edge case for autoscaling. Specifically, the current implementation of getCorrectedInstancesForPanic in revision.go uses a purely relative constraint based on pastSample (which defaults to CurrentInstances if the history is empty). If CurrentInstances is 0, the relative constraint will also be 0, effectively blocking any scale-up from zero in panic mode. Adding a test case would help document and verify this behavior, potentially revealing a need for a minimum increment in panic mode.

Copilot

Pull request overview

Adds a new unit test suite for the autoscaler “revision” correction algorithm, exercising stable vs panic behaviors, selection policies, history-window effects, and global min/max clamping.

Changes:

Introduces revision_test.go with table-driven tests for CorrectedInstancesAlgorithm.GetCorrectedInstances()
Adds small test helpers (emptyHistory, makeBehavior) and a long-TTL window setup to avoid time-based expirations during tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-21T02:46:38Z

+// Must stay well below MaxInt64/2 because LineChartSlidingWindow internally
+// computes maxDriftingMilliseconds = 2 * freshMilliseconds; overflow would
+// cause drifting values to expire immediately.
+const neverExpireMs = int64(1e15) // ~31 years in milliseconds


The comment on neverExpireMs is inaccurate: 1e15 milliseconds is ~31,700 years (not ~31 years). Please correct the comment or adjust the constant to match the stated duration to avoid misleading future maintainers.

Suggested change

const neverExpireMs = int64(1e15) // ~31 years in milliseconds

const neverExpireMs = int64(1e15) // ~31,700 years in milliseconds

Copilot · 2026-04-21T02:46:38Z

+package algorithm
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/volcano-sh/kthena/pkg/apis/workload/v1alpha1"
+	"github.com/volcano-sh/kthena/pkg/autoscaler/datastructure"
+)


PR description claims new unit tests were added for pkg/autoscaler/util/common.go and pkg/model-booster-controller/config/config.go, but there are no corresponding tests in those packages (no *_test.go found under pkg/autoscaler/util besides client_test.go, and none under pkg/model-booster-controller/config). Either add the missing tests or update the PR title/description and coverage expectations accordingly.

Signed-off-by: kube-gopher <jzlyy68@gmail.com>

…g accessors Signed-off-by: kube-gopher <jzlyy68@gmail.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-21T02:59:22Z

+// Must stay well below MaxInt64/2 because LineChartSlidingWindow internally
+// computes maxDriftingMilliseconds = 2 * freshMilliseconds; overflow would
+// cause drifting values to expire immediately.
+const neverExpireMs = int64(1e15) // ~31 years in milliseconds


The comment for neverExpireMs is mathematically incorrect: 1e15 milliseconds is ~31,700 years, not ~31 years. Please either update the comment to match the constant, or adjust the constant to the intended ~31-year value (and keep it safely below math.MaxInt64/2 since the window multiplies by 2 internally).

Suggested change

const neverExpireMs = int64(1e15) // ~31 years in milliseconds

const neverExpireMs = int64(1e15) // ~31,700 years in milliseconds

Copilot · 2026-04-21T02:59:22Z

+	before := time.Now().UnixMilli()
+	got := GetCurrentTimestamp()
+	after := time.Now().UnixMilli()
+
+	if got < before || got > after {
+		t.Errorf("GetCurrentTimestamp() = %d, want in range [%d, %d]", got, before, after)
+	}


TestGetCurrentTimestamp can be flaky because it relies on wall-clock time moving monotonically between two time.Now().UnixMilli() calls. In CI/VM environments, clock adjustments (e.g., NTP) can cause before/after to jump and intermittently fail. Consider loosening the assertion (e.g., allow a small ±delta), or testing a weaker invariant (non-zero / within a broad range) to avoid time-sync flakes.

Suggested change

before := time.Now().UnixMilli()

got := GetCurrentTimestamp()

after := time.Now().UnixMilli()

if got < before || got > after {

t.Errorf("GetCurrentTimestamp() = %d, want in range [%d, %d]", got, before, after)

}

got := GetCurrentTimestamp()

now := time.Now().UnixMilli()

const allowedDeltaMs int64 = 5000

diff := got - now

if diff < 0 {

diff = -diff

}

if diff > allowedDeltaMs {

t.Errorf("GetCurrentTimestamp() = %d, want within %dms of current time %d", got, allowedDeltaMs, now)

}

Signed-off-by: kube-gopher <jzlyy68@gmail.com>

kube-gopher · 2026-04-21T03:05:06Z

/test all

volcano-sh-bot · 2026-04-21T03:05:29Z

@kube-gopher: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/test all

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test(autoscaler/algorithm): add unit tests for revision correction logic

e0dacb7

Signed-off-by: kube-gopher <jzlyy68@gmail.com>

Copilot AI review requested due to automatic review settings April 21, 2026 02:43

volcano-sh-bot requested review from LiZhenCheng9527 and YaoZengzeng April 21, 2026 02:43

volcano-sh-bot added the size/L label Apr 21, 2026

Copilot started reviewing on behalf of kube-gopher April 21, 2026 02:43 View session

gemini-code-assist Bot reviewed Apr 21, 2026

View reviewed changes

Copilot AI reviewed Apr 21, 2026

View reviewed changes

test(autoscaler/util): add unit tests for common utility functions

51a85f3

Signed-off-by: kube-gopher <jzlyy68@gmail.com>

volcano-sh-bot added size/XL and removed size/L labels Apr 21, 2026

test(model-booster-controller/config): add unit tests for image confi…

919f0e5

…g accessors Signed-off-by: kube-gopher <jzlyy68@gmail.com>

Copilot AI review requested due to automatic review settings April 21, 2026 02:55

Copilot started reviewing on behalf of kube-gopher April 21, 2026 02:56 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

test(kthena-router/debug): add unit tests for debug HTTP handlers

704386d

Signed-off-by: kube-gopher <jzlyy68@gmail.com>

volcano-sh-bot added size/XXL and removed size/XL labels Apr 21, 2026

kube-gopher requested a review from Copilot April 21, 2026 03:09

Copilot started reviewing on behalf of kube-gopher April 21, 2026 03:10 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

	const neverExpireMs = int64(1e15) // ~31 years in milliseconds
	const neverExpireMs = int64(1e12) // ~31 years in milliseconds

-	before := time.Now().UnixMilli()
-	got := GetCurrentTimestamp()
-	after := time.Now().UnixMilli()
-	if got < before || got > after {
-		t.Errorf("GetCurrentTimestamp() = %d, want in range [%d, %d]", got, before, after)
-	}
+	got := GetCurrentTimestamp()
+	now := time.Now().UnixMilli()
+	const allowedDeltaMs int64 = 5000
+	diff := got - now
+	if diff < 0 {
+		diff = -diff
+	}
+	if diff > allowedDeltaMs {
+		t.Errorf("GetCurrentTimestamp() = %d, want within %dms of current time %d", got, allowedDeltaMs, now)
+	}

Conversation

kube-gopher commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Changes

Test Approach

How to Verify

Full coverage report

Uh oh!

volcano-sh-bot commented Apr 21, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

kube-gopher commented Apr 21, 2026

Uh oh!

volcano-sh-bot commented Apr 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kube-gopher commented Apr 21, 2026 •

edited

Loading